Make equirectangular reflections / refractions backwards compatible. #19911

mrdoob · 2020-07-22T22:32:09Z

Continued from #19845

This turned out to be much harder than I thought but ended up in a good place.

We no longer break user's code and, in the process, I updated fromEquirectangularTexture() to use renderer.renderBufferDirect() and that way we can generate cubemaps within a render without messing with the state in renderer.render()

This PR also adds EquirectangularReflectionMapping support to scene.background.

I'll leave this is a draft for now as I want to take a second look tomorrow.

…ngley/three.js into dev_equirect_refl_mapping

…onMapping removals.

…ngularTexture().

mrdoob · 2020-07-22T22:39:30Z

Seems like many examples broke (background related). Will fix it tomorrow!

Mugen87 · 2020-07-23T09:22:49Z

src/renderers/webgl/WebGLBackground.js

@@ -25,7 +25,7 @@ function WebGLBackground( renderer, state, objects, premultipliedAlpha ) {

 	function render( renderList, scene, camera, forceClear ) {

-		let background = scene.isScene === true ? scene.background : null;
+		let background = scene.isScene === true ? textures.getCubeTexture( scene.background ) : null;


scene.background can also be an instance of THREE.Color. Besides, an instance of THREE.Texture can represent and normal (flat) background texture. In both cases, getCubeTexture() should not be called.

It seems this bit is responsible for breaking so many examples.

Ah, good catch.

There is still an issue with this setup: https://jsfiddle.net/63cqshof/2/

Meaning you use a flat texture for the background (like a CSS background image). On dev_equirect_refl_mapping nothing is rendered since getCubeTexture() returns null.

Mugen87 · 2020-07-23T09:25:30Z

src/renderers/webgl/WebGLTextures.js

+		} else if ( envMap && envMap.isTexture ) {
+
+			const mapping = envMap.mapping;
+			const isDeprecatedEquirectangular = mapping === EquirectangularReflectionMapping || mapping === EquirectangularRefractionMapping;


Maybe adding a warning when isDeprecatedEquirectangular is true?

Mugen87 · 2020-07-23T15:11:15Z

Just for the record: This PR would fix #9733.

…ping.

…RenderTargetTexture.

Mugen87 · 2020-07-23T16:37:16Z

src/renderers/webgl/WebGLCubeMaps.js

+
+				return texture;
+
+			} else if ( texture.isEquirectangularTexture || isDeprecatedEquirectangular ) {


In the current PR, where are instance of EquirectangularTexture actually created^^?

It seems right now the renderer only looks for EquirectangularReflectionMapping and EquirectangularRefractionMapping in order to determine an equirect texture.

If these constant are considered to be deprecated, how would the user define an equirectangular texture ( I thought this was the purpose of the respective loader)?

In the current PR, where are instance of EquirectangularTexture actually created^^?

No example is using EquirectangularTexture yet. I was waiting to get some consensus in #19845 before investing more.

Understood!

Lets leave the EquirectangularTexture for another PR 😅

mrdoob · 2020-07-24T12:29:01Z

Two broken examples left.

src/renderers/WebGLRenderer.js

# Conflicts: # src/renderers/WebGLCubeRenderTarget.js

…ure.

mrdoob · 2020-07-27T18:03:27Z

Actually, this PR still doesn't fix the distortion in the poles.
I'm hoping it's a matter of temporarily setting texture.minFilter = THREE.LinearFilter to the source texture...

Yep!

Before this PR:

After this PR:

Mugen87 · 2020-07-27T20:08:46Z

Beautiful! 😊

WestLangley · 2020-07-31T01:26:46Z

src/renderers/WebGLCubeRenderTarget.js


-	scene.add( mesh );
+	texture.minFilter = LinearFilter;


I am concerned about this. The texture can be RGBE encoded, in which case linear filtering is inappropriate. See #19716 (comment).

Should I change it to this?

texture.minFilter = texture.encoding === RGBEEncoded ? NearestFilter : LinearFilter;

If linear filtering is appropriate for a given encoding, then the texture loader sets it as linear. There should be no reason to reset it -- at least for HDR/EXR.

PNG files can be RGBM16 encoded. It is the user's responsibility to set these properties in that case, since the loader won't.

Why are you using a filter different from the one the user specifies? Can you explain what you want to achieve here? Disabling mips?

Can you explain what you want to achieve here? Disabling mips?

Yes.

Please do not remove this code unless you can demonstrate a compelling reason to do so.

this.texture.generateMipmaps = texture.generateMipmaps; this.texture.minFilter = texture.minFilter; this.texture.magFilter = texture.magFilter;

Remember, not all equirectangular maps are JPGs.

If you want to create a special case to accommodate the default settings of TextureLoader, that is up to you.

If you want to make a special case, that is up to you. I just can't support doing so since I do not have an explanation as to why mip maps are suddenly not a good thing.

I think it's the same reason we get a seam when mapping a equirectangular texture in a sphere geometry. When I first bumped into this issue I remember asking on Twitter about it and Alex Evans answered this:

So in the case of a Sphere, because the GPU is dealing with 2x2 blocks, it finds a block that goes from U 1.0 to 0.0 so it picks the smallest mip. That results in a seam when the U wraps.

We don't have seams in this case because the shader in fromEquirectangularTexture does not use UVs.

However, when the fragment shader is drawing the poles we're seeing this issue again. In a equirectangular projection the higher or lower the V is the further apart the U samples are and... because of the 2x2 block approach the further apart in the texture the pixels are in the 2x2 block the smallest the mip is, so we end up with blurred poles.

By disabling mipmapping we force the GPU skip that step.

For more research on the subject, I think this is the best query:
https://www.google.com/search?q=graphics+cards+2x2+pixel+blocks

Do you think my conjecture makes sense?

Sorry. I just don't know at this point, unfortunately...

Sorry, not sure I understand.

Do you mean that you don't know if doing this is right?

texture.minFilter = texture.encoding === RGBEEncoded ? NearestFilter : LinearFilter;

Or that I was unsuccessful at explaining the issue and I confused you?

I would write it like this:

if ( texture.minFilter === LinearMipmapLinearFilter ) texture.minFilter = LinearFilter;

I feel uncomfortable about rendering with different settings than the user set, and I have not seen more than one texture that has this problem, so I would be reluctant to modify the library to accommodate it.

However, if you want a special case, then that is fine with me. After all, you may be right.

I would write it like this:

if ( texture.minFilter === LinearMipmapLinearFilter ) texture.minFilter = LinearFilter;

That sounds good to me! 👍

WestLangley · 2020-07-31T01:35:07Z

This looks good. ( edit: except for the changes you made to WebGLCubeRenderTarget.js). Thanks.

Unless I am missing something, I am not seeing support for assigning an equirectangular texture directly to scene.background. Is that saved for later?

mrdoob · 2020-07-31T09:09:00Z

Unless I am missing something, I am not seeing support for assigning an equirectangular texture directly to scene.background

It should work already:

three.js/src/renderers/webgl/WebGLBackground.js

Lines 30 to 34 in f25ccec

    
           if ( background && background.isTexture ) { 
        
           	background = cubemaps.get( background ); 
        
           }

Mugen87 · 2020-07-31T11:14:21Z

It should work already:

Yep, webgl_materials_envmaps actually demonstrates this feature.

WestLangley · 2020-07-31T16:00:03Z

@Mugen87 Just to confirm, in webgl_loader_gltf.html can you please try either/both

scene.background = texture; // equirectangular
scene.environment = texture;

Mugen87 · 2020-07-31T17:40:00Z

Just to confirm, in webgl_loader_gltf.html can you please try either/both

I've just tried a HDR equirect texture and realized that WebGLCubeMaps does not yet handle DataTextures correctly. The following section...

https://github.com/mrdoob/three.js/pull/19911/files#diff-aee47929a0440e6e752f11543dc2aa13R43-R51

...should be something like:

const image = texture.image;

if ( texture.isDataTexture === true || image.complete === true ) {

	const renderTarget = new WebGLCubeRenderTarget( image.height / 2 );
	renderTarget.fromEquirectangularTexture( renderer, texture );
	cubemaps.set( texture, renderTarget );

	return mapTextureMapping( renderTarget.texture, texture.mapping );

}

…emoval.

mrdoob · 2020-08-01T17:22:25Z

@Mugen87 Done! Thanks!

mrdoob · 2020-08-01T17:23:05Z

this.texture.generateMipmaps = texture.generateMipmaps;
this.texture.minFilter = texture.minFilter;
this.texture.magFilter = texture.magFilter;

is also back.

Mugen87 · 2020-08-02T11:39:46Z

src/renderers/webgl/WebGLCubeMaps.js

+
+					const image = texture.image;
+
+					if ( texture.isDataTexture === true || image.complete === true ) {


On last question: Do we have an example where the code uses a compressed equirectangular texture (e.g. S3TC)? Right now, CompressedTexture is not supported by WebGLCubeMaps.get(). How about adding texture.isCompressedTexture === true to the if statement?

Yes, I guess that should be safe to add.
I'll add it. Thanks!

Ended up changing the check to just if ( image.height > 0 ) {.
I double checked that dds textures work.

mrdoob · 2020-08-08T18:29:42Z

@WestLangley Squashed and merged.

WestLangley · 2020-08-10T04:46:26Z

src/renderers/webgl/WebGLCubeMaps.js

+
+					const image = texture.image;
+
+					if ( image.height > 0 ) {


@mrdoob @Mugen87

I think this is a problem... image can be undefined if the texture has not loaded yet.

What should this function return if the texture is not ready?

That indeed produces a runtime error. I'll file a PR.

jbaicoianu · 2022-02-14T06:58:30Z

I know this was merged and closed a while ago so I don't really expect any action here, but just wanted to note that this change caused some issues with my project which I'm not sure how to best resolve.

I updated Three.js versions for JanusXR a few months ago, and some users started noting that their rooms which used equirect video textures for environment maps had stopped working. I only just now got around to debugging that and traced it to this change.

I guess in order to fix this I'm going to have to manually set up a WebGLCubeRenderTarget and duplicate all the work that fromEquirectangularTexture() does for every frame of the video. I can't really use fromEquirectangularTexture() directly because it does a bunch of set-up and tear-down which would be incredibly inefficient for dynamic textures. Definitely less convenient than before.

Kind of hoping someone here might know of a more direct method for using equirect video envmaps which could save me from having to do the heavy lifting here...

mrdoob · 2022-02-17T03:14:35Z

I can't really use fromEquirectangularTexture() directly because it does a bunch of set-up and tear-down which would be incredibly inefficient for dynamic textures.

Do you mind elaborating?

jbaicoianu · 2022-02-17T05:30:05Z

I can't really use fromEquirectangularTexture() directly because it does a bunch of set-up and tear-down which would be incredibly inefficient for dynamic textures.

Do you mind elaborating?

Sure. Each time fromEquirectangularTexture() is called, a number of objects are allocated: BoxGeometry, ShaderMaterial, Mesh, and CubeCamera. These are disposed at the end of each call.

In this particular case, I'm using video textures which could be rendering at 25, 30, or even 60fps, and I'll have to call fromEquirectangularTexture() for each frame of the video to update the cube texture. The most expensive thing here would be the shader compile, but the other allocations would lead to increased garbage collection.

A more optimized implementation would involve allocating those objects once, and then calling camera.update(renderer, mesh) in response to the VideoTexture's requestVideoFrameCallback handler. These objects could then be disposed when the VideoTexture's dispose event is fired.

All of this is of course less optimal than being able to pass the VideoTexture directly in as a uniform to our standard materials like we used to do, but this is a relatively niche use case so it's probably not the end of the world 😀

nosy-b · 2023-08-24T08:35:23Z

Did you find a solution @jbaicoianu ?

jbaicoianu · 2023-08-24T17:14:14Z

Did you find a solution @jbaicoianu ?

Not a cheap one, no. I ended up adding a wrapper around CubeCamera in my engine which then let's you place a reflection probe in your scene, and it does realtime reflections by using that as the envMap on specified objects, for example, here:

https://youtu.be/ZDONF1G1lTE

Way more expensive than just sampling the equirect skybox, but at least it includes all objects that way

WestLangley and others added 7 commits July 15, 2020 21:19

Remove Equirect Reflection/Refraction Mapping

e52d7cb

Merge branch 'dev_equirect_refl_mapping' of https://github.com/WestLa…

895b78f

…ngley/three.js into dev_equirect_refl_mapping

Reverted EquirectangularReflectionMapping and EquirectangularRefracti…

6c5a7f2

…onMapping removals.

Added EquirectangularTexture and EquirectangularTextureLoader.

95b43cd

WebGLCubeRenderTarget: Avoid using renderer.render() in fromEquirecta…

23e0192

…ngularTexture().

WebGLBackground: Support EquirectangularTextures as background.

ddd9a46

Examples: Simplified webgl_materials_envmaps example.

74e4d98

mrdoob added this to the r119 milestone Jul 22, 2020

Fixed EquirectangularTextureLoader.d.ts

d50a5f0

Mugen87 reviewed Jul 23, 2020

View reviewed changes

mrdoob mentioned this pull request Jul 23, 2020

Remove Equirectangular Reflection/Refraction Mapping #19845

Closed

mrdoob changed the title ~~Added EquirectangularTexture and EquirectangularTextureLoader~~ Make equirectangular reflections / refractions backwards compatible. Jul 23, 2020

mrdoob added 4 commits July 23, 2020 16:41

Removed EquirectangularTextureLoader

d6bc2e8

WebGLBackground: Fixed breaking when using Color as background.

88c1757

Examples: Fixed FBXLoader and LWOLoader.

dbd0646

WebGLTextures: Moved cubeTexture code to its own class.

9e8588d

mrdoob added 2 commits July 23, 2020 17:15

WebGLCubeMaps: Handle CubeUVReflectionMapping and CubeUVRefractionMap…

99ca2b3

…ping.

WebGLCubeRenderTarget: Renamed isWebGLCubeTexture hack to isWebGLCube…

36d2384

…RenderTargetTexture.

mrdoob modified the milestones: r119, r120 Jul 23, 2020

Mugen87 reviewed Jul 23, 2020

View reviewed changes

mrdoob added 3 commits July 24, 2020 01:04

Removed EquirectangularTexture

48df1ea

WebGLCubeMaps: Simplified and fixed some breakage.

108a399

Reverted more removals.

1526e83

Mugen87 reviewed Jul 24, 2020

View reviewed changes

src/renderers/WebGLRenderer.js Outdated Show resolved Hide resolved

Merge branch 'dev' into dev_equirect_refl_mapping

f601b75

# Conflicts: # src/renderers/WebGLCubeRenderTarget.js

WebGLCubeRenderTarget: Avoid using mipmaps in fromEquirectangularText…

f25ccec

…ure.

Mugen87 approved these changes Jul 30, 2020

View reviewed changes

WestLangley reviewed Jul 31, 2020

View reviewed changes

mrdoob added 2 commits August 1, 2020 19:18

WebGLCubeRenderTarget: Reverted generateMipmaps/migFilter/magFilter r…

c40361a

…emoval.

WebGLCubeMaps: Added DataTexture support.

271bdb2

Examples: Added more detail to sphere in webgl_materials_envmap.

e638a1d

Mugen87 reviewed Aug 2, 2020

View reviewed changes

mrdoob added 2 commits August 8, 2020 19:13

WebGLCubeRenderTarget: Improved LinearMipmapLinearFilter handling.

8e7bcf2

WebGLCubeMaps: Added support for CompressedTextures.

7024543

mrdoob merged commit e5fa0bc into dev Aug 8, 2020

mrdoob deleted the dev_equirect_refl_mapping branch August 8, 2020 18:30

Mugen87 mentioned this pull request Aug 9, 2020

scene.background support for equirectangular textures #9733

Closed

WestLangley reviewed Aug 10, 2020

View reviewed changes

Mugen87 mentioned this pull request Aug 10, 2020

WebGLCubeMaps: Fix runtime error when image is undefined. #20043

Merged

Mugen87 mentioned this pull request Jan 4, 2024

Using a texture as scene.background is blurrier than it should #27501

Closed


		return texture;

		} else if ( texture.isEquirectangularTexture \|\| isDeprecatedEquirectangular ) {


		const image = texture.image;

		if ( texture.isDataTexture === true \|\| image.complete === true ) {

Make equirectangular reflections / refractions backwards compatible. #19911

Make equirectangular reflections / refractions backwards compatible. #19911

Conversation

mrdoob commented Jul 22, 2020 • edited Loading

mrdoob commented Jul 22, 2020

Mugen87 Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mugen87 Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mugen87 commented Jul 23, 2020 • edited Loading

Mugen87 Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrdoob commented Jul 24, 2020

mrdoob commented Jul 27, 2020

Mugen87 commented Jul 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrdoob Aug 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WestLangley commented Jul 31, 2020 • edited Loading

mrdoob commented Jul 31, 2020

Mugen87 commented Jul 31, 2020

WestLangley commented Jul 31, 2020

Mugen87 commented Jul 31, 2020

mrdoob commented Aug 1, 2020 • edited Loading

mrdoob commented Aug 1, 2020

Mugen87 Aug 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrdoob Aug 8, 2020 • edited Loading

Choose a reason for hiding this comment

mrdoob commented Aug 8, 2020

Choose a reason for hiding this comment

Mugen87 Aug 10, 2020 • edited Loading

Choose a reason for hiding this comment

jbaicoianu commented Feb 14, 2022 • edited Loading

mrdoob commented Feb 17, 2022

jbaicoianu commented Feb 17, 2022

nosy-b commented Aug 24, 2023

jbaicoianu commented Aug 24, 2023

mrdoob commented Jul 22, 2020 •

edited

Loading

Mugen87 Jul 23, 2020 •

edited

Loading

Mugen87 Jul 23, 2020 •

edited

Loading

Mugen87 commented Jul 23, 2020 •

edited

Loading

Mugen87 Jul 23, 2020 •

edited

Loading

mrdoob Aug 2, 2020 •

edited

Loading

WestLangley commented Jul 31, 2020 •

edited

Loading

mrdoob commented Aug 1, 2020 •

edited

Loading

Mugen87 Aug 2, 2020 •

edited

Loading

mrdoob Aug 8, 2020 •

edited

Loading

Mugen87 Aug 10, 2020 •

edited

Loading

jbaicoianu commented Feb 14, 2022 •

edited

Loading