Skip to content
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

HDR image offset #4

Closed
varomix opened this issue Aug 19, 2020 · 9 comments · Fixed by #49
Closed

HDR image offset #4

varomix opened this issue Aug 19, 2020 · 9 comments · Fixed by #49
Labels
in discussion Still requires a discussion

Comments

@varomix
Copy link

varomix commented Aug 19, 2020

Here comes the bug reports 🐛
Loading an hdri in the dome light is offset by -90 degrees in the X axis

image

@Vochsel
Copy link
Contributor

Vochsel commented Aug 20, 2020

This stems from Cycles coordinate system being based around Blender which is Z up compared to Houdini which is Y up. Our use cases is to treat Z up as the ground truth when rendering USD data.

As it stands currently, our Z up Blender exports (with dome lights) are rendered correctly.

We have found no elegant solution to handle this situation as the stage up axis is not respected in Houdini properly (Treated more as a suggestion).

A compromise might be to have dome lights treated as Z up, enabled via environment variable. Whether or not it is the default is another question. I lean towards keeping it disabled by default and treating Houdini and other Y up DCCs as the exception.

Technically this issue is larger than dome lights. I suspect coordinate based materials in general will be the wrong axis when authored in Houdini.

@trebconnell
Copy link
Contributor

I repro'd this in usdview with a Y-Up usda (below for reference).

The issue in usdview seems to be that the changing upAxis results in a different camera transform being given to hdCycles. This works for mesh. But causes issues for Skydome, which should have the inverse transform applied so they're 'Always Up'.

I'll see if I can determine how/if Storm and hdPrman handle this. Is there a way to get Storm to visualize HDRI?

#usda 1.0 
( upAxis = "Y" )
def DomeLight "domeLight"
{
    asset texture:file = @StinsonBeach.exr@
    token texture:format = "latlong"
}

@varomix
Copy link
Author

varomix commented Aug 20, 2020

yeah this is a tricky situation, had to hardcode transforms when I was trying to integrate Cycles into Houdini a while ago

@trebconnell
Copy link
Contributor

Hmm this issue has been brought to Renderman: PixarAnimationStudios/OpenUSD#938

The 'fix' seems to be that stages with z-up should apply a transform to their HDRI to accommodate. So Blender would call the function added here when it exports a dome light.
PixarAnimationStudios/OpenUSD@d26a8e4

Imo would be preferable to have Hydra adjust the transform of dome lights, like it does the camera... I will raise that on Usd itself and see what Pixar's thoughts are on this,

@trebconnell
Copy link
Contributor

Raised in PixarAnimationStudios/OpenUSD#1298

@Vochsel , We'll see if that issue gets any traction. Currently it seems that blender should adjust the transform of it's dome lights on export, but idk if you want to actually start baking that into your scenes...
I'll see if I can detect what the scene up is and modify the transform myself to mitigate....

@trebconnell
Copy link
Contributor

I can't detect automatically in render delegate... This information is not exposed. Could do via environment variable as suggested...

@Vochsel
Copy link
Contributor

Vochsel commented Aug 20, 2020

Thanks @trebconnell
Yep that was basically the conclusion we came to here. Will wait to hear from Pixar.

Going with the OpenEXR spec for environment maps does makes some sense.

Our previous versions of Blender were on USD-19.11 so they didn't have that function, but we have recently moved to 20.08 so we can make use of that. I think if we can make the default usage of HdCycles match 1 to 1 with what cycles expects then its preferable at the moment.

@bfloch bfloch added the in discussion Still requires a discussion label Aug 21, 2020
@c1112
Copy link

c1112 commented Sep 1, 2020

this may be a stupid question but is there any particular reason to not just switch Houdini to z up in the prefs?

@boberfly
Copy link
Contributor

#49 fixed here as well

@Vochsel Vochsel linked a pull request Jan 25, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in discussion Still requires a discussion
Projects
None yet
6 participants