MONOGRID / gainmap-js

A Javascript (TypeScript) Port of Adobe Gainmap Technology for storing HDR Images using an SDR Image + a gainmap
https://monogrid.github.io/gainmap-js/
MIT License
70 stars 5 forks source link

Artifact in converted .hdr #15

Closed elalish closed 7 months ago

elalish commented 7 months ago

When I converted this HDR file to JPG using your tool, I found it created an artifact at the equirect seam:

image

As opposed to the correct version with the original .hdr:

image
daniele-pelagatti commented 7 months ago

Interesting, I cannot reproduce this problem if I convert the HDR you posted and then apply it to the example I submitted to three.js.

I'm trying to determine if the bug is coming from the converter (encoding phase) or the library (decoding phase)

This is my converted JPG file (zipped because you never now if online services like github will strip the gain map metadata or not). ARHusky_09.zip, can you try it in your scene and tell me if it still causes the problem?

If it doesn't cause problems then the problem must be in the encoding phase: can you post your converted JPG and maybe tell me the hardware you used to do the conversion? If it still causes problems then the problem must be on your usage of the decoder (mipmaps maybe?).

Let's see

elalish commented 7 months ago

Thanks, I do see the same problem with your JPG. I tried disabling mipmap generation like your example, but it still repros. Also, why do you disable mipmap generation? That should work fine on a half-float texture, right?

Also, your JPG does repro the issue for me in the three.js example (I'm on a Macbook Pro, OS 14.1.1, using Chrome):

image
daniele-pelagatti commented 7 months ago

ok good to know it's not a fault in the encoding process (more complicated :D ) , tomorrow I'll try to reproduce the problem on your hardware and, if it's mipMap related then #17 should solve it.

daniele-pelagatti commented 7 months ago

Also, why do you disable mipmap generation? That should work fine on a half-float texture, right?

I do not disable it, rather, I suspect it is enabled where it shouldn't be and this seems to cause problems, #17 allows for complete control over when to enable or disable it and disables it by default, we'll see if this is the case, I'll let you know

elalish commented 7 months ago

The plot thickens - this does not repro for me on Safari. My best guess is some kind of undefined behavior in the shader?

elalish commented 7 months ago

Also strange, on Chrome I see the error in the background skybox, but not in the PMREM reflection, so I guess this has something to do with the conversion to a cubemap?

daniele-pelagatti commented 7 months ago

Alright!

The problem did not manifest when using toDataTexture() in order to generate the background image but I was able to reproduce the problem on your hardware + the dev version of threejs which allows to skip the toDataTexture() "hack" in order to use the equirectangular rendertarget as scene background.

I can confirm #17 solves it. I'm not sure exactly why this was happening but the conversion of the renderTarget.texture to cubemap suffers when the renderTarget is created with generateMipmaps = true, this can be seen at the poles with any texture, including spruit sunrise

this is a comparison of v2.07 vs the upcoming v3.0.0 with #17 applied

before

hdrJpg = new HDRJPGLoader( renderer )
    .load( 'textures/gainmap/ARHusky_09.jpg', function ( ) {

        hdrJpgPMREMRenderTarget = pmremGenerator.fromEquirectangular( hdrJpg.renderTarget.texture );
        hdrJpgEquirectangularMap = hdrJpg.renderTarget.texture;
        hdrJpgEquirectangularMap.mapping = THREE.EquirectangularReflectionMapping;
        hdrJpgEquirectangularMap.minFilter = THREE.LinearFilter;
        hdrJpgEquirectangularMap.magFilter = THREE.LinearFilter;
        hdrJpgEquirectangularMap.generateMipmaps = false;
        hdrJpg.dispose( false );

    } );

image

after

hdrJpg = new HDRJPGLoader( renderer )
        // NEW optional method which allows to specify options
        // including generateMipmaps: true if you are going to use the texture
        // (for example) as an HDR lightmap
        // NOTE: the following line is not needed for the example to work 
        // wrapS/wrapT can be left to THREE.ClampToEdgeWrapping and the background still works
        .setRenderTargetOptions( { wrapS: THREE.RepeatWrapping, wrapT: THREE.RepeatWrapping } )
    .load( 'textures/gainmap/ARHusky_09.jpg', function ( ) {

        hdrJpgPMREMRenderTarget = pmremGenerator.fromEquirectangular( hdrJpg.renderTarget.texture );
        hdrJpgEquirectangularMap = hdrJpg.renderTarget.texture;
        // no need to disable generateMipmaps and specify minFilter
                // they both default to  THREE.LinearFilter
        hdrJpgEquirectangularMap.mapping = THREE.EquirectangularReflectionMapping;
        hdrJpg.dispose( false );

    } );

image

daniele-pelagatti commented 7 months ago

3.0.0 should fix this, let me know otherwise and I'll re-open it