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

Inline tonemapping: None, Linear, Reinhard and Uncharted2 #8250

Merged
merged 21 commits into from
Mar 1, 2016

Conversation

bhouston
Copy link
Contributor

NOTE: This PR builds upon the previous material.premultipledAlpha PR.

This PR adds the general ability for tone mapping within a material fragment shader. The reason for this is that if one is using RGBA8 buffers, which is the only buffer type available if one is not using extensions for float or half, one can not do tone mapping effectively as a post process. Post processing tone mapping is not an option if the resulting values of out a material are outside of the range 0-1 because they will already be clamped. The solution is to tone map in the material before writing to the RGBA8 buffer.

This PR does that. I have added the following parameters:

WebGLRenderer.toneMapping = THREE.NoToneMapping | THREE.LinearToneMapping | THREE.ReinhardToneMapping | THREE.Uncharted2ToneMapping;
WebGLRenderer.toneMappingExposure = <float>
WebGLRenderer.toneMappingWhitePoint = <float>

Uniforms are used for exposure and whitePoint, but only if toneMapping is enabled so changes to exposure/whitePoint are instant and do not require shader recompilation. No uniforms are used if one uses NoToneMapping. There is no if tests in the code, the choice of tone mapping function is done via code injection, thus changing the toneMapping function does require shader recompilation, but I think that is reasonable as generally one does not change the toneMapping function at runtime.

We can add more tone mapping modes as time goes on.

@bhouston
Copy link
Contributor Author

@bhouston
Copy link
Contributor Author

The other motivator for this PR is that post processing is costly in terms of performance, especially if using float/half buffers. This PR allows one to get high quality visual results without relying upon post processing or half/float buffers. Which is a huge win on mobile.

@WestLangley
Copy link
Collaborator

This is good stuff, @bhouston!

As you noted, you are doing tonemapping on a per-fragment basis, rather than as a post-processing step. That means that alpha-blending is being done with tonemapped colors, rather than linear colors, but I think the benefits of using this approach make it worth it.

Furthermore, I believe that for opaque scenes, the approach here is, in fact, correct.

This is going to be of tremendous value if we switch to luminance and SI light units. : - )

@bhouston
Copy link
Contributor Author

@WestLangley You are correct that it isn't a perfect method when it comes to transparency. But in my experience it is close enough, in fact I've never really noticed it being an issue. :) People can still use post processing tone mapping if they are picky about transparency.

Should I give SI light units a try again? I am up for it, but only if a PR in that regards is likely to get accepted.

@WestLangley
Copy link
Collaborator

Should I give SI light units a try again?

@bhouston I did quite a lot of work on switching from radiance units to luminance units last year, and I believe I now understand the units (lumens, lux, or nits) of the intensity parameter for each light type. Making the code changes is not going to be that complicated, but tonemapping will be required, because luminance is unbounded. In fact, we may need a global tone mapping solution for ease-of-use. Switching to SI units would be a major change in user experience, IMO. I am willing to work on it whether it is accepted or not. I feel certain we are going to have to implement it anyway before we get any feedback.

Quite frankly, I have some work to do to catch up with your many recent improvements : - ) , so do you mind if we wait a bit before we revisit SI lighting?

@bhouston
Copy link
Contributor Author

Global tone mapping is easy, just average out the last frame and use it to control the WebGLRenderer.toneMapExposure parameter. You can average out the frame intensity even if the buffer is 8bit per channel, just will take a bit longer to adapt. Thus it is just a pass whose result feeds back into toneMapExposure. :) @MiiBond already wrote that code in his adaptive tone mapping pass, we can just repurpose it.

@WestLangley
Copy link
Collaborator

Global tone mapping is easy, just average out the last frame and use it to control the WebGLRenderer.toneMapExposure parameter

Oh, sweet! It would be awesome to add that feeback loop to your example in this PR, with a GUI toggle to auto-set the exposure.

@bhouston
Copy link
Contributor Author

bhouston commented Mar 1, 2016

@WestLangley wrote:

It would be awesome to add that feeback loop to your example in this PR, with a GUI toggle to auto-set the exposure.

That is a good idea but I would like to keep this PR simple and straightforward, it is easier to get it accepted that way. We can further extend it in future PRs. :)

@WestLangley
Copy link
Collaborator

That is a good idea but I would like to keep this PR simple and straightforward, it is easier to get it accepted that way. We can further extend it in future PRs. :)

Oh yes. That is what I meant.

@MasterJames
Copy link
Contributor

The example link was black on mobile.
Samsung Galaxy Note 3 chrome.

@bhouston
Copy link
Contributor Author

bhouston commented Mar 1, 2016

@MasterJames wrote:

The example link was black on mobile.
Samsung Galaxy Note 3 chrome.

Hmm.... I just tried the example link on a Samsung Galaxy TabPro and a Samsung Galaxy Tab E and it worked. Could it just be slow to load?

mrdoob added a commit that referenced this pull request Mar 1, 2016
Inline tonemapping: None, Linear, Reinhard and Uncharted2
@mrdoob mrdoob merged commit de5c4a9 into mrdoob:dev Mar 1, 2016
@mrdoob
Copy link
Owner

mrdoob commented Mar 1, 2016

Sweet stuff Ben!

@MasterJames
Copy link
Contributor

You are correct! I must have had a server error because I did wait 30 to 60 seconds before making a claim like that to be sure. Next time I won't be so hasty as at a time there seemed to be a rash of that I got in the bad habit. Because the "one code" for everywhere is really important. In rendering I'm sure it will be rather dynamic depending on fps by changing various features and settings. Still devices are getting faster as the demand and need goes up. Today I think the human visual limit is about 8K maybe 16K steteo res.
Honestly I never though WebGL could be so good. Your work is proof to go all in with THREE.js

@MasterJames
Copy link
Contributor

Finally got the https://developer.chrome.com/devtools/docs/remote-debugging going again. Darn drivers. I was going to report what the error was on the mobile (Samsung Note3), when it was really only just a network delivery glitch. Thought you might like to see these while I was at it.
All I get in the console is this handful of warnings now. Looks Perfect.

THREE.WebGLRenderer 75dev
three.min.js:28220 THREE.WebGLRenderer: OES_texture_float_linear extension not supported.
three.min.js:30806 WebGL: INVALID_OPERATION: bindTexture: textures can not be used with multiple targets
three.min.js:28220 THREE.WebGLRenderer: EXT_shader_texture_lod extension not supported.
(5) three.min.js:30806 WebGL: INVALID_OPERATION: bindTexture: textures can not be used with multiple targets

Distance of course effects FPS but the Composer mode is more then twice as fast as Rendered mode but has the minimal aliasing flicker, which is the primary point of it of course. Anyway this particular mobile model gets 15 fps and say 40 fps when in Composer mode for the most part (distance dependent).
The shadow is the same projection conceptually.

@mattdesl
Copy link
Contributor

mattdesl commented Mar 1, 2016

Would love to see this implemented. PRs like these are inching ThreeJS closer to AAA-level rendering approaches. 😄

@mrdoob
Copy link
Owner

mrdoob commented Mar 13, 2016

@bhouston Not sure if it was this PR, but webgl_shaders_tonemapping.html and webgl_terrain_dynamic.html seems to be broken at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants