fix(oiiotool): --autocc bugfix and color config inventory cleanup #4060
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The important bug fix is in oiiotool image input when autocc is used, where we reversed the sense of a test and did the autocc-upon-input if the file's color space was equivalent to the scene_linear space (pointlessly), and skipped the conversion if they were different (oh no, big bug!). Along the way: * Add missing try/catch around OCIO call that should have had it. * Some very ugly SPI-specific recognize-by-name color space heuristics. I hate this, sorry, but it solves a really important problem for me. * A bunch of additional debugging messages during color space inventory, enabled only when debugging, so nobody should see these but me. * A couple places where we canonicalize the spelling of "oiio:ColorSpace". Note that there is still an issue with the color space classification using OCIO 2.3+'s identification of built-in equivalents by name. These are shockingly expensive. I will return to this later. Signed-off-by: Larry Gritz <lg@larrygritz.com>
lgritz
changed the title
jessey-git
reviewed
Nov 23, 2023
| @@ -698,26 +726,36 @@ void | |||
| ColorConfig::Impl::identify_builtin_equivalents() | |||
| { | |||
| #if OCIO_VERSION_HEX >= MAKE_OCIO_VERSION_HEX(2, 3, 0) | |||
| Timer timer; | |||
There was a problem hiding this comment.
OIIO_MAYBE_UNUSED Timer timer;
There was a problem hiding this comment.
Is that needed? I'm not seeing any build errors on CI. I think that the constructor of the timer itself "does something", so it's not unused.
There was a problem hiding this comment.
Unsure if it's really needed but another method a little below this used that pattern.
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 24, 2023
…ademySoftwareFoundation#4060) The important bug fix is in oiiotool image input when autocc is used, where we reversed the sense of a test and did the autocc-upon-input if the file's color space was equivalent to the scene_linear space (pointlessly), and skipped the conversion if they were different (oh no, big bug!). Along the way: * Add missing try/catch around OCIO call that should have had it. * Some very ugly SPI-specific recognize-by-name color space heuristics. I hate this, sorry, but it solves a really important problem for me. * A bunch of additional debugging messages during color space inventory, enabled only when debugging, so nobody should see these but me. * A couple places where we canonicalize the spelling of "oiio:ColorSpace". Note that there is still an issue with the color space classification using OCIO 2.3+'s identification of built-in equivalents by name. These are shockingly expensive. I will return to this later. Signed-off-by: Larry Gritz <lg@larrygritz.com>
1div0
pushed a commit
to 1div0/OpenImageIO
that referenced
this pull request
Feb 24, 2024
…ademySoftwareFoundation#4060) The important bug fix is in oiiotool image input when autocc is used, where we reversed the sense of a test and did the autocc-upon-input if the file's color space was equivalent to the scene_linear space (pointlessly), and skipped the conversion if they were different (oh no, big bug!). Along the way: * Add missing try/catch around OCIO call that should have had it. * Some very ugly SPI-specific recognize-by-name color space heuristics. I hate this, sorry, but it solves a really important problem for me. * A bunch of additional debugging messages during color space inventory, enabled only when debugging, so nobody should see these but me. * A couple places where we canonicalize the spelling of "oiio:ColorSpace". Note that there is still an issue with the color space classification using OCIO 2.3+'s identification of built-in equivalents by name. These are shockingly expensive. I will return to this later. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The important bug fix is in oiiotool image input when autocc is used, where we reversed the sense of a test and did the autocc-upon-input if the file's color space was equivalent to the scene_linear space (pointlessly), and skipped the conversion if they were different (oh no, big bug!).
Along the way:
Add missing try/catch around OCIO call that should have had it.
Some very ugly SPI-specific recognize-by-name color space heuristics. I hate this, sorry, but it solves a really important problem for me.
A bunch of additional debugging messages during color space inventory, enabled only when debugging, so nobody should see these but me.
A couple places where we canonicalize the spelling of "oiio:ColorSpace".
Note that there is still an issue with the color space classification using OCIO 2.3+'s identification of built-in equivalents by name. These are shockingly expensive. I will return to this later.