-
Notifications
You must be signed in to change notification settings - Fork 648
feat(python): _repr_png_ implementation for ImageBuf #4753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(python): _repr_png_ implementation for ImageBuf #4753
Conversation
Uses the IOVecOutput IOProxy to write the ImageBuf as a png into memory. Then, returns that as a python bytes object. Signed-off-by: glowies <[email protected]>
Signed-off-by: glowies <[email protected]>
Signed-off-by: glowies <[email protected]>
|
This is super cool. Just thinking aloud... if we're writing 8bit pngs for the sake of ephemeral preview, we should enable dithering as well. Perhaps for a future PR, I'd love for this to automatically incorporate an It's not at all difficult to enable this functionality in OCIO; but it's not something that's enabled in the OCIO ACES configs, so we'd have to modify the built-in OCIO config that OIIO uses by default. (Which is something I'd recommend for OIIO in general, since OIIO doesn't necessarily share the same assumptions as the ACES configs do). |
lgritz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Simple, really cool feature that unlocks some new use cases, and it's also a great example of IOProxy to write image files to memory.
LGTM on the code!
I would like to ask for you to add something in the docs to the python chapter? This needs to be more discoverable by users. Also, is there any way to test this as part of the testsuite?
I'll look into adding dithering there! Not exactly sure how to do it, but I'm sure its somewhere in the docs 😀 And a follow-up issue for the Display/View transforms sounds like a great idea. Definitely sounds more involved than I thought! |
I'll happily add to the docs! Where would be the most appropriate? I see there is a large Python Bindings page. Should it go in there or have a separate page dedicated to python notebooks? For the testsuites, I think I can add some very simple tests to see if the correct bytes get output from repr_png and that it doesnt crash for 'dimensionless' images. |
|
This is a great start (modulo needing a mention in the docs). Don't feel like you need to keep piling on additional features into the first PR. I'm a big fan of small incremental PRs. OIIO's png output already has a provision for dithering the output when converting from float to uint8 upon output. All you have to do (I think) is add the attribute "oiio:dither" (with a nonzero value) to the metadata, which you can also do through the ImageBuf interface. |
Yes, exactly what I had in mind. |
(Totally agree, feel free to ignore me!) |
… 8bit png Signed-off-by: glowies <[email protected]>
Signed-off-by: glowies <[email protected]>
|
The dither addition looks right. |
Signed-off-by: glowies <[email protected]>
Signed-off-by: glowies <[email protected]>
Signed-off-by: glowies <[email protected]>
Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Larry Gritz <[email protected]>
lgritz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After previewing the docs, I made two extremely small changes to for you, didn't change any text, just to fix it so the formatting was more like the surrounding section.
The rest all looks good to me, I think we are ready to merge this cool new feature!
…Foundation#4753) Fixes AcademySoftwareFoundation#4680 Adds a `_repr_png_` implementation to the Python bindings of ImageBuf. This allows ImageBuf objects to be displayed in Jupyter notebooks, as requested in AcademySoftwareFoundation#4680 Uses the IOVecOutput IOProxy to write the ImageBuf as a png into memory. Then, reads and returns that as a python bytes object.  --------- Signed-off-by: glowies <[email protected]>
…Foundation#4753) Fixes AcademySoftwareFoundation#4680 Adds a `_repr_png_` implementation to the Python bindings of ImageBuf. This allows ImageBuf objects to be displayed in Jupyter notebooks, as requested in AcademySoftwareFoundation#4680 Uses the IOVecOutput IOProxy to write the ImageBuf as a png into memory. Then, reads and returns that as a python bytes object.  --------- Signed-off-by: glowies <[email protected]> Signed-off-by: Scott Wilson <[email protected]>
) This PR fixes and improves a tiny constellation of OCIO-related bits, directly or indirectly pertaining to OCIO Viewing Rules and File Rules, and the propagation of `oiio:ColorSpace`. ## Description The focus of this PR: - feat: Permit the `ociodisplay` ImageBufAlgo to dynamically select a default View for the `fromspace` (or current `oiio:ColorSpace` value), as directed by the OCIO config's Viewing Rules (if present) - feat: Have the `ociodisplay` IBA appropriately set the destination ImageSpec's color space. As part of this effort, a handful of additional changes and improvements are addressed: - fix: Re-enable the "looks" override functionality, matching the previous OCIO-1-style behavior. - fix: Improve `ColorConfig::getDisplayViewColorSpaceName(...)`'s handling of Shared Views - feat: Overload `ColorConfig::getDefaultViewName(...)` with function that takes an additional `inputColorSpace` argument - change: Alter the `ociofiletransform` IBA's behavior pertaining to setting the destination spec's color space: instead of storing the filepath as the `oiio:ColorSpace` value, attempt to get the color space from the filepath; else, leave the `oiio:ColorSpace` attribute alone. This isn't 100% foolproof, obviously, but I'd prefer to reserve the `oiio:ColorSpace` attribute for color space names, unless there's a reason not to. In a future PR, we can additionally attempt to divine a valid color space string from specific CLF and CTF metadata, if present. - feat: Implement a `ColorConfig::filepathOnlyMatchesDefaultRule(...)` function, which wraps the OCIO::Config function of the same name. - change: Alter `ColorConfig::getColorSpaceFromFilepath(...)` to behave more like the OCIO::Config function of the same name. In particular, return the Config's default color space name (instead of failing over to OCIO-1 string-parsing heuristics, which, in turn, fail over to an empty string); and provide an overloaded variant for implementing the legacy (i.e., current) behavior. Update all usages in the codebase (i.e., in oiiotool, w.r.t. autocc) to use the overloaded variant, so there's no apparent change in behavior to how autocc functions. ## Background This PR was inspired by #4753, which adds to the Python ImageBuf bindings a `_repr_png_` magic method which returns a PNG bytestring (which Jupyter notebooks can display inline), similar in behavior to other libraries, like Pillow. This is great for previewing gamma-encoded output-referred buffer data, but less great for previewing "linear" encodings. Ideally, the `_repr_png_` method would apply an `ociodisplay` ImageBufAlgo just before writing the PNG; but there would need to be some means of controlling for the buffer "image state" when determining how / whether to apply an `ociodisplay` IBA. For example, it would be appropriate to apply the default ColorConfig's "ACES 1.0 - SDR Video" View to an image buffer that contains scene-referred data; but very evidently inappropriate to apply that same View to an image buffer that holds display-referred data, yielding a "double-LUTted" appearance. Fortunately, OCIO-2 `Viewing Rules` provides a solution. Viewing Rules allow OCIO Config authors to selectively filter the set of Views available for a given Display, on a per-input-colorspace basis. Ostensibly, applications use the OCIO API's DisplayViewHelper class for building View menu UX widgets that dynamically update as a function of both the selected Display and Input Color Space; but there are implications for headless applications, as well -- crucially, the Config author may order Active Views such that different Input Color Spaces elicit different default Views. ## Tests I've included a test OCIO config with some Viewing Rules and File Rules in testsuite/python-colorconfig; I'm testing that the new and updated python bindings behave as expected; and I'm testing that the `ociodisplay` IBA is OCIO-Viewing-Rules-aware and is properly propagating `oiio:ColorSpace` to the dst buffer's spec. Notably: ```python import os import OpenImageIO as oiio import numpy as np os.environ["OCIO"] = "/path/to/testsuite/python-colorconfig/src/oiio_test_v0.9.2.ocio" a = oiio.ImageBuf(np.array([[[0.1, 0.2, 0.3]]])) # The first time the ociodisplay IBA is applied, the lack of an `oiio:ColorSpace` # attribute will be interpreted as the OCIO config's default color space -- in this case, # "lin_rec709", which corresponds to a scene-referred default view (ACES 1.0 - SDR Video). b = oiio.ImageBufAlgo.ociodisplay(a, "", "") # The second time around, the `oiio:ColorSpace` attribute is now "sRGB (~2.22) - Display", # a display-referred color space, thanks to the previous operation; as such, the IBA uses # a different colorimetric "pass-thru" View as its default ("Colorimetry"), instead of the # ACES View. Upshot is, values remain unchanged this time. assert b.spec()['oiio:ColorSpace'], 'sRGB (~2.22) - Display' c = oiio.ImageBufAlgo.ociodisplay(b, "", "") assert c.spec()['oiio:ColorSpace'], 'sRGB (~2.22) - Display' # Subsequent applications will be noops as well. d = oiio.ImageBufAlgo.ociodisplay(c, "", "") assert np.allclose(b.get_pixels(), c.get_pixels()) assert np.allclose(b.get_pixels(), d.get_pixels()) assert not np.allclose(b.get_pixels(), a.get_pixels()) ``` ## TL;DR This PR enables OCIO config authors to dynamically control `ociodisplay` IBA behavior w.r.t. the `oiio:ColorSpace` attribute, such that `ImageBufAlgo.ociodisplay(buf, "", "")` always "does the right thing", regardless of whether `buf` holds scene-referred or display-referred data. --------- Signed-off-by: Zach Lewis <[email protected]>
…Foundation#4753) Fixes AcademySoftwareFoundation#4680 Adds a `_repr_png_` implementation to the Python bindings of ImageBuf. This allows ImageBuf objects to be displayed in Jupyter notebooks, as requested in AcademySoftwareFoundation#4680 Uses the IOVecOutput IOProxy to write the ImageBuf as a png into memory. Then, reads and returns that as a python bytes object.  --------- Signed-off-by: glowies <[email protected]>
…ademySoftwareFoundation#4780) This PR fixes and improves a tiny constellation of OCIO-related bits, directly or indirectly pertaining to OCIO Viewing Rules and File Rules, and the propagation of `oiio:ColorSpace`. ## Description The focus of this PR: - feat: Permit the `ociodisplay` ImageBufAlgo to dynamically select a default View for the `fromspace` (or current `oiio:ColorSpace` value), as directed by the OCIO config's Viewing Rules (if present) - feat: Have the `ociodisplay` IBA appropriately set the destination ImageSpec's color space. As part of this effort, a handful of additional changes and improvements are addressed: - fix: Re-enable the "looks" override functionality, matching the previous OCIO-1-style behavior. - fix: Improve `ColorConfig::getDisplayViewColorSpaceName(...)`'s handling of Shared Views - feat: Overload `ColorConfig::getDefaultViewName(...)` with function that takes an additional `inputColorSpace` argument - change: Alter the `ociofiletransform` IBA's behavior pertaining to setting the destination spec's color space: instead of storing the filepath as the `oiio:ColorSpace` value, attempt to get the color space from the filepath; else, leave the `oiio:ColorSpace` attribute alone. This isn't 100% foolproof, obviously, but I'd prefer to reserve the `oiio:ColorSpace` attribute for color space names, unless there's a reason not to. In a future PR, we can additionally attempt to divine a valid color space string from specific CLF and CTF metadata, if present. - feat: Implement a `ColorConfig::filepathOnlyMatchesDefaultRule(...)` function, which wraps the OCIO::Config function of the same name. - change: Alter `ColorConfig::getColorSpaceFromFilepath(...)` to behave more like the OCIO::Config function of the same name. In particular, return the Config's default color space name (instead of failing over to OCIO-1 string-parsing heuristics, which, in turn, fail over to an empty string); and provide an overloaded variant for implementing the legacy (i.e., current) behavior. Update all usages in the codebase (i.e., in oiiotool, w.r.t. autocc) to use the overloaded variant, so there's no apparent change in behavior to how autocc functions. ## Background This PR was inspired by AcademySoftwareFoundation#4753, which adds to the Python ImageBuf bindings a `_repr_png_` magic method which returns a PNG bytestring (which Jupyter notebooks can display inline), similar in behavior to other libraries, like Pillow. This is great for previewing gamma-encoded output-referred buffer data, but less great for previewing "linear" encodings. Ideally, the `_repr_png_` method would apply an `ociodisplay` ImageBufAlgo just before writing the PNG; but there would need to be some means of controlling for the buffer "image state" when determining how / whether to apply an `ociodisplay` IBA. For example, it would be appropriate to apply the default ColorConfig's "ACES 1.0 - SDR Video" View to an image buffer that contains scene-referred data; but very evidently inappropriate to apply that same View to an image buffer that holds display-referred data, yielding a "double-LUTted" appearance. Fortunately, OCIO-2 `Viewing Rules` provides a solution. Viewing Rules allow OCIO Config authors to selectively filter the set of Views available for a given Display, on a per-input-colorspace basis. Ostensibly, applications use the OCIO API's DisplayViewHelper class for building View menu UX widgets that dynamically update as a function of both the selected Display and Input Color Space; but there are implications for headless applications, as well -- crucially, the Config author may order Active Views such that different Input Color Spaces elicit different default Views. ## Tests I've included a test OCIO config with some Viewing Rules and File Rules in testsuite/python-colorconfig; I'm testing that the new and updated python bindings behave as expected; and I'm testing that the `ociodisplay` IBA is OCIO-Viewing-Rules-aware and is properly propagating `oiio:ColorSpace` to the dst buffer's spec. Notably: ```python import os import OpenImageIO as oiio import numpy as np os.environ["OCIO"] = "/path/to/testsuite/python-colorconfig/src/oiio_test_v0.9.2.ocio" a = oiio.ImageBuf(np.array([[[0.1, 0.2, 0.3]]])) # The first time the ociodisplay IBA is applied, the lack of an `oiio:ColorSpace` # attribute will be interpreted as the OCIO config's default color space -- in this case, # "lin_rec709", which corresponds to a scene-referred default view (ACES 1.0 - SDR Video). b = oiio.ImageBufAlgo.ociodisplay(a, "", "") # The second time around, the `oiio:ColorSpace` attribute is now "sRGB (~2.22) - Display", # a display-referred color space, thanks to the previous operation; as such, the IBA uses # a different colorimetric "pass-thru" View as its default ("Colorimetry"), instead of the # ACES View. Upshot is, values remain unchanged this time. assert b.spec()['oiio:ColorSpace'], 'sRGB (~2.22) - Display' c = oiio.ImageBufAlgo.ociodisplay(b, "", "") assert c.spec()['oiio:ColorSpace'], 'sRGB (~2.22) - Display' # Subsequent applications will be noops as well. d = oiio.ImageBufAlgo.ociodisplay(c, "", "") assert np.allclose(b.get_pixels(), c.get_pixels()) assert np.allclose(b.get_pixels(), d.get_pixels()) assert not np.allclose(b.get_pixels(), a.get_pixels()) ``` ## TL;DR This PR enables OCIO config authors to dynamically control `ociodisplay` IBA behavior w.r.t. the `oiio:ColorSpace` attribute, such that `ImageBufAlgo.ociodisplay(buf, "", "")` always "does the right thing", regardless of whether `buf` holds scene-referred or display-referred data. --------- Signed-off-by: Zach Lewis <[email protected]>
…ademySoftwareFoundation#4780) This PR fixes and improves a tiny constellation of OCIO-related bits, directly or indirectly pertaining to OCIO Viewing Rules and File Rules, and the propagation of `oiio:ColorSpace`. ## Description The focus of this PR: - feat: Permit the `ociodisplay` ImageBufAlgo to dynamically select a default View for the `fromspace` (or current `oiio:ColorSpace` value), as directed by the OCIO config's Viewing Rules (if present) - feat: Have the `ociodisplay` IBA appropriately set the destination ImageSpec's color space. As part of this effort, a handful of additional changes and improvements are addressed: - fix: Re-enable the "looks" override functionality, matching the previous OCIO-1-style behavior. - fix: Improve `ColorConfig::getDisplayViewColorSpaceName(...)`'s handling of Shared Views - feat: Overload `ColorConfig::getDefaultViewName(...)` with function that takes an additional `inputColorSpace` argument - change: Alter the `ociofiletransform` IBA's behavior pertaining to setting the destination spec's color space: instead of storing the filepath as the `oiio:ColorSpace` value, attempt to get the color space from the filepath; else, leave the `oiio:ColorSpace` attribute alone. This isn't 100% foolproof, obviously, but I'd prefer to reserve the `oiio:ColorSpace` attribute for color space names, unless there's a reason not to. In a future PR, we can additionally attempt to divine a valid color space string from specific CLF and CTF metadata, if present. - feat: Implement a `ColorConfig::filepathOnlyMatchesDefaultRule(...)` function, which wraps the OCIO::Config function of the same name. - change: Alter `ColorConfig::getColorSpaceFromFilepath(...)` to behave more like the OCIO::Config function of the same name. In particular, return the Config's default color space name (instead of failing over to OCIO-1 string-parsing heuristics, which, in turn, fail over to an empty string); and provide an overloaded variant for implementing the legacy (i.e., current) behavior. Update all usages in the codebase (i.e., in oiiotool, w.r.t. autocc) to use the overloaded variant, so there's no apparent change in behavior to how autocc functions. ## Background This PR was inspired by AcademySoftwareFoundation#4753, which adds to the Python ImageBuf bindings a `_repr_png_` magic method which returns a PNG bytestring (which Jupyter notebooks can display inline), similar in behavior to other libraries, like Pillow. This is great for previewing gamma-encoded output-referred buffer data, but less great for previewing "linear" encodings. Ideally, the `_repr_png_` method would apply an `ociodisplay` ImageBufAlgo just before writing the PNG; but there would need to be some means of controlling for the buffer "image state" when determining how / whether to apply an `ociodisplay` IBA. For example, it would be appropriate to apply the default ColorConfig's "ACES 1.0 - SDR Video" View to an image buffer that contains scene-referred data; but very evidently inappropriate to apply that same View to an image buffer that holds display-referred data, yielding a "double-LUTted" appearance. Fortunately, OCIO-2 `Viewing Rules` provides a solution. Viewing Rules allow OCIO Config authors to selectively filter the set of Views available for a given Display, on a per-input-colorspace basis. Ostensibly, applications use the OCIO API's DisplayViewHelper class for building View menu UX widgets that dynamically update as a function of both the selected Display and Input Color Space; but there are implications for headless applications, as well -- crucially, the Config author may order Active Views such that different Input Color Spaces elicit different default Views. ## Tests I've included a test OCIO config with some Viewing Rules and File Rules in testsuite/python-colorconfig; I'm testing that the new and updated python bindings behave as expected; and I'm testing that the `ociodisplay` IBA is OCIO-Viewing-Rules-aware and is properly propagating `oiio:ColorSpace` to the dst buffer's spec. Notably: ```python import os import OpenImageIO as oiio import numpy as np os.environ["OCIO"] = "/path/to/testsuite/python-colorconfig/src/oiio_test_v0.9.2.ocio" a = oiio.ImageBuf(np.array([[[0.1, 0.2, 0.3]]])) # The first time the ociodisplay IBA is applied, the lack of an `oiio:ColorSpace` # attribute will be interpreted as the OCIO config's default color space -- in this case, # "lin_rec709", which corresponds to a scene-referred default view (ACES 1.0 - SDR Video). b = oiio.ImageBufAlgo.ociodisplay(a, "", "") # The second time around, the `oiio:ColorSpace` attribute is now "sRGB (~2.22) - Display", # a display-referred color space, thanks to the previous operation; as such, the IBA uses # a different colorimetric "pass-thru" View as its default ("Colorimetry"), instead of the # ACES View. Upshot is, values remain unchanged this time. assert b.spec()['oiio:ColorSpace'], 'sRGB (~2.22) - Display' c = oiio.ImageBufAlgo.ociodisplay(b, "", "") assert c.spec()['oiio:ColorSpace'], 'sRGB (~2.22) - Display' # Subsequent applications will be noops as well. d = oiio.ImageBufAlgo.ociodisplay(c, "", "") assert np.allclose(b.get_pixels(), c.get_pixels()) assert np.allclose(b.get_pixels(), d.get_pixels()) assert not np.allclose(b.get_pixels(), a.get_pixels()) ``` ## TL;DR This PR enables OCIO config authors to dynamically control `ociodisplay` IBA behavior w.r.t. the `oiio:ColorSpace` attribute, such that `ImageBufAlgo.ociodisplay(buf, "", "")` always "does the right thing", regardless of whether `buf` holds scene-referred or display-referred data. --------- Signed-off-by: Zach Lewis <[email protected]>
Fixes #4680
Description
Adds the ability for ImageBuf objects to be displayed in Jupyter notebooks, as requested in #4680

Uses the IOVecOutput IOProxy to write the ImageBuf as a png into memory. Then, reads and returns that as a python bytes object.
Checklist:
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.