Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGLTextures: Fix warning when using 3D Textures. #24753

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

clemenssielaff
Copy link
Contributor

Hi all,

I am working on a 3D volumetric renderer and came across a warning today that I thought was an error. After fixing it, I realized that the reason why I wasn't seeing anything was in my code, not in three.js, but my solution contained a one-line fix for a real, albeit inconsequential, issue with this great library that I thought I'd might as well share.

Description

Previously, calling WebGLRenderer.setRenderTarget(texture3D, layer) would cause a warning framebufferTexture2D: Bad 'imageTarget': 0x806f (where 0x806f translates to TEXTURE_3D).

This fix simply checks whether the textureTarget argument is valid or not before calling framebufferTexture2D. If it is not valid, the call is skipped - which is the same behavior that we have right now minus the annoying warning.

Tests and linting succeeds 👍

Let me know if you need any more info or whatnot.

Cheers,

  • Clemens

Previously, calling `WebGLRenderer.setRenderTarget(texture3D, layer)` would cause a warning `framebufferTexture2D: Bad 'imageTarget': 0x806f` (where `0x806f` translates to `TEXTURE_3D`). The warning is inconsequential but annoying.
@mrdoob
Copy link
Owner

mrdoob commented Oct 7, 2022

Thanks for the check 🙏

Although the logic seems little bit hard to follow to me. @Mugen87 what do you think?

@mrdoob mrdoob added this to the r146 milestone Oct 7, 2022
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2022

I'm afraid I need a live example to better understand when the warning occurs.

@clemenssielaff
Copy link
Contributor Author

Haha, yes 😄
I will write one up. Just a sec.

But it might help to look up a few lines before, at line 1254, there is already a check whether or not the texture target is a 3D texture or a 2D texture array. If it is possible that the texture target is a 3d/2d array texture up there, that must still be the case a few lines down where textureTarget will be passed to framebufferTexture2D ... which will cause the warning.

@mrdoob
Copy link
Owner

mrdoob commented Oct 7, 2022

Yes, but I think it's more readable to check if something "is" something instead of checking if something "isn't" 🤓

@clemenssielaff
Copy link
Contributor Author

Here is a tiny svelte component to reproduce the warning:

<script lang="ts">
	import { onMount } from 'svelte';
	import { WebGLRenderer, WebGL3DRenderTarget } from 'three';

	onMount(() => {
		// create a renderer
		let renderer = new WebGLRenderer();

		// put it in the DOM
		document.querySelector('#scene-container')?.append(renderer.domElement);

		// create a 3d render target with arbitrary dimensions
		const volumeRenderTarget = new WebGL3DRenderTarget(32, 32, 32);
		
		// set the render target to the first slice of the volume
		renderer.setRenderTarget(volumeRenderTarget, 0); // <-- warning here
	});
</script>

<div id="scene-container" />

The call stack to the warning is as follows:

  1. WebGLRenderer::setRenderTarget
  2. WebGLTextures::setupRenderTarget
  3. WebGLTextures::setupFrameBufferTexture

it's more readable to check if something "is" something instead of checking if something "isn't

Agreed ... in general.
framebufferTexture2D allows 7 enum values: TEXTURE_2D + 6x TEXTURE_CUBE_MAP_* and checking for two that I know are wrong felt less invasive than checking for 7 right ones...
Then again maybe cube maps will never go here (I don't know all calling code) and it is enough to check for TEXTURE_2D only?
IDK. Your call 😄

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2022

Live example: https://jsfiddle.net/vmgud9x4/

When debugging the code, the line in question does not report a warning. Do I miss something in the example?

@clemenssielaff
Copy link
Contributor Author

clemenssielaff commented Oct 7, 2022

Interesting. No, the live example is the same. And I do see the warning in the Developer Tools log when using Firefox.
Not when using Chrome though. 🤔 What browser do you use?

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2022

Chrome. When using Firefox, I can reproduce the warning, too.

WebGL warning: framebufferTexture2D: Bad imageTarget: 0x806f

Interesting to see this is browser dependent 🤔 .

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2022

It seems nothing breaks if the framebufferTexture command is removed for 3D render targets and array render targets. Firefox is probably more restrictive in reporting.

Then again maybe cube maps will never go here (I don't know all calling code) and it is enough to check for TEXTURE_2D only?

Nope, it would also be necessary to test for all TEXTURE_CUBE_MAP_* constants.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2022

Anyway, let's update the PR respectively. At the end, this explicit style is less confusing.

to check for all valid texture targets explicitly
@clemenssielaff
Copy link
Contributor Author

Done 👍

@Mugen87 Mugen87 changed the title Fixed warning when using 3D Textures WebGLTextures: Fix warning when using 3D Textures. Oct 7, 2022
@Mugen87 Mugen87 merged commit eec492d into mrdoob:dev Oct 10, 2022
@clemenssielaff clemenssielaff deleted the fix-warning-with-3d-texture branch October 10, 2022 14:13
@mrdoob
Copy link
Owner

mrdoob commented Oct 11, 2022

Thanks! Way easier to read 🙏

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.

3 participants