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

gltf: support compressed textures #1461

Merged
merged 1 commit into from
Apr 27, 2021
Merged

gltf: support compressed textures #1461

merged 1 commit into from
Apr 27, 2021

Conversation

belom88
Copy link
Collaborator

@belom88 belom88 commented Apr 19, 2021

Background

  • To use PBR Material in SimpleMeshLayer

Change List

  • Support compressed textures in PBR Material

const image = gltfTexture.texture.source.image;
let textureOptions;
let specialTextureParameters = {};
if (image.compressed) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, why do we need this logic? Add a comment?

Is this always needed for compressed textures? If we have a standardized format for compressed textures, can this just be handled in the Texture constructor so that we can pass in the texture and not have to handle this here?

Copy link
Collaborator Author

@belom88 belom88 Apr 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Data structure we pass to Texture2D constructor is different:
  • For ImageBitmap we need do pass {data: image};
  • For compressed texture we already have image with data field {data: Buffer, compressed: true, mipmaps: false, ...};
  1. We add important parameter: TEXTURE_MIN_FILTER . It has different value for compressed textures

@belom88
Copy link
Collaborator Author

belom88 commented Apr 22, 2021

I don't have permission for merging it.

@belom88 belom88 merged commit e8e37a2 into visgl:master Apr 27, 2021
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.

2 participants