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

Add many_materials stress test #11592

Closed
wants to merge 5 commits into from
Closed

Conversation

atlv24
Copy link
Contributor

@atlv24 atlv24 commented Jan 28, 2024

Objective

Solution

  • Brings back animated_material example but cranked up and with shadows

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Code looks good, stress test runs (and stresses both me and my machine).

I don't understand why 3d_scene got changed as part of this PR though: I think that's just a git mistake?

@atlv24
Copy link
Contributor Author

atlv24 commented Jan 28, 2024

Whoops good catch. That was leftovers from testing a bug in bevy main that im gonna be opening an issue for soon

Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

1 similar comment
Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Benchmarks Stress tests and benchmarks used to measure how fast things are A-Animation Make things move and change over time labels Jan 28, 2024
mut meshes: ResMut<Assets<Mesh>>,
mut materials: ResMut<Assets<StandardMaterial>>,
) {
let n = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a stress test this should probably be configurable using argh. Something like bevymark.

Copy link
Contributor

@IceSentry IceSentry Jan 28, 2024

Choose a reason for hiding this comment

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

Maybe giving it a specific amount would be more useful though.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you're making it configurable with argh, please copy the cfg blocks from the other stress tests using it (eg. many_cubes) as argh::from_env() panics in wasm.

Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

Other than the cli config suggestion LGTM

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 28, 2024
Cargo.toml Outdated Show resolved Hide resolved
});

// Helmets
let helmet = asset_server.load("models/FlightHelmet/FlightHelmet.gltf#Scene0");
Copy link
Member

Choose a reason for hiding this comment

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

using the helmet instead of a pbrbundle means you're also adding hierarchy and complex meshes to the stress test

it also means we don't have direct control of the material and can't try easily if it's because one of the property of it

I would prefer to use a PbrBundle as it makes a more focused stress test, and it wouldn't need the make_materials_unique system

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I think I agree with this analysis. It would be better to be able to properly isolate the effects here.

@alice-i-cecile alice-i-cecile removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 29, 2024
Co-authored-by: François <[email protected]>
Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@atlv24 atlv24 closed this Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-Rendering Drawing game state to the screen C-Benchmarks Stress tests and benchmarks used to measure how fast things are
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a stress test for animating materials
5 participants