Examples: Add webgl_loader_texture_hdrjpg #27183
Conversation
|
From the previous thread:
It looks like there's nothing stopping me from integrating this into Regarding the failed test - how many platforms have you tested this on? Sometimes the CI fails because SwiftShader gets different results (CPU vs GPU rendering). It might be worth poking around a bit to see if your shader has any lines that hit popular driver bugs. |
The failed test is weird, I'm not sure if its caused by I re-ran Now it seems the same "black torus knot" is happening also in the ci env which uses macos AFAIK...but I'm really not sure why that happens, tomorrow I'll test in chrome + macos plus other browsers. I've tested my example in
Tomorrow I'll test in iOS and OSX. The library uses |
|
Yeah, so the difference between running in puppeteer vs normal Chrome is the first uses SwiftShader (CPU rendering), while the second uses your GPU driver. Like all drivers, SwiftShader has its own set of bugs. Do you know what version of WebGL / extensions you're targeting? |
Got it. I'm targeting Webgl2 with EXT_color_buffer_half_float (I believe that's required for half float render targets, which we use) Given the fact that the torus is black, maybe the discrepancy is caused by my usage of a rendertarget as source for PMREMGenerator This usage is maybe not very common? It worked in my tests but I'm not sure. Also (maybe unrelated) there's a line that shouldn't be there in my code. During preliminary tests, Firefox didn't behave correctly when performing |
|
Okay, I did more tests, the example + the decoder in https://gainmap-creator.mono-grid.com (which uses the same library) both run fine on
but I was able to reproduce the "black torus" bug on iOS 17.1 + Safari. The decoding part done by the library seems to work fine because I can see the It seems the part of the example involving EDIT: I tested again in the same iOS 17.1 + Safari hardware and the decoding on https://gainmap-creator.mono-grid.com runs fine, the problem has definitely something to do with the |
…ompatibility problems in some browsers and platforms + adds progress handler for separate gainmap data
|
Alright, it looks like my shader needed to clamp to half float values |
|
some infos found for gainmaps and ImageMagic ImageMagick/ImageMagick#6377 |
|
One last thing... What's the reasoning behind the |
Good point, I'm conflicted on this because a JPEG file with an embedded gain map has not been given a established and commonly used name yet (at least AFAIK) , that only definition I found around is from the libultrahdr code which refers to these files with the naming JPEGR so I assumed they are going to push for that naming in the future, especially considering they are planning on supporting the technology in Android 14, which I, again, assume will be able to read/write/shoot gain map images natively. They also define the whole format as Ultra HDR as opposed to the generic Gain Map from Adobe... sooooo... naming is a bit of an Issue right now 😕 I decided to stick with the gain map terminology in regards to the technology but settled on JPEGR because, simply put, no-one else has given it a different name to it yet 😄 an alternative could be UltraHDRJPEGLoader maybe? but then if google pushes for another naming it could be confusing, I'm not sure yet on the matter, i'm open to suggestions and maybe @elalish can shed light of google's future naming plans (if he knows them or can ask around) EDIT: another point to consider is that gain maps are can be embedded in |
|
I remember @elalish saying that Ultra HDR actually has less range than To me, Ultra HDR sounds like a higher range than what we already had, so to avoid that confusion I thought just calling it HDR JPG was more clear. |
Well... I wasn't quite right. UltraHDR was designed primarily for TV HDR, which has less range than .hdr, but the actual format they settled on (this gain map stuff) is actually generic enough to do a good job on arbitrarily large HDR data. But I refuse to care about naming enough to debate it 😄 |
|
okay then HDRJPGLoader it is, expect an update with the renaming soon |
|
@daniele-pelagatti would you mind double-checking that textures assigned to It might just require changes upstream in |
|
Sure, I'll look into it. I indeed use |
mrdoob
changed the title
|
Okay, a new version of the package has been published with the requested fixes, the example needs to be updated to using version |
|
Done! c15506e |
|
@daniele-pelagatti I've started integrating your library into
|
|
@elalish yes, all you have said for point 1 and 2 is true and I've found is linked to the line you mentioned in three.js during the coding of the example, that is why I added those warnings in the docs. I think it has something to do with how an Equirectangular texture is automatically transformed into a proper CubeMap but I'm not sure why the transformation does not accept a
this is an interesting use case I didn't think about. The reason why the library returns the whole "QuadRenderer" (and not the rendertarget.texture) is because this way the user is able to change one parameter of the hdr reconstruction rendering: maxDisplayBoost (see here and search for "Max display boost") allows to change the HDR "intensity" and is part of the gain map proper specification. By returning the renderer an user is able to do const result = await hdrJpgLoader.loadAsync('image.jpg')
// do something here with the result
// update the gain map data
result.material.maxDisplayBoost = 1 // this would effectively render the sdr representation
result.render()
// do something with the updated result.renderTarget.texture to be honest I'm not entirely sure yet how much that will be useful in the future (especially in the context of three.js) but
I've decided to let the user experiment and decide by themselves what this feature (re-render on-the-fly) can or cannot do for them. Now, to answer your question: I'm not sure how to approach it, How about
let myTexture
const result = await hdrJpgLoader.loadAsync('image.jpg')
if (result.success) {
myTexture = result.renderTarget.texture
} else {
myTexture = result.fallback
}this way they don't waste time re-loading a texture when it does not contain a gain map? let me know if this would solve your use case. EDIT: side note, as a result of my decision to keep the "re-rendering" of the gain map available to the user, you must call |
|
Interesting - I hadn't considered the idea of Possible alternative: what if And you just had the loader cache the last image internally so that repeated calls are efficient? I suppose you'd still need a |
Um, I'm not sure anymore why this check is in place by I think it was PMREM related. Looking at the code now, it should be safe to remove it. The next |
I'm not convinced by this solution because when doing something like: const loader = new HDRJPGLoader();
loader.maxDisplayBoost = 0;
const tex1 = loader.load('01.jpg');
loader.maxDisplayBoost = 16;
const tex2 = loader.load('02.jpg');the loader would use the last value of I feel this would unnecessarily complicate my code in order to manage it and the first solution that comes to mind is doing something like const loader = new HDRJPGLoader();
loader.setMaxDisplayBoost('01.jpg', 0);
const tex1 = loader.load('01.jpg');
loader.setMaxDisplayBoost('02.jpg, 16);
const tex2 = loader.load('02.jpg');and so it would also need something like I think the api I proposed (with the addition of a single proper dispose method) is more straightforward? I'm not sure, let me know
be careful because I've just tried to force a bypass of that line by passing a |
|
nevermind my previous proposals and mindstorming, I've just realized any proposed fallback would eventually fail under this scenario pitfall of my proposalconst loader = new HDRJPGLoader()
const result = loader.load('gainmap.jpeg') // notice the usage of load, not loadAsync
// at this point "result" must be usable somewhere, even if not loaded
let texture: Texture
if (!result.success && result.fallback) { // will not work because the texture is not loaded yet
texture = result.fallback
} else {
texture = result.renderTarget.texture
}pitfall of your proposalconst loader = new HDRJPGLoader()
loader.maxDisplayBoost = 1;
// in the following line, **before** knowing if the texture is a real gainmap or not
// I must return a texture, but it needs to be either a isRenderTargetTexture = true or false
// which i don't know beforehand
const myTexture = loader.load('image.jpg'); also cutting off the ability to re-render the gain map on the fly will lead to the problem of the second scenario, I'm not sure if this is solvable or not at this point, to be honest EDIT: I just realized I can render the base jpeg without adding the gainmap data ( I'll need to do some tests first). I think I'll return same renderTarget as before with the base rendition rendered inside (plus a console warning) in case the gain map is not found. This appears to be the only solution to our use case that keeps the return type of |
|
@elalish I have a solution ready for both your use case and the
here's the documentation for this method
also, side note: from now on maybe it could be better to open issues directly in the gain map repository to avoid polluting this thread further? issue reports are welcome there 😄 |
|
This sounds like a good solution, thanks for thinking through the various ramifications. I'll test this today if you have pushed an npm release and try changing that one-liner in three.js to get rid of |
|
Okay, looks like that one-liner fixed the background problem. I verified it in model-viewer and cleaned up the example to remove |
|
@elalish release 2.0.7 has been published on NPM with those fixes, sorry yesterday was late for me :) |
* initial work for adding gainmap loader example * added tags * better example with file size and resolution comparison * updated example to latest gainmap-js release 2.0.3 which fixes some compatibility problems in some browsers and platforms + adds progress handler for separate gainmap data * should solve test failing problems * updated loader name to HDRJPGLoader * renamed vars for clarity * some more renaming * restored magFilter configuration for HDR
Related issue: #27171
Description
Adds an external example demonstrating the usage of the gainmap-js library with its loaders (see issue for a detailed description of what gain maps are and what they do)