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.

Sign up for GitHub

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

Jump to bottom

Avoid spawning gizmo meshes when no gizmos are being drawn #8180

Merged
merged 2 commits into from
Apr 17, 2023
Merged

Avoid spawning gizmo meshes when no gizmos are being drawn #8180

merged 2 commits into from
Apr 17, 2023

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Mar 23, 2023 •
edited
Loading

Objective

Avoid queuing empty meshes for rendering.
Should prevent #8144 from triggering when no gizmos are in use. Not a real fix, unfortunately.

Solution

Add an in_use field to GizmoStorage and only set it to true when there are gizmos to draw.

@james7132 james7132 added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times labels Mar 23, 2023
@james7132 james7132 added this to the 0.11 milestone Mar 23, 2023
@james7132 james7132 added the C-Bug An unexpected or incorrect behavior label Mar 23, 2023
@tim-blackbird
Copy link
Contributor Author

@doup @rparrett Please give this a test on the 2d examples (besides 2d_gizmos).

@doup
Copy link
Contributor

doup commented Mar 23, 2023

The 2d/UI examples work for me now. 👍

@ManevilleF ManevilleF mentioned this pull request Mar 25, 2023
Closed
@james7132
Copy link
Member

Do we still need this after #8223?

@tim-blackbird
Copy link
Contributor Author

tim-blackbird commented Apr 17, 2023 •
edited
Loading

Do we still need this after #8223?

It feels icky to queue meshes for rendering that we know are empty as this will trigger creation of the pipeline even if an app never uses gizmos

@JMS55
Copy link
Contributor

JMS55 commented Apr 17, 2023

Do we still need this after #8223?

Yes, imo. It avoids allocating and uploading empty mesh data to a buffer every frame which is a waste, in addition to the pipeline like mentioned above.

@cart cart added this pull request to the merge queue Apr 17, 2023
Merged via the queue into bevyengine:main with commit e54057c Apr 17, 2023
@DanielHZhang DanielHZhang mentioned this pull request Apr 18, 2023
Closed
mockersf added a commit to mockersf/bevy that referenced this pull request Apr 18, 2023
@mockersf
Revert "Avoid spawning gizmo meshes when no gizmos are being drawn (b…
fb61977 
…evyengine#8180)"

This reverts commit e54057c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@james7132 james7132 james7132 left review comments

@cart cart cart approved these changes

@JMS55 JMS55 JMS55 approved these changes

@IceSentry IceSentry Awaiting requested review from IceSentry

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Milestone
0.11
Development

Successfully merging this pull request may close these issues.
5 participants
@tim-blackbird @doup @james7132 @JMS55 @cart