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

Faster cube uv envmaps #8284

Merged
merged 9 commits into from
Mar 7, 2016
Merged

Faster cube uv envmaps #8284

merged 9 commits into from
Mar 7, 2016

Conversation

bhouston
Copy link
Contributor

@bhouston bhouston commented Mar 3, 2016

This PR adds a flag, DISABLE_CUBE_UV_MIPMAP_INTERPOLATION, into the cube_uv_reflection_fragment.glsl that disables mipmap interpolation. This halves the complexity of this part shader in terms of code as well as number of texel fetches while keeping quality roughly the same -- I can not tell the difference in the current examples.

We have had reports over the last few days that the cube_uv_reflection_fragment shader was too intense for some older MBPs who relied on Intel GPUs to drive retina displays - thx @mattdesl, and @wvl. This should help with that and other lower end devices driving high resolution displays.

This easy fix was suggested by @spidersharma03.

@mattdesl
Copy link
Contributor

mattdesl commented Mar 3, 2016

Awesome! 👍

@bhouston
Copy link
Contributor Author

bhouston commented Mar 3, 2016

@mattdesl I've done a bit more optimization in the last couple minutes. Is there a way you can test it on your MBP retina?

Old slower version: http://ci.threejs.org/api/pullrequests/8278/examples/webgl_materials_envmaps_hdr.html

New faster (?) version:
http://ci.threejs.org/api/pullrequests/8284/examples/webgl_materials_envmaps_hdr.html

You might have to zoom the torus to get enough pixel coverage on screen.

@spidersharma03
Copy link
Contributor

Great optimizations @bhouston !

@mattdesl
Copy link
Contributor

mattdesl commented Mar 3, 2016

How would you turn this on / off in code?

@mattdesl
Copy link
Contributor

mattdesl commented Mar 3, 2016

I am not fill-rate bound in that scene even on full screen so I am not dipping below 60FPS. But in my own scene with god rays etc it becomes quickly fill rate bound when zooming into a mesh with the old cubeUV sampling. I will checkout this PR locally and test the optimizations. 😄

@bhouston
Copy link
Contributor Author

bhouston commented Mar 3, 2016

How would you turn this on / off in code?

Since I haven't seen a visual quality difference yet, there is no way to turn off the opimizations I am doing.

@mattdesl
Copy link
Contributor

mattdesl commented Mar 3, 2016

@bhouston
Seamless cubemaps just landed in FF Nightly across OSX and Windows, and they've enabled it in a way that it works in WebGL1 as well. From what I understand Chrome and Edge also intend to match this behaviour. This might be good news for the future of MeshStandardMaterial. 😄

In an ideal world we would have a reliable way to query/detect seamless cubemap support – I've opened some discussion here.

@bhouston
Copy link
Contributor Author

bhouston commented Mar 3, 2016

We should still generate them using the pmrem but then load them into a
cubemap for fast access. I speak from experience here. Clara.io's private
branch of threejs has been creating roughness cube lod maps for over a
year. They are visibly poor quality at the lowest lod levels. So much so
that even on Windows we used pre-generated irradiance maps to get
sufficient quality. Thus we should use manual pmrem generation and also do
not use the bottom smallest 2 or 3 cube maps as we do in cubeuv because
there just isn't the necessary pixels for a smooth result. Also we can not
use autogen of seamless cubemap with encoded textures.

Best regards,
Ben Houston
http://Clara.io Online 3d modeling and rendering
On Mar 3, 2016 5:04 PM, "Matt DesLauriers" notifications@github.com wrote:

@bhouston https://github.com/bhouston
Seamless cubemaps just landed in FF Nightly across OSX and Windows, and
they've enabled it in a way that it works in WebGL1 as well. From what I
understand Chrome and Edge also intend to match this behaviour. This might
be good news for the future of MeshStandardMaterial. [image: 😄]

In an ideal world we would have a reliable way to query/detect seamless
cubemap support – I've opened some discussion here
KhronosGroup/WebGL#1528.


Reply to this email directly or view it on GitHub
#8284 (comment).

@@ -1,5 +1,9 @@
#define DISABLE_CUBE_UV_MIPMAP_INTERPOLATION
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is temporal/testing code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was doing interpolation between different levels of roughness (based on material roughness) as well as different mipmap levels (based on texel derivatives). I have turned of interpolation between mipmap levels for now as I didn't see it having a huge quality impact (although we haven't tested this on complex models yet, so I worry that maybe it has an impact in complex cases, which is why I have temporarily kept it) and it did have a performance impact, at least as reported by @MattDes.

A faster way to do this would be to load the calculated roughness maps into a cubemap mipmap structure -- that option is only available in WebGL 2.0 or if the textureCubeLodEXT extension is available (e.g EXT_shader_texture_lod which has 70% availability.) I think that is the next optimization -- optionally load into cubemap mipmaps when possible. It would cut out all of this code at run time, but we would still fall back to it when the EXT_shader_texture_lod isn't available.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can remove the commented code by now and restore from a previous state if needed. Just want to keep the code tidy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the code in the latest commit per your recommendation.

mrdoob added a commit that referenced this pull request Mar 7, 2016
@mrdoob mrdoob merged commit e8eee06 into mrdoob:dev Mar 7, 2016
@mrdoob
Copy link
Owner

mrdoob commented Mar 7, 2016

Thanks!

@bhouston bhouston deleted the faster_cube_uv_envmaps branch March 14, 2016 13:45
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.

4 participants