Color management: linear workflow #808
Conversation
garyo
force-pushed
the
feature/color-management
branch
from
4e8be16
to
b6b6ebe
Compare
|
@fredludlow any thoughts on this? I'd like to get it merged at some point. |
|
Sorry - I made a few false starts at looking at it earlier. The transparent surface example you put in #775 looks great, I'm slightly concerned that we lose a bit of the 'depth' effect, e.g. here (managed): vs original: I find it harder to perceive 3D structure from a static image in the linear color managed version - I guess because the darkening of the curved surfaces is less extreme they don't seem to pop out of the image as much - is this by design, or something to do with the loss of precision? (Or might it be fixed/compensated for by updating the lighting model - or maybe it's just what I'm used to and I'd adapt if I tried it for a few weeks...?) |
|
ps - Built versions here (this PR: https://fredludlow.com/ngl/pr-808/examples/webapp.html?script=color/atomindex ) and current master https://fredludlow.com/ngl/master/examples/webapp.html?script=color/atomindex ) in case anyone wants to see side-by-side. If you stick an 'av' surface at ~70% opacity the linear one looks significantly nicer! |
|
Hi Fred; thanks so much for putting up these builds for comparison! You're right that in the lighter colors there's somewhat less contrast and there's less over-darkening around the edges. It's more physically accurate but you're also right that sometimes inaccurate can look better... Currently ambient light defaults to 0.2; I think that's partly to compensate for the fact that the mids and darks were too dark otherwise. Perhaps along with this PR I should default ambient lower, like 0.05 (or even off altogether). That gets back some of the contrast. Also with a linear workflow that makes more phyical sense. What do you think? Adding fog can help too (fog should be more accurate now). Try lowering the Fog Near param. Also adding a bit of metalness always looks good; that should bring back some of the darkening around the edges due to the Fresnel effect. But I don't know that we want to change those defaults; they'll be kind of specific to the model. |
|
I've asked a few of my users, and one came up with a view sort of like this (similar to the images above but more typical of what they'd look at day-to-day): Linear Color (with ambientIntensity: 0) Original Version Their view was the linear color version looks 'flatter' and a bit more cartoony, which I think is what I was trying to get across above. This is more of an issue if you're preparing images for publication (when you're working 'live' you adjust the rotation constantly to help maintain your 3D mental model). In your original post you proposed having the option to switch between modes, which would be my preference if it's doable. You said you weren't sure how to thread the value through - I'm not sure I have any great ideas, but is it something you'd want to allow different settings on different representations/buffers or is it a global setting? (I'm guessing global as you change renderer.outputEncoding?) |
|
Thanks Fred -- user feedback is invaluable for this. I'll work on creating an option. Yes, it has to be global -- it affects the colormakers as well. |
Set the renderer output to the recommended sRGB encoding, and linearize all object colors so the internal three.js workflow is fully linear. This also allows correct glTF export of models. This is WIP because it doesn't propagate the color management settings from the top-level viewer down to the colormaker yet.
This allows use with the three.js EffectComposer, which needs NGL to render into a non-null renderTarget. Should have no impact on normal use.
This makes the color space for colormaker global, since it doesn't really work to have different color spaces for each colormaker. That makes it possible to easily set the color space (encoding) from the viewer, just as we set the output color encoding. This commit resets the default back to sRGB internal workflow (no conversion of colormaker colors to linear, no color conversion back to sRGB on output) to preserve existing behavior, per comments in nglviewer#808. To set up a linear workflow, just do: ```ts stage.viewer.setColorEncoding(Three.LinearEncoding) stage.viewer.setOutputEncoding(Three.sRGBEncoding) ```
garyo
force-pushed
the
feature/color-management
branch
from
b6b6ebe
to
3652a34
Compare
|
Added a viewer function |
|
Apologies once again for the big time-lag... I'm looking at this now and wondering if you can help me understand it a bit... In the master branch, With a fresh build from your branch the outputEncoding is set to 3001 - I can recreate the original behaviour with So, to summarize: The old way is working in sRGB internally with no transformation upon output, the properly managed way is to work with linear colors internally and transform to sRGB on output? If I'm right (big if :) ), is there any reason why you'd ever want to If I've got this all wrong please let me know! (Happy to have a call if it's easier than my long-delayed responses..) |
|
Thanks for the review, @fredludlow . Yes, you're exactly right in your summary. My intent with the last commit 3652a34 was to set the default behavior back to the traditional way (sRGB colors internally, compositing in sRGB, no output conversion), but looks like perhaps I didn't get it all the way done. |
Just one new API function: `viewer.setColorWorkflow(encoding)`
|
Fixed defaults, and tested default behavior vs. master and it looks the same. |
|
Thank you for the fix @garyo, now merged! |
Here's my color-management code -- linear workflow internally, turned on by default.
I've been using it for a while and it works well for me. There are two issues to discuss, I think:
How to allow users to specify the color encoding, if they want to go back to the default? I've added a
colorEncodingparam to the viewer, which seems like a good place for it, but I don't have a good idea on how to thread it through to colormaker.ts where it's needed. So right now that param does nothing -- in order to get back to the old workflow you have to changecolorSpacein colormaker.ts. Ideas appreciated.@arose commented in Color management in NGL #775 that it might be better to implement color management in the shader; I'd like to propose that it's at least OK if not better to do it this way because the values (vertex colors, param values, and textures) are explicitly linear, which works properly e.g. when exporting models as glTF. The only real issue is that in colormaker.ts, it's still creating 8-bit linear color values because it uses
.toHex. Ideally it should produce float rgb triples and use those everywhere, but that's a bigger change than I wanted to get into for this PR. It does mean you lose some precision when specifying very dark colors, but I don't think that's so bad for now; moving to full-float colors would be a good next step.