-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Debug Draw #1625
Debug Draw #1625
Conversation
Cargo.toml
Outdated
@@ -51,6 +51,7 @@ bevy_winit = ["bevy_internal/bevy_winit"] | |||
trace_chrome = ["bevy_internal/trace_chrome"] | |||
trace = ["bevy_internal/trace"] | |||
wgpu_trace = ["bevy_internal/wgpu_trace"] | |||
debug_draw = ["bevy_internal/debug_draw"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug_draw = ["bevy_internal/debug_draw"] | |
debug_draw = ["bevy_internal/bevy_debug_draw"] |
I think it must be named this, to succesfully compile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem to help. The other crates do not require a bevy_
prefix either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is neccesary here, as you added it to bevy_internal
this way:
bevy_debug_draw = { path = "../bevy_debug_draw", optional = true, version = "0.4.0" }
See the bevy_
prefix?
Those that don't need it, also don't have the prefix:
wgpu_trace = ["bevy_wgpu/trace"]
trace = [ "bevy_app/trace", "bevy_ecs/trace" ]
trace_chrome = [ "bevy_log/tracing-chrome" ]
For me applying that change allows cargo to compile, though there are still several compile errors:
- The
With<>
I suggested below still needs to be imported. - The trait functions in the gizmo file, do not need a
pub
But at least it's progress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have sent you a PR on your Fork, with all the fixes. The5-1#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @MinerSebas .
I merged the PR on my fork and have the same behavior.
Will try and find the bug.
fix unused returned entity for debug_draw mesh entity Co-authored-by: MinerSebas <[email protected]>
Fix unnecessary DebugDraw3DComponent query. Co-authored-by: MinerSebas <[email protected]>
Hi you may want to also checkout my gizmos crate https://github.com/lassade/bevy_gizmos, its already usable, it's missing one thing or other; Features
|
Fix compilation
Thanks, I added the repos to the initial post. My wish is to ultimately have the ease of use of a |
Thats the ahole point of my crate, but it also needed to be non intrusive, this means using only |
@lassade I am having issues running your crate. |
Sorry for that, can run with my animations bevy branch, here |
@lassade your minimal call to draw something seems to look like: gizmos.draw(|context| {
// If gizmos are disabled this won't run
let points = generete_your_expensive_spline(too_many_points);
context.line_list(point);
}) In my implementation so far it looks like this: debug_draw.line(vec3_a, vec3_b, Color::ORANGE ); I am in favor of my shorter syntax. Your crate already supports more shapes and filled drawing and has the potential for a general drawing crate that is not limited to only debugging but may ultimately be useful for actual, pretty drawing in release builds or the editor. If we want to get something usable for (with the sole purpose of quick debugging) we could merge this PR right now. So how to proceed with this? If we opt for the current solution I would:
|
I like this approach. Small PRs with basic useful functionality is a good way to go. |
Yeah, I'd love your input on the primitives, especially if we want to use them in this PR eventually. |
Then the next steps are:
|
I'm not sure if this is bevy territory, but Vulkan has the My thoughts are if we can (without huge effort) have line thickness options then we should, as they're useful in 3d for assessing the depth of the line. This also makes the drawing a little better for prototype art, but that's mostly a moot point. |
I do not believe that wgpu-rs supports lineWidth. I know that gfx-rs has it for supported targets but it'll require some work to get this implemented as an extension in wgpu-rs. |
@alice-i-cecile Ah! I didn't see my name in the list so I figured it's fine, Went ahead and gave my OK too ;) |
Awesome, thank you! Yeah, you didn't have any merged PRs at the time so it wasn't automatically added :) |
@The5-1 this is super cool; are you interested in continuing this work now that we have more merge bandwidth? If so, rebasing would be appreciated so we can review more effectively. |
@alice-i-cecile I have not dabbed into Bevy since half a year (was waiting for progress on the UI front), so I wound need to refresh my knowledge and get the hang of what changed with the new versions. If any of the other posters/interested on this topic are more up to speed, they can also have a go at it (if that even is technically possible, given it is my PR). But otherwise, I am keen on getting back into it, so fixing this could be a good re-entry point ;) |
Sounds good! I expect #rendering-dev on Discord will have plenty of feedback and thoughts.
Yep, they can either make a PR to your branch, or if you're particularly overwhelmed, create a new PR from a fork off your branch, preserving your commit history (and credit). |
Closing in favor of #6529. Going to try to get this resolved before 0.11 releases. |
# 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. <details> <summary>Performance</summary> Stress tested using Bevy's recommended optimization settings for the dev profile with the following command. ```bash 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 </details> <details> <summary>Fancy bloom screenshot</summary> ![Screenshot_20230207_155033](https://user-images.githubusercontent.com/29694403/217291980-f1e0500e-7a14-4131-8c96-eaaaf52596ae.png) </details> ## Changelog * Added `GizmoPlugin` * Added `Gizmos` system parameter for drawing lines and wireshapes. ### TODO - [ ] Update changelog - [x] Update performance numbers - [x] 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](https://github.com/LiamGallagher737/bevy_mod_gizmos)) - Fancier line drawing. (See [bevy_polyline](https://github.com/ForesightMiningSoftwareCorporation/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 <[email protected]> Co-Authored By: @The5-1 <[email protected]> Co-Authored By: @Toqozz <[email protected]> Co-Authored By: @nicopap <[email protected]> --------- Co-authored-by: Robert Swain <[email protected]> Co-authored-by: IceSentry <[email protected]> Co-authored-by: Carter Anderson <[email protected]>
As suggested by @TheRawMeatball on discord, this PR is to coordinate efforts for a Debug Draw crate.
Primitive shapes for drawing should ideally be supplied by #1621.
The Idea is to have a convenient way to quickly visualize something in the viewport.
Ideally on par with the ease of calling
println!
but in order to draw visual cues.The drawing happens in "immediate mode", meaning the lines are removed after a frame.
This requires some discussion on how a robust API should look like.
The current approach is only intended for visual debugging.
Once we establish a crate for a more general drawing solution like the ones from @lassade 's or @Toqozz 's here
we could rework this immediate mode debug draw crate to actually use the proper drawing supplied by such a crate.
Minimal Example:
Working Example:
run with
cargo run --package bevy --example debug_draw_3d --features="debug_draw"
The code in question can be seen here.
https://github.com/bevyengine/bevy/pull/1625/files#diff-c501be256b1a5d25b2ea56f6813ba342eef03e471d38e12aab026015f3365bebR17-R35
Some walktrough of the current implementation:
A Resource provides functions to draw lines.
Calling these functions pushes vertices into the resource.
At the end of the frame this vertex data is copied over into a entity's Mesh component while the data on the resource is cleared.
The entity then draws the mesh as per usual.
The Line simply uses the line primitive type rather than drawing nice, anti-aliased lines.
This is most likely sufficient for debugging purposes and keeps the implementation really simple.
Since the data is cleared every frame the lines do not persist (immediate mode debug draw).
I feel this plays quite well with most systems updating every frame anyways.
For other use cases lines with a certain lifetime or persistent ones may be implemented (e.g. using Line structs with a lifetime field).
It should be easy enough to extend the system for these cases and to accept triangles or other primitives as well.
I am not quite sure if my general approach of querying a singular entity to update its mesh is sound.