Skip to content

Conversation

@Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Oct 10, 2019

...and use WebGLRenderTargetCube.fromEquirectangularTexture() instead.

exrBackground = cubemapGenerator.renderTarget;
var cubeMapTexture = cubemapGenerator.update( renderer );
var options = {
minFilter: THREE.NearestFilter
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that makes sense. Here, or in the lines above. Probably should be Linear.

You will have to step through the code to see if it matters.

Also, you will almost surely have to change the loader output data type to HalfFloat to duplicate the previous results.

.setDataType( THREE.HalfFloatType )

Copy link
Collaborator Author

@Mugen87 Mugen87 Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that makes sense. Here, or in the lines above. Probably should be Linear.

Why do you think are the previous texture filtering settings wrong? It does actually look good.

Also, you will almost surely have to change the loader output data type to HalfFloat to duplicate the previous results.

That's true. Let me fix this!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your example is throwing errors on my machine.

I have always used Linear interpolation when the encoding supports it. Some encodings like RGBE are not compatible with linear, so nearest must be used. I have never seen Linear and Nearest mixed in the same texture. If there is a use case for that, I am not aware of it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your example is throwing errors on my machine.

Um, can't reproduce this on my iMac with Chrome. What errors are logged?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failure is on macOS Safari.

WebGL: INVALID_OPERATION: texImage2D: type HALF_FLOAT_OES but ArrayBufferView is not NULL

Copy link
Collaborator Author

@Mugen87 Mugen87 Oct 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I can reproduce with Safari. It seems WebGLRenderTargetCube.fromEquirectangularTexture() throws this error when the input texture is of THREE.HalfFloatType. Using THREE.FloatType with EXRLoader as before solves the issue. However, since WebGLRenderTargetCube.fromEquirectangularTexture() strictly derives the type of the source texture (unlike EquirectangularToCubeGenerator which does type: options.type || this.sourceTexture.type), I can't quickly fix this issue without changing previous values (see #17708 (comment)).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #17727

@WestLangley
Copy link
Collaborator

@Mugen87

  1. I forked your branch and tested webgl_loader_gltf_extensions.html, and it appears to be OK.

  2. webgl_materials_envmaps_exr.html has the issues I mentioned above.

  3. I did not study webgl_loader_gltf.html. You can do that one. ;-)


As I did, you need to make sure encoding, generateMipmaps, magFilter, minFilter, and type are unchanged from the previous examples -- for both the background and the environment map. If the settings are unchanged, but do not make sense, then this is an opportunity to fix them.

As I said above, PMREM changes the input texture, so you need to check the background texture after PMREM is finished.


Personally, I think setting a background cube map with mipmapping disabled is probably not what we want to be recommending, but I guess that can be a separate issue.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 11, 2019

I've checked all examples again and made sure the properties of the background's texture are the same as before. Let me list those values in the following table:

Background:

Example encoding generateMipmaps minFilter magFilter type
gltf RGBEEncoding false NearestFilter NearestFilter UnsignedByteType
gltf_ext RGBEEncoding false NearestFilter NearestFilter UnsignedByteType
exr (exr) LinearEncoding false LinearFilter LinearFilter HalfFloatType
exr (png) sRGBEncoding true LinearFilter LinearFilter UnsignedByteType

EnvMap:

Example encoding generateMipmaps minFilter magFilter type
gltf RGBM16Encoding false LinearFilter LinearFilter UnsignedByteType
gltf_ext RGBM16Encoding false LinearFilter LinearFilter UnsignedByteType
exr (exr) LinearEncoding false LinearFilter LinearFilter HalfFloatType
exr (png) sRGBEncoding true LinearFilter LinearFilter UnsignedByteType

Measured after PMREM has finished.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 11, 2019

If the settings are unchanged, but do not make sense, then this is an opportunity to fix them.

The PNG background/env maps in the EXR example have set generateMipmaps to true although only LinearFilter is used. I think this can be changed, right?

@Mugen87 Mugen87 added this to the r110 milestone Oct 13, 2019
@WestLangley
Copy link
Collaborator

Right. Fix it if you can. Otherwise, wait until the PMREM rewrite is finalized.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 14, 2019

Merging for now. The settings in webgl_materials_envmaps_exr can be optimized with a different PR.

Sidenote: Let's not forget webgl_materials_envmaps_exr does not work in FF (see #16778).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants