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

Immediate Mode Line/Gizmo Drawing #6529

Merged
merged 72 commits into from
Mar 20, 2023
Merged

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Nov 9, 2022

Objective

Add a convenient immediate mode drawing API for visual debugging.

Fixes #5619
Alternative to #1625
Partial alternative to #5734

Based off https://github.com/Toqozz/bevy_debug_lines with some changes:

  • Simultaneous support for 2D and 3D.
  • Methods for basic shapes; circles, spheres, rectangles, boxes, etc.
  • 2D methods.
  • Removed durations. Seemed niche, and can be handled by users.
Performance

Stress tested using Bevy's recommended optimization settings for the dev profile with the
following command.

cargo run --example many_debug_lines \
    --config "profile.dev.package.\"*\".opt-level=3" \
    --config "profile.dev.opt-level=1"

I dipped to 65-70 FPS at 300,000 lines
CPU: 3700x
RAM Speed: 3200 Mhz
GPU: 2070 super - probably not very relevant, mostly cpu/memory bound

Fancy bloom screenshot

Screenshot_20230207_155033

Changelog

  • Added GizmoPlugin
  • Added Gizmos system parameter for drawing lines and wireshapes.

TODO

  • Update changelog
  • Update performance numbers
  • Add credit to PR description

Future work

  • Cache rendering primitives instead of constructing them out of line segments each frame.
  • Support for drawing solid meshes
  • Interactions. (See bevy_mod_gizmos)
  • Fancier line drawing. (See bevy_polyline)
  • Support for RenderLayers
  • Display gizmos for a certain duration. Currently everything displays for one frame (ie. immediate mode)
  • Changing settings per drawn item like drawing on top or drawing to different RenderLayers

Co-Authored By: @lassade felipe.jorge.pereira@gmail.com
Co-Authored By: @The5-1 agaku@hotmail.de
Co-Authored By: @Toqozz toqoz@hotmail.com
Co-Authored By: @nicopap nico@nicopap.ch

@nicopap nicopap self-requested a review November 9, 2022 21:13
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen C-Testing A change that impacts how we test Bevy or how users test their apps X-Controversial There is active debate or serious implications around merging this PR labels Nov 9, 2022
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bikeshedding for a better long term name: bevy_gizmos? It'd fit better into the context of a user-facing editor.

crates/bevy_debug_draw/src/debug_draw.rs Outdated Show resolved Hide resolved
crates/bevy_debug_draw/src/debug_draw.rs Outdated Show resolved Hide resolved
crates/bevy_internal/Cargo.toml Show resolved Hide resolved
crates/bevy_debug_draw/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_debug_draw/src/lib.rs Outdated Show resolved Hide resolved
@james7132
Copy link
Member

Credit should be given to @lassade, @The5-1, and @Toqozz for their prior work in this space.

Could you add Co-Authored By: <EMAIL> to the PR description? Bors will merge it in with that in the final commit to main.

Separately, @lassade's existing implementation for bevy_gizmos seems to also have support for filling in the internal space with a flat color. We can add it in a follow-up PR or leave it be.

crates/bevy_debug_draw/src/debug_draw.rs Outdated Show resolved Hide resolved
crates/bevy_debug_draw/src/debug_draw.rs Outdated Show resolved Hide resolved
@The5-1
Copy link

The5-1 commented Nov 14, 2022

Oh I totally missed this here, good stuff 👌

Was still struggling to get my old attempt ported to 0.8 and then 0.9 hit ;p
Gotta take a closer look at this tomorrow to see how its done!

@nicopap
Copy link
Contributor

nicopap commented Nov 14, 2022

Note that I also heavily contributed to Toqozz/bevy_debug_lines. I did the port to the new renderer for 0.6, it was basically an entire rewrite.

@alice-i-cecile
Copy link
Member

Once #5303 by @IceSentry is merged, should we reuse the wireframe material here?

@tim-blackbird
Copy link
Contributor Author

Once #5303 by @IceSentry is merged, should we reuse the wireframe material here?

This PR needs colors to be per vertex with an option to disable depth testing + a 2d version, so that's not an option, and the Material(2d) traits and AsBindGroup are currently not flexible enough to support this.

@james7132 james7132 mentioned this pull request Mar 8, 2023
@github-actions
Copy link
Contributor

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

1 similar comment
@github-actions
Copy link
Contributor

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work!

Ultimately I think we'll want to explore rendering using methods similar to Freya Holmers Shapes lib, as normal "mesh line drawing" has limitations when it comes to thickness, anti-aliasing + quality, etc. Definitely a "hard problem" though. I'm glad we started with the mesh-line approach so we can start building tools + visual debuggers now.

) -> Result<RenderPipelineDescriptor, SpecializedMeshPipelineError> {
let mut shader_defs = Vec::new();
shader_defs.push("GIZMO_LINES_3D".into());
shader_defs.push(ShaderDefVal::Int(
Copy link
Member

@cart cart Mar 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not something to fix here, but the fact that we need to manually plug these values in, despite not using them in the shader, is unfortunate.

@cart cart enabled auto-merge March 20, 2023 20:45
@cart cart added this pull request to the merge queue Mar 20, 2023
@cart cart merged commit 6a85eb3 into bevyengine:main Mar 20, 2023
@cart
Copy link
Member

cart commented Mar 20, 2023

Turns out this was based on @Toqozz's bevy_debug_lines crate, which is licensed under the mit license only. This will need to be re-licensed if we're going to use it in Bevy.

@Toqozz: can you write "I agree to re-license the code from bevy_debug_lines used in this PR to the dual MIT/Apache-2.0 license Bevy uses" here if you agree to the re-license? No pressure btw. It is your code and you can do whatever you want with it. We will revert if we don't hear from you / if the answer is no.

@Toqozz
Copy link

Toqozz commented Mar 20, 2023

I agree to re-license the code from bevy_debug_lines used in this PR to the dual MIT/Apache-2.0 license Bevy uses.

I didn't even realise that the license was limiting.

@cart
Copy link
Member

cart commented Mar 20, 2023

Much appreciated! Thank you for building cool stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Testing A change that impacts how we test Bevy or how users test their apps S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved rendering (And documentation) for 2d and 3d Lines