[Merged by Bors] - Add 2d meshes and materials

[Davier]

Objective

The current 2d rendering is specialized to render sprites, we need a generic way to render 2d items, using meshes and materials like we have for 3d.

Solution

I cloned a good part of bevy_pbr into bevy_sprite/src/mesh2d, removed lighting and pbr itself, adapted it to 2d rendering, added a ColorMaterial, and modified the sprite rendering to break batches around 2d meshes.

The PR is a bit crude; I tried to change as little as I could in both the parts copied from 3d and the current sprite rendering to make reviewing easier. In the future, I expect we could make the sprite rendering a normal 2d material, cleanly integrated with the rest. edit: see #3460 (comment)

Remaining work

• don't require mesh normals out of scope
• add an example done
• support 2d meshes & materials in the UI?
• bikeshed names (I didn't think hard about naming, please check if it's fine)

Remaining questions

• should we add a depth buffer to 2d now that there are 2d meshes? let's revisit that when we have an opaque render phase
• should we add MSAA support to the sprites, or remove it from the 2d meshes? I added MSAA to sprites since it's really needed for 2d meshes
• how to customize vertex attributes? Add vertex buffer layout to Mesh #3120

[alice-i-cecile]

@cart this appears to be needed for bevy_prototype_lyon to migrate; do you want to add it to the milestone? (ping @Nilirad)

[mockersf]

This fixes #3326 so it's part of the 0.6 milestone

For bevy_prototype_lyon, it's using its own shaders / pipeline so probably not related to this one

[Davier]

For bevy_prototype_lyon, it's using its own shaders / pipeline so probably not related to this one

This PR is what will allow people to use their own shaders/pipelines in 2d along the new sprite pipeline.

---

This implementation has an important difference with how it worked in 0.5 with SpriteBundle. The vertices' positions used to be scaled with the sprite's size. I don't think we should keep that behaviour, but I'm open to feedback.

[sarkahn]

This implementation has an important difference with how it worked in 0.5 with SpriteBundle. The vertices' positions used to be scaled with the sprite's size. I don't think we should keep that behaviour, but I'm open to feedback.

The default orthographic camera is set to render 1 world unit as 1 pixel. If we change this to be 1-1 with world units then the default 2d camera (and sprite scaling) should also be changed, right?

For the record I much prefer keeping the meshes in world unit scale and adjusting the camera projection scale to correctly display the target pixels per unit.

However, that is not what the default is set up for right now. May be better to keep it consistent with sprites (scale by size/pixels) and leave the alternative for a separate pr/poc.

[rparrett]

Attempted to port bevy_prototype_lyon, but I think it needs some sort of support for vertex colors. I think it's really only possible to do solid colors, but strokes and fill can be different colors and they are crammed into the same mesh.

Is that possible as-is with a custom pipeline / shader, or would something like #3120 be required?

I suppose it could generate a separate mesh for stroke and fill, but that doesn't seem ideal.

bevy_prototype_lyon could also use UI support if it were added, but I'm not sure how many people were actually using that feature. "ui shapes" never even made it into a crate release.

[Davier]

The default orthographic camera is set to render 1 world unit as 1 pixel. If we change this to be 1-1 with world units then the default 2d camera (and sprite scaling) should also be changed, right?

I'm not planning on touching sprite or camera scaling, that stays the same. The difference only applies for people that used a SpriteBundle in 0.5 with custom meshes, and switch to the new 2d meshes with ColorMaterial. Those that use the new SpriteBundle won't be affected.

Attempted to port bevy_prototype_lyon, but I think it needs some sort of support for vertex colors. I think it's really only possible to do solid colors, but strokes and fill can be different colors and they are crammed into the same mesh.

Is that possible as-is with a custom pipeline / shader, or would something like #3120 be required?

It's already possible to define the vertex buffer as you need if you use a custom pipeline and shader, I think #3120's goal it to make it less involved.

I suppose it could generate a separate mesh for stroke and fill, but that doesn't seem ideal.

It sounds like a good temporary hack until #3120 is merged.

bevy_prototype_lyon could also use UI support if it were added, but I'm not sure how many people were actually using that feature. "ui shapes" never even made it into a crate release.

The UI rendering is basically a copy of the sprite rendering already. When we reach a consensus on how 2d meshes and materials should work, I should be able to either clone it into bevy_ui, or maybe add a generic parameter somewhere to reuse it.

[sarkahn]

I'm not planning on touching sprite or camera scaling, that stays the same. The difference only applies for people that used a SpriteBundle in 0.5 with custom meshes, and switch to the new 2d meshes with ColorMaterial. Those that use the new SpriteBundle won't be affected.

I understand, what I'm trying to say is it could be confusing to have opposite default scaling behavior between a custom 2d mesh and a 2d sprite (scaling by world units vs scaling by pixels). They should stay the same, and if we're going to change one we should change both in my opinion.

In other words if someone makes a Mesh2dBundle with a unit sized quad for a mesh, wouldn't it make intuitive sense for it to render identically to a sprite when using the default orthographic camera?

[Davier]

In other words if someone makes a Mesh2dBundle with a unit sized quad for a mesh, wouldn't it make intuitive sense for it to render identically to a sprite when using the default orthographic camera?

In that case, why are they not using a sprite? 2d meshes are for a different use case IMO. Also, the texture is optional in ColorMaterial, how would it scale when it's not set?

[sarkahn]

In other words if someone makes a Mesh2dBundle with a unit sized quad for a mesh, wouldn't it make intuitive sense for it to render identically to a sprite when using the default orthographic camera?

In that case, why are they not using a sprite? 2d meshes are for a different use case IMO. Also, the texture is optional in ColorMaterial, how would it scale when it's not set?

In my case I am building a single mesh made up of unit sized squares (a tilemap basically). I'm not using individual sprites because every tile in my grid is built from the same texture and my shader reads unique data to render each tile from the mesh verts.

Of course I have no issue adjusting if the default scaling is changed, just saying it's a case where I intuitively expect it to work similar to a sprite, but maybe my use case is outside the norm.

If the handle doesn't point to a texture I would expect it to default to world scaling.

[sarkahn]

Actually nevermind, now that I'm thinking about it I'm having to manually scale each "tile" anyways otherwise they would all be the size of the entire texture. I think changing the default does make sense. You were right in the first place, sorry about that.

[Davier]

No problem, thanks for testing the PR and giving feedback 😄

[Davier]

I wasn't happy with the previous solution to break sprite batching. It simply allowed other kinds of 2d items to add their depth in ExtractedSprites (and did it for those based on 2d materials).

Pros:

• keeps performance in bevymark
• few modifications to bevy_sprite

Cons:

• only works for sprite, it would be difficult for someone to batch their custom 2d pipeline too

In order for batching to work for all 2d pipelines, it needs to work directly on the render phase, after all items have been added.

Pros:

• can work for any number of pipelines
• it's easy to add batching to custom pipelines

Cons:

• reduces perf in bevymark (around 15% on my computer), since the items are all copied into the render phase (instead of only adding one item per batch)

The performance comparison is not exactly fair, it was worse at first so I did an optimization pass (but that could be backported to the previous batching solution). Also, the extract function is now more than twice as fast, which will make actual pipelining even more desirable.

The new solution introduces a BatchedPhaseItem trait, based on CachedPipelinePhaseItem, and a system in the PhaseSort stage that does the batching. Batching is not added to 2d meshes.

Other changes:

• most work is now done in RenderStage::Queue, is that OK or should I split as much as possible into RenderStage::Prepare at the cost of some perf?
• added the image size in GpuImage to avoid having to hash the Handle of every sprite
• switched to directly use GlobalTransforms instead of calculating a Mat4
• changed how rect and atlas_size worked since I found it very confusing, and added a custom_size field to TextureAtlasSprite for parity with Sprite

[djeedai]

Silly question maybe but I still don't understand why 2D and 3D are that much separated. And now, as the PR description details, we're cloning even more functionality from 3D into 2D to be on parity. Why can't there be a single path for both 2D and 3D meshes? Isn't a 2D mesh just a corner case of a 3D one? Is there any other engine going to that length to separate 2D and 3D? We have already some optimizations for 2D cases, like the layered distance calculation (Z-based instead of Euclidian). What else is so specific that it cannot be made generic over 2D and 3D?

[Davier]

I am able to reproduce the slight perf hit in bevymark vs main, but on my (mac m1) machine, it's more like 5-10% perhaps. (with cargo run --example bevymark --release -- 5 100)

The performance of bevymark on my computer has a lot of variation (even over time for a single run), so it's difficult to give a precise number. I kept 15% as a higher bound to be safe, but on average it seems closer to your numbers.

Might be a good idea to put together an example/demo to ensure that draw order / layering and stuff works properly when mixing colored sprites, textured sprites and mesh2d.

I tested it works by adding a few items to the sprite example. I didn't include it in the PR since it should be a dedicated example, and I couldn't find a nice idea of what to show. Would you like to include your example? (I think you could open a PR to this branch)

I updated my game that utilizes this (by way of bevy_prototype_lyon), without any issues in native or web builds.

That's great 😄

[rparrett]

It didn't seem like a super interesting example from a user's perspective to me, and it's pretty much just a mashup of sprite.rs, rect.rs, and #2973. So I'm not personally very interested in cleaning it up and trying to include it.

Seemed like a useful thing to have in the commentary section of this PR though.

Maybe a simpler example than mesh2d.rs or #2973 showing how to use Mesh2dBundle would be useful though?

Happy to work on any of the above if it helps this PR through.

edit: Cleaned up that gist a bit just in case.

[sarkahn]

I can't seem to get Mesh2dBundle to render. I made a simple test:
https://gist.github.com/sarkahn/f77b591103e88fee7e773966ab1f510a

Renders nothing for me.

[rparrett]

I can't seem to get Mesh2dBundle to render. I made a simple test: https://gist.github.com/sarkahn/f77b591103e88fee7e773966ab1f510a

Renders nothing for me.

let material = materials.add(asset_server.load("branding/icon.png").into());

Seems to fix it.

[sarkahn]

I can't seem to get Mesh2dBundle to render. I made a simple test: https://gist.github.com/sarkahn/f77b591103e88fee7e773966ab1f510a
Renders nothing for me.

let material = materials.add(asset_server.load("branding/icon.png").into());

Seems to fix it.

Ahh yup, that did it. The new version of Sprites take the direct image handle so that's how I got mixed up, thank you!

Edit: Updated my gist to a working version with comments if you're interested in including a ColorMaterial example

[cart]

Great work! I think this is probably good to go for now after the one comment is resolved.

Part of me wishes we waited to implement generic batch merging: the sprite perf loss is regrettable (~100k at 60fps to ~90k). We've already lost a bit of perf as the renderer expanded, so losing even more "hurts". And I'm worried that in practice this impl won't batch across z-levels as much as it could:

[A(z = 1), B(z = 2), A(z=2)]

We could technically batch A, but because B got arbitrarily sorted in front of A, the batches will remain split. Handling that case properly would probably require moving batching back to being more local, then somehow checking if something somewhere would split the batch. That would have the benefit of allowing us to do arbitrary batch logic per-context (and maybe regain some sprite batching perf). But that would require some synchronized "is there something else on this z-level" operation. Not sure doing that work would be a net win on average, but its worth discussing.

If the old "non-generic" version is still available somewhere, id like to see it (github doesnt show the old version). But I wouldn't block on that.

[Davier]

And I'm worried that in practice this impl won't batch across z-levels as much as it could:

[A(z = 1), B(z = 2), A(z=2)]]

We could technically batch A, but because B got arbitrarily sorted in front of A, the batches will remain split. Handling that case properly would probably require moving batching back to being more local, then somehow checking if something somewhere would split the batch. That would have the benefit of allowing us to do arbitrary batch logic per-context (and maybe regain some sprite batching perf). But that would require some synchronized "is there something else on this z-level" operation. Not sure doing that work would be a net win on average, but its worth discussing.

That's an option too, we could bench it. I'm more partial to a generic solution that also accounts for the on-screen overlap to really maximize batches.

Part of me wishes we waited to implement generic batch merging: the sprite perf loss is regrettable (~100k at 60fps to ~90k). We've already lost a bit of perf as the renderer expanded, so losing even more "hurts".

If the old "non-generic" version is still available somewhere, id like to see it (github doesnt show the old version). But I wouldn't block on that.

The commit before the generic version is 798a0ba, we could use it for 0.6 and postpone the generic stuff (we might want to backport the perf improvements). Nical suggested on discord to use the copyless crate, and it does recover some of the lost fps, so we could fast-track that. We could also remove the ComputedVisibility component on sprites to reduce time spent in the visibility system at the cost of frustum culling.

[Davier]

I pushed the other possible perf improvements in this branch if you want to try them: https://github.com/Davier/bevy/tree/mesh2d_perf

[rparrett]

Seems like frustum culling starts becoming a win again in many_sprites on my machine when doubling this value

let map_size = Vec2::splat(320.0); // culling pulls ahead at 640.0 or so, or ~380k sprites

(that mesh2d_perf branch with/without culling commit)

[cart]

The commit before the generic version is 798a0ba,

Bawhaha my commit search was apparently laughably bad 😄

I pushed the other possible perf improvements in this branch if you want to try them: https://github.com/Davier/bevy/tree/mesh2d_perf

I'm getting ~8k more sprites with these. Seems worth it! Looks like removing ComputedVisibility and copyless yield similar sized wins and the partial_cmp change barely registers. I dont think we should directly expose alloc().init() where possible. Instead RenderPhase::add and BufferVec::push should do it internally. Lets add those to the impl currently on this branch and call it a day!

Seems like frustum culling starts becoming a win again in many_sprites on my machine when doubling this value

Note that Sprites don't currently have frustum culling. Visibility just gets mirrored to ComputedVisibility.

[rparrett]

That explains a great deal.

[cart]

Also: @Davier let me know if you won't be able to add the optimizations to this pr by the end of the day. Happy to pick up that work.

[Davier]

@cart done! I'll go to sleep now though, please handle it if I broke anything 😄

[cart]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-android
• build (nightly, ubuntu-latest)
• build (stable, macos-latest)
• build (stable, ubuntu-latest)
• build (stable, windows-latest)
• build-wasm (nightly, ubuntu-latest)
• build-wasm (stable, ubuntu-latest)
• check-benches
• check-doc
• check-markdown-links
• check-missing-examples-in-docs
• check-unused-dependencies
• ci
• markdownlint
• run-examples