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

fix the bug of ktx2's texture minfilter #9876

Merged
merged 4 commits into from
Nov 9, 2021
Merged

Conversation

vtxf
Copy link
Contributor

@vtxf vtxf commented Oct 15, 2021

It only happens for ktx2 texture. cesium does not use gltf's origin minfilter.

origin:
image

fixed:
image

test gltf file:
t2.zip

@cesium-concierge
Copy link

Thanks for the pull request @vtxf!

  • ❌ Missing CLA.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.

@vtxf
Copy link
Contributor Author

vtxf commented Oct 15, 2021

Thanks for the pull request @vtxf!

  • ❌ Missing CLA.

  • ❔ Unit tests were not updated.

    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.

I have send CLA.

@ebogo1
Copy link
Contributor

ebogo1 commented Nov 5, 2021

This should also fix #9746:

On this branch:
image

On main:
image

Copy link
Contributor

@ebogo1 ebogo1 left a comment

Choose a reason for hiding this comment

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

Thanks @vtxf! This change looks good to me, and this should be good to merge to main. Since we missed the 1.87 release on this one (sorry about that), could you please merge main into your branch and update the CHANGES.md entry to put the fix into a new section for the 1.88 release?

I had a couple minor suggestions and this should be ready to go.

CONTRIBUTORS.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
@vtxf
Copy link
Contributor Author

vtxf commented Nov 6, 2021

Thanks @vtxf! This change looks good to me, and this should be good to merge to main. Since we missed the 1.87 release on this one (sorry about that), could you please merge main into your branch and update the CHANGES.md entry to put the fix into a new section for the 1.88 release?

I had a couple minor suggestions and this should be ready to go.

Thanks for your suggestions and I have updated the branch.

@ebogo1
Copy link
Contributor

ebogo1 commented Nov 9, 2021

Thanks again @vtxf! I pushed a small commit to update CHANGES.md after a mini release we did today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants