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

[Merged by Bors] - many_cubes: Add a cube pattern suitable for benchmarking culling changes #4126

Closed

Conversation

superdump
Copy link
Contributor

Objective

  • Add a cube pattern to many_cubes suitable for benchmarking culling changes

Solution

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 6, 2022
@superdump
Copy link
Contributor Author

While investigating:

  • Visibility as a marker component #3796
    I wanted a 3D benchmark to test with. The recent changes to many_cubes seemed promising. However, the pattern of placing cubes on the surface of a large cube produce view-dependent number of visible meshes. This was intended to demonstrate frustum culling is working properly. I needed something more consistent so I found an algorithm for reasonably evenly distributing the cubes on the surface of a sphere. This does indeed maintain the number of visible meshes at fairly consistent numbers, making it more suitable for benchmarking changes to the culling and visibility systems.

@superdump superdump requested a review from mockersf March 7, 2022 00:02
@superdump superdump added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples and removed S-Needs-Triage This issue needs to be labelled labels Mar 7, 2022
@superdump
Copy link
Contributor Author

Screenshot 2022-03-07 at 01 05 34

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.

Well-motivated, code quality is solid. This is also very pretty!

Should we consider moving this and bevymark to their own folder? They're not exactly useful as learning material IMO.

@superdump
Copy link
Contributor Author

Should we consider moving this and bevymark to their own folder? They're not exactly useful as learning material IMO.

I think it could be useful to have a place for tests/benchmarks that exercise various aspects of the engine and enable developers to test/check things without having to (re)write the same things over and over. At least I find I’m very commonly using examples, possibly slightly modified, for testing various things during development. I also think we should make a gltf viewer out of load_gltf and a flying camera implementation like that in the shadow biases example. But that’s all off topic for the PR. :)

@alice-i-cecile
Copy link
Member

Agreed; we should consider that in a separate PR I think.

@mockersf
Copy link
Member

mockersf commented Mar 7, 2022

Could more people test this example to ensure it doesn't cause dizziness or discomfort?

Should we consider moving this and bevymark to their own folder? They're not exactly useful as learning material IMO.

bevy_mark, many_sprites, many_cubes, text_debug and font_atlas_debug come to mind. But yes, another PR 🙂

@mockersf
Copy link
Member

mockersf commented Mar 8, 2022

@superdump could you also update the few ..Default::default() that slipped through in the last PR on that example to the new ..default()?

@superdump
Copy link
Contributor Author

@superdump could you also update the few ..Default::default() that slipped through in the last PR on that example to the new ..default()?

Done.

@alice-i-cecile
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 8, 2022
@superdump
Copy link
Contributor Author

bors try

@bors
Copy link
Contributor

bors bot commented Mar 8, 2022

try

Already running a review

@superdump
Copy link
Contributor Author

Oh, I thought the test progress would be shown at the bottom and it had failed again. I see the status is elsewhere.

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Mar 8, 2022
…ges (#4126)

# Objective

- Add a cube pattern to `many_cubes` suitable for benchmarking culling changes

## Solution

- Use a 'golden spiral' mapped to a sphere with the strategy of optimising for average nearest neighbour distance, as per: http://extremelearning.com.au/how-to-evenly-distribute-points-on-a-sphere-more-effectively-than-the-canonical-fibonacci-lattice/
@alice-i-cecile
Copy link
Member

We had a couple sets of eyes on this for nausea, and this is a very niche example. We can tweak it later if we get complaints.

@bors bors bot changed the title many_cubes: Add a cube pattern suitable for benchmarking culling changes [Merged by Bors] - many_cubes: Add a cube pattern suitable for benchmarking culling changes Mar 8, 2022
@bors bors bot closed this Mar 8, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
…ges (bevyengine#4126)

# Objective

- Add a cube pattern to `many_cubes` suitable for benchmarking culling changes

## Solution

- Use a 'golden spiral' mapped to a sphere with the strategy of optimising for average nearest neighbour distance, as per: http://extremelearning.com.au/how-to-evenly-distribute-points-on-a-sphere-more-effectively-than-the-canonical-fibonacci-lattice/
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…ges (bevyengine#4126)

# Objective

- Add a cube pattern to `many_cubes` suitable for benchmarking culling changes

## Solution

- Use a 'golden spiral' mapped to a sphere with the strategy of optimising for average nearest neighbour distance, as per: http://extremelearning.com.au/how-to-evenly-distribute-points-on-a-sphere-more-effectively-than-the-canonical-fibonacci-lattice/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants