UltraHDR lighting support #4578
Merged
Conversation
This file contains 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
elalish
commented
Dec 5, 2023
| (resolve, reject) => this.imageLoader.load(url, (result) => { | ||
| const {texture} = result.renderTarget; | ||
| result.dispose(false); | ||
| resolve(texture) |
There was a problem hiding this comment.
Thanks for the fixes @daniele-pelagatti! All seems to be working now. One thing I was curious about: if I change this to result.dispose(true), my textures go black. However as it is, I believe I lose my handle to the renderTarget and can therefore never dispose of it. Not sure that's a huge deal, but I'd like to know what your recommended pattern is here?
There was a problem hiding this comment.
result.dispose(true) calls renderTarget.dispose() too and can be used when , for example, you do
const result = await loader.loadAsync('something.jpeg')
// once pmremgenerator has generated its own texture, result.renderTargetis not used anymore
pmremgenerator.fromEquirectangular(result.renderTarget.texture)
// or
texture.map = result.toDataTexture()
// or
myfile.serialize(result.toDataTexture())
result.dispose(true) // <-----in other words, if you are going to use result.renderTarget.texture in any significant way in the future, don't call result.dispose(true), just call it with false or no params
Merged
JL-Vidinoti
pushed a commit
to vidinoti/model-viewer
that referenced
this pull request
Apr 22, 2024
* UltraHDR working * update an HDR environment * add dispose * fixed texture name bug
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.
This is an exciting one! Custom lighting environments with 10-30x less file size, using UltraHDR (a backward-compatible, HDR version of JPEG) rather than .hdr. This is pulling in a dependency on https://github.com/MONOGRID/gainmap-js (thanks @daniele-pelagatti!), which is a very fast and lightweight decoder that leverages the browser's in-built JPEG decoding that ends up faster than the JS .hdr decoder we use now.
Monogrid has also helpfully made available a simple webpage that will do local conversion of .hdr to UltraHDR .jpg files. This brought Spruit Sunrise down from 1.5MB to 122KB - and the compression is even more impressive with grayscale studio lighting. We're only going to support the single-file .jpg approach for now - separate WebP files gives a bit of additional compression, but it seems too inconvenient to our users.
We put an upstream fix into three.js to make this work better, so I'll wait to merge this until we've updated to the next release.