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

Update GLTFLoader doc for texture transform extension #23417

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

takahirox
Copy link
Collaborator

Description

If I'm right, as Source has been introduced #17766, GLTFLoader texture transform extension no longer needs duplicate texture upload to GPU.

/cc @donmccurdy

As Source has been introduced, texture transform extension
no longer needs duplicate texture upload to GPU.
@donmccurdy
Copy link
Collaborator

We should check that this works as expected, but yes it sounds like this has been fixed! 🎉

@mrdoob
Copy link
Owner

mrdoob commented Feb 3, 2022

I think we are not there yet.

We have to get rid of this code next:
https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLMaterials.js#L176-L287

@takahirox
Copy link
Collaborator Author

takahirox commented Feb 4, 2022

Yes, for perfect texture transform extension support we need to allow uv transform per texture in a material.

But, thanks to Source, at least the following sentence is out dated and I think it can be remove from the doc.

Each use of a texture with a unique transform will result in an additional GPU texture upload.

@mrdoob mrdoob added this to the r138 milestone Feb 4, 2022
@mrdoob mrdoob merged commit c93db53 into mrdoob:dev Feb 4, 2022
@mrdoob
Copy link
Owner

mrdoob commented Feb 4, 2022

Thanks!

@takahirox takahirox deleted the GLTFTextureTransformDoc branch February 4, 2022 23:53
donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
As Source has been introduced, texture transform extension
no longer needs duplicate texture upload to GPU.
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