Skip to content

Bug fix in Gizmo grid #19697

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

theotherphil
Copy link
Contributor

@theotherphil theotherphil commented Jun 17, 2025

Objective

Fix #19480

Solution

Fix calculation of line counts in each direction

Testing

Added grids to the gizmos screen of testbed/example_3d

image

@theotherphil
Copy link
Contributor Author

theotherphil commented Jun 17, 2025

What's the best way to add tests for this @mockersf ? I remember talk of you having automated testing based on running examples but I failed to find the docs for this.

Also: how does anyone feel about changing the outer_edges_* names? I found it confusing that in 2d outer_edges_x means draw the top and bottom and outer_edges_y means draw the left and right - I would have expected the other way around. Maybe there are clearer names available (I claim, while conspicuously not suggesting any).

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Gizmos Visual editor and debug gizmos S-Needs-Help The author needs help finishing this PR. labels Jun 18, 2025
@rparrett
Copy link
Contributor

You could add to Scene::Gizmos in

https://github.com/bevyengine/bevy/blob/main/examples/testbed/2d.rs
https://github.com/bevyengine/bevy/blob/main/examples/testbed/3d.rs

How does anyone feel about changing the outer_edges_* names?

It feels pretty ambiguous to me, and either interpretation could be plausibly correct. I also can't think of better names, but I do think that perhaps the docs could potentially be improved with language like "parallel to the x axis" instead of "along the x axis."

@theotherphil
Copy link
Contributor Author

Aha! Thanks, that's what I was failing to find.

I don't have strong opinions on the names - I could just clarify the docs. I considered outer_(left_right|top_bottom|front_back) but I'm not sure if that's better.

Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19697

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

1 similar comment
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19697

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@theotherphil theotherphil added the M-Deliberate-Rendering-Change An intentional change to how tests and examples are rendered label Jun 26, 2025
@theotherphil theotherphil changed the title [DRAFT] Bug fix in Gizmo grid Bug fix in Gizmo grid Jun 26, 2025
@theotherphil theotherphil added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Needs-Help The author needs help finishing this PR. labels Jun 26, 2025
@theotherphil
Copy link
Contributor Author

theotherphil commented Jun 26, 2025

The CI failure is in bevy_ecs, which this PR doesn't touch.

Edit: another merge of master fixes that issue and breaks even more unrelated crates! Hopefully if I just keep merging master then CI will be green eventually...

Edit2: hurrah! Now green.

@theotherphil theotherphil requested a review from rparrett June 26, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Gizmos Visual editor and debug gizmos C-Bug An unexpected or incorrect behavior M-Deliberate-Rendering-Change An intentional change to how tests and examples are rendered S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grid gizmo with either x or y outer edges doesn't render correctly
3 participants