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

WebGLArrayRenderTarget: Constructor texture options are not respected #28100

Open
gkjohnson opened this issue Apr 9, 2024 · 5 comments
Open
Labels
Milestone

Comments

@gkjohnson
Copy link
Collaborator

Description

When passing texture options into the WebGLArrayRenderTarget constructor they are not retained on the target.texture objects as expected.

Reproduction steps

  1. Create a new WebGLArrayRenderTarget with custom texture options
  2. See that options are not retained

Code

target = new THREE.WebGLArrayRenderTarget( 1, 1, 1, { wrapS: THREE.RepeatWrapping } );
target.texture.wrapS === THREE.RepeatWrapping;

Live example

Run above code in three.js docs console

Screenshots

No response

Version

163

Device

Desktop

Browser

Chrome

OS

MacOS

@gkjohnson gkjohnson added this to the r164 milestone Apr 9, 2024
@gkjohnson gkjohnson added the Bug label Apr 9, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 9, 2024

This is because of the following line:

this.texture = new DataArrayTexture( null, width, height, depth );

Meaning the texture reference is redefined with a new instance. WebGL3DRenderTarget has the same issue.

The solution is using the same approach like in WebGLCubeRenderTarget. Meaning applying the options to the newly created texture.

@gkjohnson
Copy link
Collaborator Author

The solution is using the same approach like in WebGLCubeRenderTarget. Meaning applying the options to the newly created texture.

this.texture = new CubeTexture( images, options.mapping, options.wrapS, options.wrapT, options.magFilter, options.minFilter, options.format, options.type, options.anisotropy, options.colorSpace );

I'm wondering if it would be nice to have a texture.setValues (or other name) function the way Material has so setting these options is a bit more readable.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 11, 2024

I've noticed something confusing. E.g. the default value of the minFilter option in the RenderTarget class is LinearFilter. DataArrayTexture however uses NearestFilter. If we would reuse the same default parameter preset for all type of render targets, we end up with misconfigured internal textures.

WebGL3DRenderTarget and WebGLArrayRenderTarget both rely on data textures so it seems both need a different preset for default options.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 11, 2024

I wonder if we should just redefine the configuration options of both WebGL3DRenderTarget and WebGLArrayRenderTarget. When we added Data3DTexture and DataArrayTexture, we decided to avoid the large ctor signature known from Texture. So if we just decide to decrease the number of properties you can set it options, we wouldn't have to change anything. Devs are then required to do this:

target = new THREE.WebGLArrayRenderTarget( 1, 1, 1 );
target.texture.wrapS === THREE.RepeatWrapping;

instead of

target = new THREE.WebGLArrayRenderTarget( 1, 1, 1, { wrapS: THREE.RepeatWrapping } );

@gkjohnson
Copy link
Collaborator Author

I wonder if we should just redefine the configuration options of both WebGL3DRenderTarget and WebGLArrayRenderTarget

My preference would be to not break a bunch of usages of WebGLRenderTarget for such a minor superficial change.

I've noticed something confusing. E.g. the default value of the minFilter option in the RenderTarget class is LinearFilter. DataArrayTexture however uses NearestFilter.

If we add something like a setValues function it's fairly easy to default these to the expected LinearFilter value:

this.texture = new DataTexture( null, width, height, depth )
this.texture.setValues( {
  minFilter: LinearFilter,
  magFilter: LinearFilter,
  ...options
} );

@mrdoob mrdoob modified the milestones: r164, r165 Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants