Skip to content

Add test for mesh transformations #18194

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 10 commits into
base: main
Choose a base branch
from

Conversation

greeble-dev
Copy link
Contributor

@greeble-dev greeble-dev commented Mar 7, 2025

Objective

Add a test that will repro issues with mesh transformations, like #17992.

Solution

The PR adds a test_transform_mesh example that transforms a mesh in three different ways - each transformation should have the same visual result.

The screenshot shows the test reproducing #17992 - mesh 1 should have matched mesh 2 and 3.

image

The mesh uses an anisotropic material so that mesh tangents and normals are significant.

Concerns

  • Right now the test is not suitable for screenshot comparison - the light is animated.
  • Anisotropy is arguably an imprecise way to verify tangents and normals. But it's convenient and does everything in a single screenshot.

Testing

Tested on Win10/Vulkan/Nvidia.

cargo run --example test_transform_mesh --features "pbr_anisotropy_texture"

@greeble-dev
Copy link
Contributor Author

greeble-dev commented Mar 7, 2025

CI failure appears unrelated to the PR.

https://github.com/bevyengine/bevy/actions/runs/13727867039/job/38398331235?pr=18194#logs

Run cargo deny check advisories
error[unmaintained]: paste - no longer maintained
    ┌─ /home/runner/work/bevy/bevy/Cargo.lock:369:1
    │
369 │ paste 1.0.15 registry+https://github.com/rust-lang/crates.io-index
    │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ unmaintained advisory detected
    │
    ├ ID: RUSTSEC-2024-0436
    ├ Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0436

@mockersf
Copy link
Member

mockersf commented Mar 7, 2025

rather than using anisotropy, didn't we use to have a debug view to show tangents/normals?

@greeble-dev
Copy link
Contributor Author

greeble-dev commented Mar 8, 2025

The shader_prepass example shows normals, but I couldn't find anything that shows tangents. Although a TangentPrepass might be a valid (if somewhat niche) feature?

(EDIT: Noticed this abandoned PR that added debug materials: #7390)

I also considered drawing a per-vertex gizmo. But that doesn't cover the third test case which validates transforms in the shader.

So my guess is that anisotropy is the least worst choice right now for showing that both tangents and normals are correct.

name = "transform_mesh"
path = "tests/3d/transform_mesh.rs"
doc-scrape-examples = true
required-features = ["pbr_anisotropy_texture"]
Copy link
Contributor Author

@greeble-dev greeble-dev Mar 8, 2025

Choose a reason for hiding this comment

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

Requiring feature pbr_anisotropy_texture is a bug in the shader. The example doesn't actually use anisotropy textures. I'll file an issue if this PR goes through.

@alice-i-cecile alice-i-cecile 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 A-Math Fundamental domain-agnostic mathematical operations S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 8, 2025
@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Mar 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations 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 X-Uncontroversial This work is generally agreed upon
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants