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

do not spend time drawing text with is_visible = false #893

Merged
merged 2 commits into from
Nov 22, 2020

Conversation

marius851000
Copy link
Contributor

I have found that all the process of drawing text are performed, even when the is_visible property of draw is set to false (althought it didn't drawed them before). This should allow instanced invisible text to consume nearly no ressource. For an exemple, on a debug build with 100 texts on firafont-bold, it run at around 1 fps wihout this patch, and at 60 fps with this patch.

@minecrawler
Copy link

minecrawler commented Nov 19, 2020

Just wondering, but what happened to keeping the pyramid small and using early return (or continue in this case)? I really have to re-evaluate my own standards and would love some feedback with PRs of this kind in mind :)

if !draw.is_visible { continue; }

@marius851000
Copy link
Contributor Author

@minecrawler I used an if x { do stuff } rather than an early continue for no particular reason. (although I never considered using a continue just for reducing the number of indent. Will keep this in mind the next time I'm in a similar situation.).

Also, the clippy error in the CI seem unrelated.

@Moxinilian Moxinilian added C-Performance A change motivated by improving speed, memory usage or compile times A-UI Graphical user interfaces, styles, layouts, and widgets labels Nov 19, 2020
@cart cart force-pushed the do-not-draw-invisible-text branch from 5912e59 to b68a63f Compare November 21, 2020 21:54
@cart
Copy link
Member

cart commented Nov 21, 2020

Yeah I do like the if !draw.is_visible { continue; } approach a bit more. I made the change (and rebased on master, which has CI fixes)

@cart cart merged commit eb587b2 into bevyengine:master Nov 22, 2020
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants