EXR: Stop translating "worldToNDC" to "worldtoscreen". #2609
Conversation
|
This looks good, but I am not sold on adding the "fill in the other matrix automatically" idea. It only works seamlessly in some directions and is very confusing in others. For example if the file on disk has worldToNDC (written out from worldtoscreen via an old OIIO). Now a shader that reads "worldToScreen" via the new OIIO will get a shifted matrix. But I think getting nothing back from the new OIIO is more "correct" and less surprising. (And this is the exact unfortunate mess we got ourselves into). Another example, if someone was filling in worldToNDC correctly and directly (not relying on the translation from worldtoscreen) they would suddenly get an extra matrix in their header that they didn't ask for without any way to remove it. |
|
Yeah, I'm on the fence about it. Do you think it's worth having an option to control this? Or just rip it out entirely? |
|
I think its only likely to cause more confusion. I would just rip it out. Files written the old way will fail to read the worldToScreen matrix (as some of our shaders do) - but I rather error out and force those files to be re-generated rather than try to accomodate them automagically with a heuristic that could fail in some other direction. |
This seems wrong. EXR standard metadata "worldToNDC" is exactly what it says, not screen space. I'm not sure how we ended up with the historical accident of reading "worldToNDC" from exr headers and auto-translating it to "worldtoscreen". The hope is that nearly no one is affected by any of this, since the primary use case for these matrices was shadow maps, which were never fully implemented in OIIO's TextureSystem anyway (they were last on the list, and the VFX world switched to ray tracing before anybody got angry enough at my procrastination to force me to implement it). Signed-off-by: Larry Gritz <lg@larrygritz.com>
|
Updated a more minimal patch. No more generation of missing matrices. |
|
I'm going to merge this as it stands now -- simply ending the silent translation of the name. If this causes problems later and people speak up, we can always put in an option that restores the old behavior or tries to supply the missing matrices. |
Oiio 2.2 made a change to how it handles worldtoscreen metadata that makes exr images generated with < 2.2 incompatible with our texture-based cards geometry implementation in usdImaging. For details on the oiio change, see AcademySoftwareFoundation/OpenImageIO#2609. Regenerating the texture asset with oiio >= 2.2 is the "correct" fix. To aid with the transition, however, this change checks for and uses "worldToNDC" as a fallback if it fails to find "worldtoscreen". We issue a warning recommending regenerating the texture asset if "worldToNDC" is present instead of "worldtoscreen". (Internal change: 2291623)
This seems wrong. EXR standard metadata "worldToNDC" is exactly what it
says, not screen space. I'm not sure how we ended up with the historical
accident of reading "worldToNDC" from exr headers and auto-translating
it to "worldtoscreen".
The hope is that nearly no one is affected by any of this, since the
use case for these matrices was shadow maps, which were never
fully implemented in OIIO's TextureSystem anyway (they were last on
the list, and the VFX world switched to ray tracing before anybody got
angry enough at my procrastination to force me to implement it).
Signed-off-by: Larry Gritz lg@larrygritz.com