Make sprites and text2d work with RenderLayers (add ComputedVisibility) #4007

johanhelsing · 2022-02-21T13:07:33Z

Objective

Make sprites and text2d work with RenderLayers, so a subset of sprites could be rendered with one camera.

Solution

Add ComputedVisibility to SpriteBundle and Text2DBundle
Extract the entity handle as well when extracting sprites
Create a hash map out of VisibleEntities per view.
Check if extracted sprites are present in the hashmap

Maybe there are more efficient ways of doing this than recreating hashmaps all the time?

Anyways, I didn't see any drop in performance when running the bevymark example, so perhaps it's ok.

EDIT: It's a ~17% decline in performance with many_sprites, but no change in bevymark. bevymark is perhaps a bit contrived? Isn't it better to be correct than performant? Most of the performance hit seems to be caused by sampling the hashmap.

crates/bevy_sprite/src/render/mod.rs

mockersf · 2022-02-21T23:21:06Z

Anyways, I didn't see any drop in performance when running the bevymark example, so perhaps it's ok.

Please also check with the many_sprites example

mcobzarenco · 2022-02-22T01:38:20Z

Maybe there are more efficient ways of doing this than recreating hashmaps all the time?

To avoid the memory allocations, you could store the hash set in a Local and reuse it

crates/bevy_sprite/src/render/mod.rs

johanhelsing · 2022-02-22T09:04:14Z

Please also check with the many_sprites example

Before: 45.5
Now (build and check hashset in queue_sprite): 37.5
Alternative: Change type of VisibleEntities::entities to HashSet<Entity>: 37
Naive checking (no HashSet, just check the Vec, O(n*m)-style): 0.5

Some possible insights into what exactly makes it slower:

Only add ComputedVisibility, but don't use it in queue_sprites: 42.5
Build the HashSet in queue_sprites, but don't check it: 42

So it seems that building the hash set is not the problem (or at least a very small one), it's actually checking it when iterating the extracted_sprites.

TL;DR: It's a 17% decline in performance with many_sprites, but no change in bevymark. Not sure how contrived many_sprites is? Is it better to be correct or performant?

superdump · 2022-03-02T05:53:49Z

Did you try iterating the visible entities and getting them from the query and not using a HashSet? I think getting an entity from a query is faster (O(1) maybe?) than checking whether an entity from a query is in a Vec.

johanhelsing · 2022-03-07T12:38:46Z

Did you try iterating the visible entities and getting them from the query and not using a HashSet? I think getting an entity from a query is faster (O(1) maybe?) than checking whether an entity from a query is in a Vec.

Won't this mess up the z-ordering (sprites can be transparent)? I guess I could still try it for comparison. Or do you mean copy all the visible sprites into a new vec and then sort that?

Also, what I'm currently doing is building a hashset (should bo O(n)?), and then checking whether an entity is in that hashset should be average constant time, right?

EDIT: Also, in queue_sprites, ExtractedSprites is a Vec not a Query, so I don't understand how that would work. The sprite query is available in extract_sprites, but at that point I don't think there are any views yet? So I guess in order for this to work, the extracted sprites would have to exist as entities in the render world?

TethysSvensson · 2022-04-05T11:31:39Z

I would really love to get this merged, as I am using multiple camera with different RenderLayers and would like to be able to put text on only one of them.

Quick and dirty ComputedVisiblity support for sprites, text2d This is needed for RenderLayers to work properly Use a HashSet to speed up visibility checking Remove resolved todo Don't reallocate visible sprite entities map every frame

infmagic2047 · 2022-05-31T11:39:11Z

I would like to get this merged. #4745 makes it much easier to use multiple cameras, so probably more people will run into this RenderLayers problem.

This is needed for RenderLayers to work properly

johanhelsing · 2022-06-02T06:41:03Z

Rebased to fix conflicts

james7132

If performance is an issue, I have a small suggestion on how to speed it up.

james7132 · 2022-06-24T07:38:45Z

crates/bevy_sprite/src/render/mod.rs

            let extracted_sprites = &mut extracted_sprites.sprites;
            let image_bind_groups = &mut *image_bind_groups;

+            visible_entities_map.clear();


