Skip to content

Add test for GltfLoaderSettings::override_sampler and mipmap rendering #19095

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

greeble-dev
Copy link
Contributor

Objective

Add a test for GltfLoaderSettings::override_sampler. This also serves as a test of the renderer and mipmap filtering options.

gltf_override_sampler.mp4

Solution

#17875 recently added support for overriding texture samplers during glTF loading. This PR adds an example-based test for that feature - it loads a simple checkerboard model with various sampler settings and lets the user switch between them.

As a bonus, this also tests the rendering side of mipmap filtering. Currently there are no other tests or examples that do this.

But there's a catch! The test works by loading the same glTF multiple times, each time changing GltfLoaderSettings::override_sampler. That falls afoul of #18267 - subsequent loads of the same asset return the first loaded version, effectively ignoring the different settings of later loads.

I've confirmed that the test does work if the assets are duplicated. This can be reproduced in the PR branch by syncing to faefb8dda88ed97c9e127e5948c700c8a6275e0c.

Concerns

  • Is it worth commiting a broken test?
  • Should this test be framed as a glTF test or a renderer test?
    • Arguably the glTF loader behaviour should be in a unit test.
    • Maybe this test should be renamed to 3d/test_mipmap_filtering or something.
  • The test is not currently suitable for screenshot comparison.
  • The test adds some assets.
    • I'm hoping this is ok as they're tiny - roughly 3KB.

Testing

cargo run --example gltf_override_sampler

Tested on Win10/Nvidia, across Desktop/Vulkan, Chrome/WebGL, Chrome/WebGPU.

@greeble-dev greeble-dev added A-Rendering Drawing game state to the screen C-Testing A change that impacts how we test Bevy or how users test their apps S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-glTF Related to the glTF 3D scene/model format labels May 6, 2025
@greeble-dev greeble-dev added the D-Straightforward Simple bug fixes and API improvements, docs, test and examples label May 6, 2025
@alice-i-cecile
Copy link
Member

The test is not currently suitable for screenshot comparison.

We should try and fix this.

Should this test be framed as a glTF test or a renderer test?

Renderer test if we're going to be doing rendering and screenshots. Like you say, gltf loading should be tested purely in headless code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-glTF Related to the glTF 3D scene/model format A-Rendering Drawing game state to the screen C-Testing A change that impacts how we test Bevy or how users test their apps D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants