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

Use BinnedRenderPhase for Opaque2d #13091

Merged

Conversation

IceSentry
Copy link
Contributor

@IceSentry IceSentry commented Apr 25, 2024

Based on top of #12982 and #13069

Objective

  • Opaque2d was implemented with SortedRenderPhase but BinnedRenderPhase should be much faster

Solution

  • Implement BinnedRenderPhase for Opaque2d

Notes

While testing this PR, before the change I had ~14 fps in bevymark with 100k entities. After this change I get ~71 fps, compared to using sprites where I only get ~63 fps. This means that after this PR mesh2d with opaque meshes will be faster than the sprite path. This is not a 1 to 1 comparison since sprites do alpha blending.

@IceSentry IceSentry added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Performance A change motivated by improving speed, memory usage or compile times labels Apr 25, 2024
@IceSentry IceSentry added this to the 0.14 milestone Apr 25, 2024
@IceSentry
Copy link
Contributor Author

waiting on #13069 to be merged so I can clean up this PR

@IceSentry IceSentry added the S-Blocked This cannot move forward until something else changes label May 6, 2024
@IceSentry IceSentry modified the milestones: 0.14, 0.15 May 13, 2024
@jgayfer
Copy link
Contributor

jgayfer commented Jun 11, 2024

@IceSentry should I hold off on reviewing until you've cleaned up, or will the cleaning be minor?

@IceSentry
Copy link
Contributor Author

This will require enough work to patch that I might need to create a new PR. We'll have to wait on the other PR being merged to see.

@IceSentry IceSentry force-pushed the opaque2d-binned-render-phase branch from 2e53c8f to 69e5fa4 Compare August 7, 2024 03:03
@IceSentry
Copy link
Contributor Author

@jgayfer this should be ready to review now

@IceSentry IceSentry removed the S-Blocked This cannot move forward until something else changes label Aug 7, 2024
Copy link
Contributor

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

really good documentation of this kind of conversion including using MeshAllocator, ty~!

crates/bevy_sprite/src/mesh2d/material.rs Show resolved Hide resolved
@tychedelia tychedelia added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Aug 7, 2024
Copy link
Contributor

@kristoff3r kristoff3r left a comment

Choose a reason for hiding this comment

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

LGTM, I also tested a few examples on wasm

@tychedelia tychedelia added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 7, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 12, 2024
Merged via the queue into bevyengine:main with commit 9d6a4fb Aug 12, 2024
32 checks passed
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-Feature A new feature, making something new possible C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants