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

KTX2Loader: Fix regression with 3D textures. #26679

Merged
merged 3 commits into from
Sep 4, 2023
Merged

Conversation

vlucendo
Copy link
Contributor

After the latest changes in KTX2Loader of r156, data 3D textures stopped working. This PR fixes them.

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

Thank you! Would you mind sharing an example of an affected 3D texture, so that we can test to confirm this stays fixed?

Related:

examples/jsm/loaders/KTX2Loader.js Outdated Show resolved Hide resolved
@vlucendo
Copy link
Contributor Author

Yes, here's a 3D texture (size 33 , FloatType) of a LUT compressed with zstd. It has been validated with ktx validate: lut-data3d-zstd.zip

@mrdoob
Copy link
Owner

mrdoob commented Sep 1, 2023

@donmccurdy Should I merge this and release 156.0.1?

@donmccurdy
Copy link
Collaborator

@mrdoob I believe use of KTX2 with DataTexture and Data3DTexture is rare compared to storing compressed textures, so this is not hugely urgent, but if you don't mind making a r0.156.1 patch, yes that would be ideal!

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 4, 2023

In any event, the PR can be merged 😇 .

@Mugen87 Mugen87 merged commit 55783cc into mrdoob:dev Sep 4, 2023
@Mugen87 Mugen87 added this to the r157 milestone Sep 4, 2023
@Mugen87 Mugen87 changed the title KTX2Loader: fix data textures KTX2Loader: Fix regression with 3D textures. Sep 4, 2023
@donmccurdy
Copy link
Collaborator

Given #26691 ... perhaps a patch release would be best after all – thank you! 😇

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 4, 2023

Indeed!^^

@mrdoob
Copy link
Owner

mrdoob commented Sep 5, 2023

Will do!

mrdoob pushed a commit that referenced this pull request Sep 5, 2023
* KTX2Loader: fix data textures

* remove redundant mipmap assignment

* use first mipmap unconditionally
@mrdoob
Copy link
Owner

mrdoob commented Sep 5, 2023

Done! 9840403 3627573 ce0b3d0

@Mugen87 Mugen87 modified the milestones: r157, r156 Sep 5, 2023
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