Consider using a FixedBitset instead of a HashMap here. A contains check using that should be faster than a HashSet, it doesn't involve a O(capacity) scan in the worst case and does not involve a hash on each lookup. Instead of storing the full entity, convert the u32 Entity ID into a usize index, and use that to set and check the FixedBitset.

I agree that that suggestion should be an improvement to performance. I want to try to figure out if there is a way to restructure so that the set can be avoided.

@james7132 did you ever try using a fixed bit set for visible entities? I seem to remember that you did but I don’t remember.

Also, random thought that is not thought through and that may not be good but could be, is it worth storing a fixed bit set of views in which an entity is visible, per entity, instead of a fixed bit set of entities visible per-view? Maybe not as maybe the per-view set is small enough to stay in cache where the per-entity sets would be loaded in as part of iteration… I don’t know.

VisibleEntities IMO shouldn't, though I haven't tried. It'd require an &Entities on the other end to resolve the Entity from the ID, which may or may not be desirable depending on the downstream use case.

superdump

This will be a useful change if we can reduce/avoid the performance deficit.

superdump · 2022-06-24T09:18:35Z

crates/bevy_sprite/src/render/mod.rs

    events: Res<SpriteAssetEvents>,
+    mut visible_entities_map: Local<HashSet<Entity>>,


This isn’t a map, it’s a set.

superdump · 2022-06-24T09:21:22Z

crates/bevy_sprite/src/render/mod.rs

-    sprite_query: Query<(&Visibility, &Sprite, &GlobalTransform, &Handle<Image>)>,
+    sprite_query: Query<(
+        Entity,
+        &Visibility,


ComputedVisibility needs to be used for extraction.

superdump · 2022-06-24T09:21:51Z

crates/bevy_sprite/src/render/mod.rs

    atlas_query: Query<(
+        Entity,
        &Visibility,


ComputedVisibility needs to be used for extraction here too.

superdump · 2022-06-24T09:35:00Z

crates/bevy_sprite/src/render/mod.rs

            let extracted_sprites = &mut extracted_sprites.sprites;
            let image_bind_groups = &mut *image_bind_groups;

+            visible_entities_map.clear();


I agree that that suggestion should be an improvement to performance. I want to try to figure out if there is a way to restructure so that the set can be avoided.

@james7132 did you ever try using a fixed bit set for visible entities? I seem to remember that you did but I don’t remember.

Also, random thought that is not thought through and that may not be good but could be, is it worth storing a fixed bit set of views in which an entity is visible, per entity, instead of a fixed bit set of entities visible per-view? Maybe not as maybe the per-view set is small enough to stay in cache where the per-entity sets would be loaded in as part of iteration… I don’t know.

infmagic2047 · 2022-06-24T16:16:13Z

In addition to SpriteBundle, a ComputedVisibility component is needed on SpriteSheetBundle as well.

cart · 2022-06-26T06:58:16Z

Given that RenderLayers-like functionality is pretty important for things like "camera driven rendering", I think we might need to just eat this perf cost.

Added this to the 0.8 milestone as I think its worth making a call on "2d visibility controls" as part of the "camera driven rendering" story.

superdump · 2022-06-26T07:12:01Z

This was last updated 24 days ago but #4489 was merged 12 days ago and will impact real world performance. Also, I have ideas for improving sprite rendering performance further so we can probably take the hit.

kserz · 2022-06-27T17:02:13Z

I've made a version #5114 with a different tradeoff between straight logic and performance. Any opinions are much appreciated.

TethysSvensson · 2022-07-19T10:39:49Z

Has this been superseeded by #5310?

superdump · 2022-07-19T11:09:22Z

Yes, Sprites and Text2d now support RenderLayers and ComputedVisibility

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Feb 21, 2022

johanhelsing commented Feb 21, 2022

View reviewed changes

crates/bevy_sprite/src/render/mod.rs Outdated Show resolved Hide resolved

mockersf added A-Rendering Drawing game state to the screen C-Enhancement A new feature and removed S-Needs-Triage This issue needs to be labelled labels Feb 21, 2022

mcobzarenco reviewed Feb 22, 2022

View reviewed changes

crates/bevy_sprite/src/render/mod.rs Outdated Show resolved Hide resolved

johanhelsing added 4 commits June 2, 2022 08:38

Quick and dirty ComputedVisiblity support for sprites, text2d

8bceb1c

This is needed for RenderLayers to work properly

Use a HashSet to speed up visibility checking

6377ee3

Remove resolved todo

1ddb82d

Don't reallocate visible sprite entities map every frame

759c292

johanhelsing force-pushed the sprite-visibility branch from 456b5ce to 759c292 Compare June 2, 2022 06:40

infmagic2047 mentioned this pull request Jun 24, 2022

render::view::RenderLayers does not affect SpriteBundle #5089

Closed

james7132 reviewed Jun 24, 2022

View reviewed changes

superdump requested changes Jun 24, 2022

View reviewed changes

cart added this to the Bevy 0.8 milestone Jun 26, 2022

kserz mentioned this pull request Jun 27, 2022

Make sprites respect RenderLayers #5114

Closed

alice-i-cecile removed this from the Bevy 0.8 milestone Jun 27, 2022

alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Jun 27, 2022

alice-i-cecile mentioned this pull request Jul 8, 2022

User UI camera control #5252

Closed

cart mentioned this pull request Jul 12, 2022

Allow Visibility to be inherited #5146

Closed

superdump closed this Jul 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sprites and text2d work with RenderLayers (add ComputedVisibility) #4007

Make sprites and text2d work with RenderLayers (add ComputedVisibility) #4007

johanhelsing commented Feb 21, 2022 •

edited

mockersf commented Feb 21, 2022

mcobzarenco commented Feb 22, 2022

johanhelsing commented Feb 22, 2022

superdump commented Mar 2, 2022 •

edited

johanhelsing commented Mar 7, 2022 •

edited

TethysSvensson commented Apr 5, 2022

infmagic2047 commented May 31, 2022

johanhelsing commented Jun 2, 2022

james7132 left a comment

james7132 Jun 24, 2022 •

edited

superdump Jun 24, 2022

james7132 Jun 24, 2022

superdump left a comment

superdump Jun 24, 2022

superdump Jun 24, 2022

superdump Jun 24, 2022

superdump Jun 24, 2022

infmagic2047 commented Jun 24, 2022

cart commented Jun 26, 2022

superdump commented Jun 26, 2022

kserz commented Jun 27, 2022

TethysSvensson commented Jul 19, 2022

superdump commented Jul 19, 2022

		events: Res<SpriteAssetEvents>,
		mut visible_entities_map: Local<HashSet<Entity>>,

Make sprites and text2d work with RenderLayers (add ComputedVisibility) #4007

Make sprites and text2d work with RenderLayers (add ComputedVisibility) #4007

Conversation

johanhelsing commented Feb 21, 2022 • edited

Objective

Solution

mockersf commented Feb 21, 2022

mcobzarenco commented Feb 22, 2022

johanhelsing commented Feb 22, 2022

superdump commented Mar 2, 2022 • edited

johanhelsing commented Mar 7, 2022 • edited

TethysSvensson commented Apr 5, 2022

infmagic2047 commented May 31, 2022

johanhelsing commented Jun 2, 2022

james7132 left a comment

Choose a reason for hiding this comment

james7132 Jun 24, 2022 • edited

Choose a reason for hiding this comment

superdump Jun 24, 2022

Choose a reason for hiding this comment

james7132 Jun 24, 2022

Choose a reason for hiding this comment

superdump left a comment

Choose a reason for hiding this comment

superdump Jun 24, 2022

Choose a reason for hiding this comment

superdump Jun 24, 2022

Choose a reason for hiding this comment

superdump Jun 24, 2022

Choose a reason for hiding this comment

superdump Jun 24, 2022

Choose a reason for hiding this comment

infmagic2047 commented Jun 24, 2022

cart commented Jun 26, 2022

superdump commented Jun 26, 2022

kserz commented Jun 27, 2022

TethysSvensson commented Jul 19, 2022

superdump commented Jul 19, 2022

johanhelsing commented Feb 21, 2022 •

edited

superdump commented Mar 2, 2022 •

edited

johanhelsing commented Mar 7, 2022 •

edited

james7132 Jun 24, 2022 •

edited