to further what Larry is saying, you might want to check out using
numeric limits to initialise min and max based on your source data
type, (or at least for float as that is the type of your min and max
variables) see https://en.cppreference.com/w/cpp/types/numeric_limits/lowest

So you should initialise your 'min' to the value provided by
numeric_limits::max() and your 'max' value to
numeric_limits::lowest(), or you could initialise both of them to the
value of the first pixel.

kevin

On Thu, Nov 28, 2019 at 3:06 AM Larry Gritz <l...@larrygritz.com> wrote:
>
> I'm still concerned about your initialization of the "min" variable. Can you 
> see how this will give the incorrect result if the minimum pixel value in 
> your image is > 0 ?
>
> As written below, this will scale up the lightest values to fully "white", 
> but it will fail to convert the darkest values to fully black.
>
>
>
> On Nov 27, 2019, at 6:14 PM, Chris Dawlud <kdaw...@protonmail.com> wrote:
>
> Thank you. It makes a lot of sense.
> I completely forgot about discarding alpha channel. Also, I'm using half 
> format so in my specific case I should have used half instead of float in the 
> iterator constructor.
>
> For future reference, here's the working code:
>
> void ImageLoader::normalize(OIIO::ImageBuf* image)
> {
> using namespace OIIO;
>
> auto stats = ImageBufAlgo::computePixelStats(*image);
>
> auto min = 0.0f;
> auto max = 0.0f;
>
> auto nc = std::min(3, image->nchannels());
>
> for (auto c=0; c < nc; ++c) {
> min = std::min(min, stats.min[c]);
> max = std::max(max, stats.max[c]);
> }
>
> auto roi = image->roi();
> roi.chend = nc;
> ImageBufAlgo::contrast_remap(*image, *image, min, max, 0.0f, 1.0f, 1.0, 0.5f, 
> roi);
> }
>
>
>
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Thursday, 28 November 2019 02:32, Larry Gritz <l...@larrygritz.com> wrote:
>
> To clarify: I believe that the contrast_remap function is new in OIIO 2.1. On 
> older versions, or merely if you prefer it, I think you could achieve the 
> same thing with
>
>     ImageBufAlgo::sub(*image, *image, min, roi);
>     ImageBufAlgo::mul(*image, *image, (max-min), roi);
>
> Note also that this whole business of passing the roi explicitly is to limit 
> the pixel alterations to just the first 3 channels. You almost surely don't 
> want to scale the alpha values or any additional channels.
>
>
> On Nov 27, 2019, at 5:27 PM, Larry Gritz <l...@larrygritz.com> wrote:
>
> In what way doesn't it work properly? Does it crash? Appear to do nothing? Do 
> something but not what you expect? What kind of image are you trying (I mean, 
> what data type and channels)?
>
> I can only speak in generalities without this info, but here's what I spotted 
> as likely problems.
>
> * ConstIterator/Iterator<float> is likely to be problematic, because it 
> requires the buffer itself to be float. (Is it?) Usually we write a function 
> like your normalize as a template and then have a big switch statement that 
> selects a specialized variety based on the data type. (This may not really be 
> a problem if all of your images are stored as 'float' buffers internally.)
>
> * You initialized 'min' (why are these half?) to 0.0, which means that "if 
> (it[c] < min)" will probably never be true (unless you have negative values), 
> so it probably is not going to compute the correct minimum. A better way to 
> compute min/max would be:
>
>     float min = 1e6f;
>     float max = -1e6f;
>     ...
>     for (int c = 0; c < nchannels; ++c) {
>         min = std::min(max, it[c]);
>         max = std::max(max, it[c]);
>     }
>
> * Depending on your image, you may also find that scaling all channels based 
> on the biggest min/max may be wrong, particularly if it has any non-color 
> chanels like alpha or Z.
>
>
> So OIIO 2.1 (a release candidate now, scheduled to be the official supported 
> release in 3 days!) has a function that does everything you want. Assuming 
> you're able to use that, here's how I recommend doing this operation robustly 
> (this is off the top of my head, I have not tested, so take with a grain of 
> salt, but this is the basic idea):
>
>     ImageBufAlgo::PixelStats stats = ImageBufAlgo::computePixelStats(*image);
>     float min = 1e6f;
>     float max = -1e6f;
>     int nc = std::min (3, image->nchannels());  // only consider R,G,B
>     for (int c = 0; c < nc; ++c) {
>         min = std::min(max, stats.min[c]);
>         max = std::max(max, stats.max[c]);
>     }
>     ROI roi = image->roi();
>     roi.chend = 3;  // only rescale at most the first three channels!
>     ImageBufAlgo::contrast_remap(*image, *image, min, max, 0.0f, 1.0f, 1.0, 
> 0.5, roi);
>
>
> Using IBA::computePixelStats and contrast_remap has two big advantages over 
> writing the loops yourself: (1) it handles every data type that OIIO 
> supports, not just float; and (2) will go much faster because internally it's 
> multithreaded!
>
>
>
> On Nov 27, 2019, at 4:31 PM, Chris Dawlud <kdaw...@protonmail.com> wrote:
>
> Hi,
>
> I want to normalize exr to make even very bright/dark images visible.
> The idea is simple but I can't figure out how to do this correctly.
>
> I have come up with the following, but it doesn't work properly:
>
> void ImageLoader::normalize(OIIO::ImageBuf* image)
> {
> using namespace OIIO;
>
> auto min = half{ 0.0f };
> auto max = half{ 0.0f };
>
> for (auto it = ImageBuf::ConstIterator<float>{ *image }; !it.done(); ++it)
> for (auto c = 0; c < image->nchannels(); ++c) {
> if (it[c] < min) min = it[c];
> if (it[c] > max) max = it[c];
> }
>
> for (auto it = ImageBuf::Iterator<float>{ *image }; !it.done(); ++it)
> for (auto c = 0; c < image->nchannels(); ++c)
> it[c] = 0.0f + (it[c] - min) * ( 1.0f  - 0.0f ) / (max - min);
>
> }
>
>
> _______________________________________________
> Oiio-dev mailing list
> Oiio-dev@lists.openimageio.org
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>
>
> --
> Larry Gritz
> l...@larrygritz.com
>
>
>
>
> _______________________________________________
> Oiio-dev mailing list
> Oiio-dev@lists.openimageio.org
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>
>
> --
> Larry Gritz
> l...@larrygritz.com
>
>
> _______________________________________________
> Oiio-dev mailing list
> Oiio-dev@lists.openimageio.org
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>
>
> --
> Larry Gritz
> l...@larrygritz.com
>
>
>
>
> _______________________________________________
> Oiio-dev mailing list
> Oiio-dev@lists.openimageio.org
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
_______________________________________________
Oiio-dev mailing list
Oiio-dev@lists.openimageio.org
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

Reply via email to