Conversation
Contributor
Author
|
My PRs for WebGLBackground texture encoding (mrdoob/three.js#15299), inverted reflection PMREMs for WebGLRenderTargetCubes (mrdoob/three.js#15302), and the PMREM/EquirectangularToCubeGenerator memory leak (mrdoob/three.js#15330) have been merged upstream which will make this PR much easier, as well as remove some of the work arounds in place for the memory leak 🎉 |
dd7dd56 to
23ee9aa
Compare
…nvironment maps (fixes #65)
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #65, #196, #215, enables #200
Supercedes #133
I had PMREM+HDR working in #133 again from master, but the envmap was off with the skybox again, so figured lets get this working on three r99dev that added
scene.backgroundsupport for cubemaps (mrdoob/three.js#15187, mrdoob/three.js#15186, mrdoob/three.js#15081) so that we can have a good baseline for judging reflectivity correctness rather than keep flipping the skysphere.I have PMREM+HDR with
scene.backgroundhandling the sky rather than a skysphere mostly working, but there were some issues with cubemap encoding changes when used inscene.background(mrdoob/three.js#15325), and the flipping of the reflection found in WebGLRenderTargetCubes (mrdoob/three.js#15301) mentioned earlier. PR out for encoding changes (mrdoob/three.js#15299), and I have a hacky way of fixing the reflection, but my in progress PR is out (mrdoob/three.js#15302).At this point, can the HDR+PMREM work be decoupled from the
scene.backgroundchange? We could land just the HDR+PMREM changes, and flip the skysphere again, but it'll be inverted. We can't just land thescene.backgroundchange and remove the skysphere because we can't reuse the same cubemap from the envmap inscene.background(or else we getINVALID_OPERATION: bindTexture: textures can not be used with multiple targets), and the rational why we have an equirect for the skysphere (although we could deep clone the texture?). Adding PMREMs means we can use cubemaps for both the skysphere and envmap since we have 2 cubemaps per environment now, and can discard the equirects.There are still a few more caveats for the PMREM feature. The memory leak issue worked around in #227 is also in three's PMREM generators -- we would either have to do the same messy workaround, or can try to fix it upstream in mrdoob/three.js#15288. The leak FWIW would only occur when swapping out background images continuously for an hour, so not a cause for alarm (edit: using HDR images exacerbate the leak; I don't think the texture is being duplicated, but maybe more resources are created). iPhone 6 also has an issue with the PMREM implementation (mrdoob/three.js#15005).
Not quite ready for review yet, still some tests failing, and textures sometimes bleed into other elements (more alarming). These changes of course may not always look great out of the box but will enable the ability to add configuration. If missing my three.js patches, the first environment map encoding used is applied globally to all skyboxes, and the environment map projected on the model will be inversed. Almost there, wanted to document progress so far here.
Some pictures! (More pictures in #133)
LDR radiance vs HDR radiance: (maybe the LDR image should be boosted/mapped or PMREM should not be used for LDR)
Inverted env map
PMREM at different sampling levels (top/left clockwise 8, 16, 24, 32), lower samples more noticeable on smooth meshes.