-
Notifications
You must be signed in to change notification settings - Fork 124
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
IECoreArnold Camera screenWindow #539
IECoreArnold Camera screenWindow #539
Conversation
Thanks for your contribution - it's cool that you're checking out Gaffer! I do believe that we could have a bug somewhere in Cortex and/or Gaffer, but I'm a bit worried that this might not be the right fix. It seems that this PR is a change from the previous behaviour for the cases where So the question is, why are Image Engine's renders working but yours are not? At Image Engine we use cameras that are exported out of Maya using the IECoreMaya::FromMayaCameraConverter, and stored in .cob or .scc files. Are you doing the same, or are you using a different method of getting cameras into Gaffer? |
Looking at how things currently work, I'm assuming that Gaetan is using a Gaffer camera node. This currently does work very oddly - at ImageEngine, we are not affected by this because we almost always use cameras exported from Maya that have their screenWindow baked in. You're correct John that this doesn't look like the right place to fix this. The issue is here: Line 246 in 6fa38ce
The default camera behaviour in Cortex is to fit the resolution to the camera vertically if the aspect is >1, but horizontally if the aspect is <1. The weird screen window values of [-aspect, aspect] are necessary to translate from Gaffer's vertical fit to Arnold's horizontal fit. If we changed this here in CameraAlgo, you would get a horizontal fit in your renders, but the Gaffer viewport would still show a vertical fit, which would be bad. John, I know you have some longer term plans about supporting multiple resolution fit modes, but for the moment, how would you feel about just omitting that if/else on aspect in Camera::addStandardParameters, and always doing a horizontal fit? This would give behaviour that is more consistent, that matches Arnold by default, and is far more likely to be what users like Gaetan expect. Speaking to Andrew, he thinks there wouldn't be to much production impact from this change at IE - the cameras we really care about currently have their screen windows baked in, and would be unaffected by changing how we compute a default screenWindow. |
Thanks for your insights @danieldresser. If production won't be affected and it's a quick fix, then that sounds good to me. I would like to take a look at one of these problematic cameras first though, so we can be sure we're fixing the original problem - any chance you could provide something @sol-ansano-kim? |
I've made a new issue (#584) to modernize our camera spec. This should happen soon, as part of Cortex 10, so I'll close this PR for now. If you find the time to provide the original problematic scene, I'm sure that would be helpful. |
I apologize for the late reply. The new camera spec will be really helpful for us. Thanks, |
Hello,
We are in the process of evaluating gaffer for our lighting team.
We mainly work with Arnold and we couldn't get gaffer renders to match maya's one.
Looking into details, it seems the values of the screen window on the arnold camera node do not match.
By default, arnold uses normalized values on both axis, [-1, 1] and [-1, 1] and that's what we get out of maya.
Gaffer on its side is outputting [-aspect, aspect] on both axis when the aspect is above 1.0. Below 1.0, gaffer does output the expected [-1, 1]
In the code, it seems that IECore compute values so that only one of the two axis is normalize while the other uses [-1/aspect, 1/aspect]. Which axis is set to [-1, 1]
depends on the aspect value being below or above zero.
Then, further on, it seems that IECoreArnold is trying to modify those values so that both axis are normalized to match arnold's behavior.
The problem is that it systemically tries to re-normalize the same axis, independently of the aspect value.
We don't know if it is the intended behavior or it this is a bug that just went through unnoticed.
Here is our fix just in case.
Thanks,
Gaetan Guidet, Sol Kim