don't use bevy_pbr for base bevy_gizmos plugin #17581

rambip · 2025-01-28T15:35:04Z

Objective

This PR enables bevy_gizmos to be used without bevy_pbr, for user who want to create their custom mesh rendering logic.

It can also be useful for user who just want to use bevy for drawing lines (why not).

This work is part of a bigger effort to make the bevy rendering pipeline more modular. I would like to contribute an exemple to render custom meshes without bevy_pbr. Something like this

Solution

Now, bevy_pbr is an optional dependency, and used only to debug lights.

I query the ViewUniforms manually, instead of using bevy_pbr to get the heavy MeshViewLayout

Testing

I'm not used to testing with bevy at all, but I was able to use successfully in my project.
It might break for some different mesh pipelines, but I don't think so.

Showcase

So nice ...

Migration Guide

I don't think there is any breaking change

Remaining work

Before merging it, it would be useful to:

rewrite the pipeline_2d.rs logic to remove the bevy_sprite depedency too
move view.rs to bevy_render, so that it can be used in a more modular way.
~~- include the most recent changes from 0.16~~

IceSentry

I like where this is going, but needs a few changes.

IceSentry · 2025-01-28T18:35:43Z

crates/bevy_gizmos/src/lib.rs

@@ -143,8 +141,12 @@ const LINE_SHADER_HANDLE: Handle<Shader> = Handle::weak_from_u128(74148126892380
 const LINE_JOINT_SHADER_HANDLE: Handle<Shader> = Handle::weak_from_u128(1162780797909187908);

 /// A [`Plugin`] that provides an immediate mode drawing api for visual debugging.
+/// It does *not* require `bevy_pbr`


That comment isn't necessary.

crates/bevy_gizmos/src/view.rs

IceSentry · 2025-01-28T19:03:21Z

crates/bevy_gizmos/src/view.rs

+) {
+    if let Some(view_binding) = view_uniforms.uniforms.binding() {
+        for entity in &views {
+            let entries = DynamicBindGroupEntries::new_with_indices(((0, view_binding.clone()),));


This can be moved to be before the loop.

IceSentry · 2025-01-28T19:03:53Z

crates/bevy_gizmos/src/view.rs

+    layout: Res<OnlyViewLayout>,
+    views: Query<Entity>,
+) {
+    if let Some(view_binding) = view_uniforms.uniforms.binding() {


I'd prefer a let-else with early return but that's a tiny nitpick and purely optional.

Co-authored-by: IceSentry <[email protected]>

IceSentry · 2025-01-28T21:15:21Z

crates/bevy_gizmos/src/view.rs

+        DynamicBindGroupEntries, DynamicBindGroupLayoutEntries, ShaderStages,
+    },
+    renderer::RenderDevice,
+    view::{ViewUniform, ViewUniforms},


This should fix CI

Suggested change

view::{ViewUniform, ViewUniforms},

view::{ExtractedView, ViewUniform, ViewUniforms},

rambip · 2025-01-28T21:26:50Z

I did the same thing for pipeline_2d, pretty straightforward.

Now bevy_sprite is not a dependency anymore and bevy_pbr is optional.

Right now pipeline3d and pipeline2d look very similar, maybe they share some functionality that could be merged.

I would love view.rs to move to bevy_render so that I can fill a PR to rewrite the custom_mesh_pipeline example to work without bevy_pbr.

IceSentry · 2025-01-28T21:46:36Z

I'd be more than happy to review and approve a PR that moves shared things from 2d/3d to bevy_render. We want to break appart bevy_render because it's giant, but moving shared things to it still makes sense.

I'd also be happy to review and approve a PR that moves the OnlyViewLayout thing to bevy render, but I think this could be done after this current PR is merged.

rambip · 2025-01-28T21:49:22Z

Ok, I prepare a new PR for

1. moving OnlyViewLayout to bevy_render
1. updating the example.

Do you know where in the bevy_render crate the view stuff should go ?

IceSentry · 2025-01-28T21:55:07Z

Answered in discord to keep this discussion relevant to this PR

rambip · 2025-02-21T13:50:21Z

Is there something missing to merge this issue ?

IceSentry · 2025-02-21T18:38:54Z

A second review. Bevy needs 2 reviews per PR.

I'd suggest resolving all conversations just to make sure reviewer don't skip the PR because of it.

alice-i-cecile · 2025-03-09T22:31:52Z

Sorry @rambip; we needed a second reviewer. This looks sensible to me: if you get merge conflicts resolved I'll merge this ASAP.

rambip · 2025-03-10T07:15:44Z

I suspect that the issue in #17945 would be automatically fixed by my proposed solution, so I will merge with my changes.
I'm not 100% sure though.

)" This reverts commit b574599.

# Objective - #17581 broke gizmos - Fixes #18325 ## Solution - Revert #17581 - Add gizmos to testbed ## Testing - Run any example with gizmos, it renders correctly

don't use bevy_pbr for base bevy_gizmos plugin

438eb0c

alice-i-cecile requested a review from atlv24 January 28, 2025 18:23

alice-i-cecile added the D-Straightforward Simple bug fixes and API improvements, docs, test and examples label Jan 28, 2025

fix errors after sync with main (types and imports)

e044f6d

rambip force-pushed the main branch from b1d3155 to e044f6d Compare January 28, 2025 18:26

cargo fmt

e89c9b1

IceSentry suggested changes Jan 28, 2025

View reviewed changes

rambip and others added 4 commits January 28, 2025 21:45

change comment style

68d9a4a

Co-authored-by: IceSentry <[email protected]>

Filter only the entities that have a view

7df5a3a

Co-authored-by: IceSentry <[email protected]>

remove useless comment

5af05c1

improve readability of prepare_view

3b9c8af

IceSentry reviewed Jan 28, 2025

View reviewed changes

rambip added 2 commits January 28, 2025 22:22

fix import

19a932d

remove bevy_sprite dependency from bevy_gizmos

364e0ce

fix documentation comment

495ff9e

IceSentry approved these changes Jan 28, 2025

View reviewed changes

github-actions bot mentioned this pull request Feb 8, 2025

Missing Documentation - Tracker Issue makspll/bevy_mod_scripting#255

Open

5 tasks

rambip mentioned this pull request Feb 12, 2025

MeshRenderPlugin should be usable as a standalone plugin. #17536

Open

alice-i-cecile approved these changes Mar 9, 2025

View reviewed changes

alice-i-cecile 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 Mar 9, 2025

atlv24 approved these changes Mar 9, 2025

View reviewed changes

rambip added 2 commits March 10, 2025 08:18

Merge branch 'main' into main

4d11b0b

Merge branch 'main' into main

7eb4a3b

alice-i-cecile added this pull request to the merge queue Mar 10, 2025

Merged via the queue into bevyengine:main with commit b574599 Mar 10, 2025
35 checks passed

mockersf added a commit to mockersf/bevy that referenced this pull request Mar 15, 2025

Revert "don't use bevy_pbr for base bevy_gizmos plugin (bevyengine#17581

61d5590

)" This reverts commit b574599.

This was referenced Mar 15, 2025

Revert "don't use bevy_pbr for base bevy_gizmos plugin" #18327

Merged

Example 3d_viewport_to_world: gizmo is displayed in the wrong position #18325

Closed

ElliottjPierce mentioned this pull request Mar 26, 2025

Clean up incomplete migration guides tracking issue bevyengine/bevy-website#2014

Open

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

don't use bevy_pbr for base bevy_gizmos plugin #17581

don't use bevy_pbr for base bevy_gizmos plugin #17581

rambip commented Jan 28, 2025 •

edited

Loading

IceSentry left a comment

IceSentry Jan 28, 2025

IceSentry Jan 28, 2025

IceSentry Jan 28, 2025

IceSentry Jan 28, 2025

rambip commented Jan 28, 2025

IceSentry commented Jan 28, 2025

rambip commented Jan 28, 2025

IceSentry commented Jan 28, 2025

rambip commented Feb 21, 2025

IceSentry commented Feb 21, 2025 •

edited

Loading

alice-i-cecile commented Mar 9, 2025

rambip commented Mar 10, 2025

	view::{ViewUniform, ViewUniforms},
	view::{ExtractedView, ViewUniform, ViewUniforms},

don't use bevy_pbr for base bevy_gizmos plugin #17581

don't use bevy_pbr for base bevy_gizmos plugin #17581

Conversation

rambip commented Jan 28, 2025 • edited Loading

Objective

Solution

Testing

Showcase

Migration Guide

Remaining work

IceSentry left a comment

Choose a reason for hiding this comment

IceSentry Jan 28, 2025

Choose a reason for hiding this comment

IceSentry Jan 28, 2025

Choose a reason for hiding this comment

IceSentry Jan 28, 2025

Choose a reason for hiding this comment

IceSentry Jan 28, 2025

Choose a reason for hiding this comment

rambip commented Jan 28, 2025

IceSentry commented Jan 28, 2025

rambip commented Jan 28, 2025

IceSentry commented Jan 28, 2025

rambip commented Feb 21, 2025

IceSentry commented Feb 21, 2025 • edited Loading

alice-i-cecile commented Mar 9, 2025

rambip commented Mar 10, 2025

rambip commented Jan 28, 2025 •

edited

Loading

IceSentry commented Feb 21, 2025 •

edited

Loading