boberfly / GafferCycles

Cycles for Gaffer
BSD 3-Clause "New" or "Revised" License
84 stars 10 forks source link

ccl:film:exposure now affects outputs #32

Closed murraystevenson closed 4 years ago

murraystevenson commented 4 years ago

While testing the shader ball I noticed that the ccl:film:exposure option didn't do anything. Looking at the history it seems it did at one point, but then was changed to a fixed exposure of 0.5. Not sure if that was intentional or not, so here's a quick PR to fix it.

I've changed the plug default to 0.8 to match Cycles' default, though it feels to me that a default of 1.0 would be more appropriate? That's what the Blender UI provides as a default even though Cycles' internal default is 0.8.

https://github.com/boberfly/cycles/blob/89cb9a6f74b28e6f797e38304ffafdbb9b395030/src/blender/addon/properties.py#L424-L429

The default facing ratio shader is also adjusted to match Arnold at an exposure of 1.0, though we could go further and remove the multiply node from the shader if that's to become the new default.

boberfly commented 4 years ago

Hey Murray,

Yeah I forget what this was about but I think I was calibrating intensities to Arnold or Appleseed and settled upon 0.5 as I felt that the viewer in Gaffer should be the one responsible for setting exposure to lessen confusion, but perhaps that assumption wasn't correct and we should set this to 1.0 instead as the default. I think their weighting value on their shaders could've been the one that made me settle upon 0.5 at least at the time, but that option (the one on the shaders) I think should be hidden from the user as Blender doesn't expose it.

0.8 seems like a weird default to me, kind of like their diffuse default is also 0.8.

Unrelated to this, but is there a way to chat with you outside of github? About the ptrace stuff, having issues here debugging on my laptop to test out your patches.

Cheers!

boberfly commented 4 years ago

Going to close this in-favour of https://github.com/boberfly/GafferCycles/commit/14fb552609e69f844a8ab47f3d87cfa8d8ee2635 Cheers Murray!