-
Notifications
You must be signed in to change notification settings - Fork 208
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
sRGB-related crash in 1.4 beta #5695
Comments
Thanks for testing Brian! We've updated OpenImageIO in 1.4, and I think that's where the error originates. I don't know all the details, but I think there are various heuristics in OCIO and/or OIIO to try to find an sRGB transform from an OCIO config. Is there any way you could share the config in question? |
@johnhaddon Sure, here you go: https://github.com/BrianHanke/bhgc_v2.ocio/blob/main/bhgc_v2.ocio There's a lot of sRGB stuff in there, but maybe not the right kind! |
Thanks @BrianHanke ! We can repro this on Linux too with your example OCIO config. |
I think I found the source of the problem: Gaffer 1.4 seems to specifically look for an |
I'm pretty sure it's OpenImageIO that is failing to find the sRGB config rather than Gaffer itself, because we defer the color conversion to OIIO, and you get the exact same error using
I think the real question is if we should file a bug against OIIO, or if there's a good reason the sRGB transform from
I wonder if the old "best guesses" found the sRGB transform, but the new "magic of OCIO 2.2" doesn't. I think the magic actually analyses the color space (rather than the name) to see if it truly is sRGB, so perhaps it isn't? Or it is, but not described in some canonical form that OCIO can recognise? |
Aha, adding those aliases fixes it. For
becomes this
and now Gaffer launches. Seems like a pretty useless change to OIIO seeing as how it worked fine for years before this. The "magic" needs some fine tuning! |
I'm now thinking it might be reasonable to file a bug against OIIO. I think this is the code in question : It looks very much like it does some basic classification based on names (which is why it works when you change the name), then uses some heuristics if OCIO < 2.2.0 (which I think is how it worked in Gaffer 1.3 before we updated OCIO), then uses OCIO's heuristics if OCIO is on version 2.3.0 or above (which is isn't in Gaffer). We're using OCIO 2.2.1, which falls through the gap between the two set of heuristics. |
Interesting. I agree, would be good to bring this up with OIIO. I scanned through and noticed this: It seems to look specifically for |
There is some rationale in the commit message here. |
I suspect things might have worked in that commit, and that they might have been broken by this one. Need to do some more digging. In the meantime, it looks like another workaround might be to set the OIIO_DISABLE_OCIO environment variable. |
Scratch that. In fact, that breaks things even worse. |
The TextureLoader came with several problems : - It contained a hack to apply a simple sRGB->linear transform to all `.png` images, but that stopped working with OIIO 2.5 and certain custom OCIO configs. See GafferHQ#5695. - It returned non-const textures, even though they were potentially shared between multiple clients who had loaded the same thing. We now just do our own loading using an LRUCache and direct calls to OIIO.
This fixes GafferHQ#5695, by avoiding the colour conversions done by `IECoreImage::ImageReader` which were failing due to AcademySoftwareFoundation/OpenImageIO#4165. We want to phase `IECoreImage` out completely anyway, so this represents good progress in that direction. Although `_qtPixmapFromImagePrimitive()` and the `Image( ImagePrimitive )` constructor are now completely unused in Gaffer itself, we can't remove them just yet because they are used in extension code at IE. There's a reasonable case for using OpenImageIO here instead of Qt, to get access to a broader range of image formats and to match exactly the capabilities of ImageGadget. I tried implementing that but OpenImageIO isn't really usable from Python without `numpy`, and we don't ship with that module at present. But practically speaking we only really care about PNGs here, and Qt does fine with those, so that's what we're using for now.
This fixes GafferHQ#5695, by avoiding the colour conversions done by `IECoreImage::ImageReader` which were failing due to AcademySoftwareFoundation/OpenImageIO#4165. We want to phase `IECoreImage` out completely anyway, so this represents good progress in that direction. Although `_qtPixmapFromImagePrimitive()` and the `Image( ImagePrimitive )` constructor are now completely unused in Gaffer itself, we can't remove them just yet because they are used in extension code at IE. There's a reasonable case for using OpenImageIO here instead of Qt, to get access to a broader range of image formats and to match exactly the capabilities of ImageGadget. I tried implementing that but OpenImageIO isn't really usable from Python without `numpy`, and we don't ship with that module at present. It would be possible to implement an ImageBuf->QPixmap conversion in C++ and then bind that to Python if we consider this important. But practically speaking I think we only really care about common icon formats here, and Qt does fine with those, so I've gone with the simple option for now.
This fixes GafferHQ#5695, by avoiding the colour conversions done by `IECoreImage::ImageReader` which were failing due to AcademySoftwareFoundation/OpenImageIO#4165. We want to phase `IECoreImage` out completely anyway, so this represents good progress in that direction. Although `_qtPixmapFromImagePrimitive()` and the `Image( ImagePrimitive )` constructor are now completely unused in Gaffer itself, we can't remove them just yet because they are used in extension code at IE. There's a reasonable case for using OpenImageIO here instead of Qt, to get access to a broader range of image formats and to match exactly the capabilities of ImageGadget. I tried implementing that but OpenImageIO isn't really usable from Python without `numpy`, and we don't ship with that module at present. It would be possible to implement an ImageBuf->QPixmap conversion in C++ and then bind that to Python if we consider this important. But practically speaking I think we only really care about common icon formats here, and Qt does fine with those, so I've gone with the simple option for now.
The TextureLoader came with several problems : - It contained a hack to apply a simple sRGB->linear transform to all `.png` images, but that stopped working with OIIO 2.5 and certain custom OCIO configs. See GafferHQ#5695. - It returned non-const textures, even though they were potentially shared between multiple clients who had loaded the same thing. We now just do our own loading using an LRUCache and direct calls to OIIO.
Version: Gaffer 1.4.0.0b1-windows
Third-party tools: Arnold
Third-party modules: None
Description
Congrats on the 1.4 beta release! I tried running it on Windows 10 but it errors out with a few dozen lines in the console about not finding sRGB. I use my own custom OCIO config that is closely based on the official "CG" one. It works fine in Gaffer 1.3.
Here's a portion of the error output:
The text was updated successfully, but these errors were encountered: