AcademySoftwareFoundation / OpenImageIO

Reading, writing, and processing images in a wide variety of file formats, using a format-agnostic API, aimed at VFX applications.
https://openimageio.readthedocs.org
Apache License 2.0
1.92k stars 575 forks source link

[FEATURE REQUEST] Add oiiotool `--trim` functionality to ImageBufAlgo #3309

Open nrusch opened 2 years ago

nrusch commented 2 years ago

The --trim and --autotrim arguments to oiiotool are very handy for optimizing EXR data windows. That functionality feels like it would be a natural addition to ImageBufAlgo, as it would make it easy to use in bespoke processing tools without having to manually replicate the logic from oiiotool.

lgritz commented 2 years ago

This already exists, it's just that it's split into two calls. Underneath, --trim is just doing:

ImageBuf img;   // your image

// Find the smallest rectangle containing all the non-zero-valued pixels of an image
ROI nzregion = ImageBufAlgo::nonzero_region(img);

// Trim the image to this rectangle
ImageBuf cropped = ImageBufAlgo::crop(img, nzregion);

--autotrim just means is "every time you use -o and you are saving to a file format that supports separate display windows vs data windows, do the --trim operation (nondestructively) right before saving." You see, the trick is that it's only useful to do this trimming for some file formats.

Knowing that it's just two lines using existing calls, do you still think it's worth adding an additional function that combines them? Or better to leave as-is but maybe be sure that it's explained more clearly in the docs how to accomplish this with the existing calls?

nrusch commented 2 years ago

Yeah, I can certainly see the argument for not wanting to bundle every useful IBA recipe into OIIO itself.

Since it's not quite as simple as just those two calls in practice, I think adding an example to the documentation would be quite useful. In the example, I would suggest including the handling of the empty non-zero ROI.

lgritz commented 2 years ago

I want to re-open so that at the very least, I have a reminder to fix the docs.

Can you elaborate on "not quite as simple"? If the empty non-zero ROI the only important complication? (The other I can think of is that if the WHOLE image is the nonzero ROI, then you probably want to skip the needless crop.)

What do you think is the correct behavior for an all-black image? What should it crop to? The upper left pixel only, or something like that?

nrusch commented 2 years ago

Can you elaborate on "not quite as simple"

Sorry, my thoughts are basically exactly what you said: handling an empty ROI, and handling an unchanged ROI.

This is more or less what I would consider to be a minimal "complete" Python example:

nonzeroROI = oiio.ImageBufAlgo.nonzero_region(buf)
if nonzeroROI.npixels == 0:
    # Special case for an empty image: doctor the ROI to make it a
    # single zero pixel.
    nonzeroROI = buf.roi
    nonzeroROI.xend = nonzeroROI.xbegin + 1
    nonzeroROI.yend = nonzeroROI.ybegin + 1
    nonzeroROI.zend = nonzeroROI.zbegin + 1

if nonzeroROI != buf.roi:
    croppedBuf = oiio.ImageBufAlgo.crop(buf, roi=nonzeroROI)

What do you think is the correct behavior for an all-black image? What should it crop to? The upper left pixel only, or something like that?

I think a single pixel in the upper-left strikes the right balance between absolute correctness and functional usage, yeah. That said, I'm definitely coming at this from an EXR-centric perspective, so I would understand if you didn't want to put that extra logic into a "universal" example.

lgritz commented 2 years ago

Yeah, in an OpenEXR-centric world, this makes sense. For many other file formats which don't support the full generality of data and display windows, you may end up cropping to the nonzero region, only to be forced to re-pad with black in order to write a correct file.