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

RoughnessMipmapper: Preserve texture UV transform #21511

Merged
merged 1 commit into from
Mar 25, 2021
Merged

RoughnessMipmapper: Preserve texture UV transform #21511

merged 1 commit into from
Mar 25, 2021

Conversation

zeux
Copy link
Contributor

@zeux zeux commented Mar 25, 2021

RoughnessMipmapper created a new texture and replaced existing material
textures with the copy - however, the copy didn't inherit the UV
transform properties from the source.

This could be problematic when for example the roughness map came from
the glTF file that used KHR_texture_transform extension. When rendered
with standard three.js material this by itself would not be a problem
since three.js only supports one shared transform for most material maps
but when the user overrode the material with a custom one (which
happens inside model-viewer as far as I can tell), the incorrect
offset/repeat values in the cloned texture could result in incorrect
rendering.

This was originally reported in zeux/meshoptimizer#251, cc @Bketting

RoughnessMipmapper created a new texture and replaced existing material
textures with the copy - however, the copy didn't inherit the UV
transform properties from the source.

This could be problematic when for example the roughness map came from
the glTF file that used KHR_texture_transform extension. When rendered
with standard three.js material this by itself would not be a problem
since three.js only supports one shared transform for most material maps
but when the user overrode the material with a custom one (which
happens inside model-viewer as far as I can tell), the incorrect
offset/repeat values in the cloned texture could result in incorrect
rendering.
@mrdoob mrdoob added this to the r127 milestone Mar 25, 2021
@mrdoob mrdoob merged commit 93dd81a into mrdoob:dev Mar 25, 2021
@mrdoob
Copy link
Owner

mrdoob commented Mar 25, 2021

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Mar 25, 2021

/fyi @elalish

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 25, 2021

FYI: RoughnessMipmapper seems still broken on WebGL 2. At least in webgl_loader_gltf using WebGLRenderer(WebGL2) and WebGL1Renderer produce different results. I've debugged this once but had to abort in order to finish other work. A new go on this is on my list though^^.

@mrdoob
Copy link
Owner

mrdoob commented Mar 25, 2021

@Mugen87 No need to look into that. It's actually a bug in Chrome.

/nudge @kenrussell

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 25, 2021

Oh, thanks for the hint. That saved me some time 😅 .

@zeux zeux deleted the rmipxform branch March 25, 2021 17:24
@kenrussell
Copy link

Apologies for the Chromium side bug. Is it this one?

Incomplete texture reported in copyTexImage2D loop in Three.js
http://crbug.com/1120100

@mrdoob
Copy link
Owner

mrdoob commented Mar 29, 2021

I think so!

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.

4 participants