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

Setting maps on a material don't copy over the existing colorspace of that texture. #3386

Open
pushmatrix opened this issue Oct 28, 2024 · 4 comments

Comments

@pushmatrix
Copy link

pushmatrix commented Oct 28, 2024

Problem

When setting a roughnessMap on a standard material, the existing colorspace used by that texture is not applied.

Ex:

const roughness = useTexture("roughness.png");

return(
  <>
   <mesh>
        <planeGeometry />
        <meshStandardMaterial roughnessMap={roughness} />
    </mesh>
  </>
)

useTexture will load it as a THREE.NoColorSpace, which is correct for roughness. However once it gets passed to<meshStandardMaterial> it switches to be an srgb texture.

This can be fixed if I specify roughnessMap-colorSpace={NoColorSpace}, but is non-obvious.

I believe the issue is that is copying the texture, but isn't copying over the color space.

Demo

Here I create a THREE.MeshStandard material and avoid using the r3f by setting material property directly on <mesh>. This works correctly
Live Demo

Here I use a <meshStandardProperty> and pass in the roughnessMap. This is incorrect lighting, and you'll notice in the console how the colorspace changes once its applied to the r3f material.
Live Demo

Code: https://codesandbox.io/p/sandbox/3t97t5

Additional Info

Unsure if this also applies to other maps and materials, but I assume it would.

@krispya
Copy link
Member

krispya commented Oct 28, 2024

I can confirm the issue has to do with the prop copy since it works when passed in as an args: https://codesandbox.io/p/sandbox/texture-issue-forked-jtsrcg?file=%2Fsrc%2Fmodel.js

@pushmatrix
Copy link
Author

FWIW, doing a texture copy in threejs seems to preserve it

const roughness = useTexture(
    "roughness.png"
  );
  
  const tex = new Texture();
  tex.copy(roughness);
  
  
 // These are both NoColorSpace
  console.log(roughness.colorSpace, tex.colorSpace);

So is console.log(roughness.clone().colorSpace)

@CodyJasonBennett
Copy link
Member

See #3370 (comment). This was overzealous behavior for color textures that we're now trying to correct with our next major. Three's API for this is not as welcome as it could be.

@pushmatrix
Copy link
Author

Here's an additional demo where I'm not using useTexture. Just a straight up new Texture() that has NoColorSpace as its default colorspace. But as soon as its used as in a prop, it switches to srgb

https://codesandbox.io/p/sandbox/texture-issue-forked-jvfmy8

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

No branches or pull requests

3 participants