Skip to content

Add compute_*_normals benchmarks#18648

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
Waridley:compute_normals_bench
Mar 31, 2025
Merged

Add compute_*_normals benchmarks#18648
alice-i-cecile merged 1 commit intobevyengine:mainfrom
Waridley:compute_normals_bench

Conversation

@Waridley
Copy link
Copy Markdown
Contributor

@Waridley Waridley commented Mar 31, 2025

Objective

Benchmark current compute_*_normals methods to get a baseline as requested in #18552 (comment)

Solution

Since the change to the default smooth normals method will definitely cause a regression, but the previous method will remain as an option, I added two technically-redundant benchmarks but with different names: smooth_normals for whatever default weighting method is used, and face_weighted_normals to benchmark the area-weighted method regardless of what the default is. Then I'm adding angle_weighted_normals in #18552. I also added flat_normals for completeness.

IceSentry
IceSentry approved these changes Mar 31, 2025
View reviewed changes
Copy link
Copy Markdown
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.

LGTM, thank you!

@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 X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 31, 2025
@IceSentry IceSentry added C-Performance A change motivated by improving speed, memory usage or compile times and removed C-Benchmarks Stress tests and benchmarks used to measure how fast things are X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 31, 2025
@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 Mar 31, 2025
@IceSentry
Copy link
Copy Markdown
Contributor

IceSentry commented Mar 31, 2025

Oh no, sorry @alice-i-cecile looks like 2 people changing labels at the same time is broken 😅

@IceSentry IceSentry added C-Benchmarks Stress tests and benchmarks used to measure how fast things are X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed C-Performance A change motivated by improving speed, memory usage or compile times labels Mar 31, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 31, 2025
Merged via the queue into bevyengine:main with commit 89e00b1 Mar 31, 2025
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

+1 more reviewer

@IceSentry IceSentry IceSentry approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Rendering Drawing game state to the screen C-Benchmarks Stress tests and benchmarks used to measure how fast things are D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon

Projects

No open projects
Rendering (Old)
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Waridley @IceSentry @alice-i-cecile