search

Autodesk / sitoa

Arnold plugin for Softimage
Apache License 2.0
33 stars 16 forks source link

Fix export of camera screen_window_min/max #43

Closed JenusL closed 5 years ago

JenusL commented 5 years ago

So here's what I've done. First, I put all the code for the screen windows inside the loop that sets other array values like matrix and fov. This means that screen windows are now exported with proper motion blur samples. I also changed the logic. Before it went something like this:

if (subpixelzoom)
   // set screen windows for Subpixel Zoom
else if (orthographic)
   // set screen windows for Orthographic
else
   // set screen windows for Optical Center Shift

That makes each mode exclusive, which means you can't use optical shift or orthographic and subpixel zoom at the same time. So I changed it to this:

if (orthographic)
   // set screen windows for Orthographic
else
   // set screen windows for Optical Center Shift
if (subpixelzoom)
   // offset screen windows for Subpixel Zoom

Now subpixel zoom can be used together with Optical Center Shift and Orthographic cameras.

While testing this, I exported a couple of .asses with and without motion blur and as far as I can see all parameters of the persp_camera is exported properly now. I noticed however that aperture_size is exported with a value of 0 if the camera has a Arnold_Camera_Options on it, even if DoF is disabled. I removed that as well just to clean up a bit.

Passes testsuite and closes #42

sjannuz commented 5 years ago

Not sure about the screen_window_min/max changes. As said, although these parameters are "ready" for motion blur, Arnold should not support them. If, as I understand, only the first value is used, you are now using the first transformation key time for them. I'd rather stick to a single-valued array, computed based on in_frame.

JenusL commented 5 years ago

I just thought that it would be a good idea to export the motion samples so that it would be prepared for motion blur in the future, but I see your point. Would it be ok if I tested the values i get using frame against in_frame, and issue a warning that screen_window_min/max doesn't support motion blur if they are different. And I will use the in_frame value for all array values. That way, it would be really easy to support motion blur again if it ever becomes functional.

JenusL commented 5 years ago

I opted for minimum code change and just replaced all frame with in_frame and added some info about it in the comments. It will still export n-number of motion samples but they will just be the same value. Would this be ok or do you really want a single-valued array?

sjannuz commented 5 years ago

That's ok, I see you correctly commented on it, well done.