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

Add ruler mode to 3D #100162

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Conversation

ryevdokimov
Copy link
Contributor

@ryevdokimov ryevdokimov commented Dec 8, 2024

Implements and closes: godotengine/godot-proposals#8839

Starts on mouse down and is cleared on mouse up. Uses collision detection functionality from #96740 to snap to objects.

2024-12-11.19-08-37.mp4

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected. This is a great start 🙂

Some feedback:

  • I suggest renaming the Measure tool to Ruler (also with its internal enum name), so that it matches the name in the 2D editor. This should also be done for the shortcut name.
  • Draw the distance text with a black outline, and use the bold editor font with a slightly increased font size. Doing this will help a lot with visibility on mixed-color backgrounds, which can be an issue right now:

image

See the 2D editor for reference:

image

  • Display a the unique with a m suffix for "meters" (e.g. 123.456 m), as 3D units are meters in Godot.
  • Round the display to 3 decimals using String.pad_decimals(), so there's always 3 decimals displayed (even if technically not needed in some cases). This prevents the text from jumping around as the ruler moves.
  • The ruler line occasionally disappears depending on which direction it's going towards:
ruler_disappear.mp4

This issue depends on the camera angle. It never occurs if the camera is looking straight down by pressing KP 7.

  • You could look into replicating the rectangle and angle measurements from the 2D ruler in the 3D ruler, but this can be left for a future PR.

@ryevdokimov
Copy link
Contributor Author

  • I suggest renaming the Measure tool to Ruler (also with its internal enum name), so that it matches the name in the 2D editor. This should also be done for the shortcut name.

Done.

  • Draw the distance text with a black outline and use the bold editor font with a slightly increased font size. Doing this will help a lot with visibility on mixed-color backgrounds, which can be an issue right now:sin
    itor for reference:

Done.

  • Display a the unique with a m suffix for "meters" (e.g. 123.456 m), as 3D units are meters in Godot.

Done.

  • Round the display to 3 decimals using String.pad_decimals(), so there's always 3 decimals displayed (even if technically not needed in some cases). This prevents the text from jumping around as the ruler moves.

Done.

  • The ruler line occasionally disappears depending on which direction it's going towards:

I can't reproduce this. Does the target mesh have a collision node associated with it? At the moment the end points snap to only collisions, since snapping to geometry is a different animal.

  • You could look into replicating the rectangle and angle measurements from the 2D ruler in the 3D ruler, but this can be left for a future PR.

Will probably save this for another PR since there is a lot of different avenues this kind of functionality can take. Persisting measurements, more than one measurement on the screen, measuring volume, angles, etc. I think it makes sense to wait for feedback from the public on this initial step.

@ryevdokimov
Copy link
Contributor Author

See video below:

In one instance the line does go behind the mesh and extends depth-wise in the viewport, but this is expected.

2024-12-09.10-32-03.mp4

@ryevdokimov ryevdokimov changed the title Add measure mode to 3D Add ruler mode to 3D Dec 9, 2024
@Calinou
Copy link
Member

Calinou commented Dec 9, 2024

In one instance the line does go behind the mesh and extends depth-wise in the viewport, but this is expected.

I see, that makes sense. In this case, I'd do what the 3D selection box does:

  • Draw a solid line without the No Depth Test flag (as is done now).
  • Draw a transparent line at the same coordinates with No Depth Test flag, with the same color but lower opacity.

This way, you can see the line going through objects at reduced opacity, giving it a x-ray look.

@ryevdokimov
Copy link
Contributor Author

Done.

2024-12-09.15-55-54.mp4

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Works great now 🙂

image

Code looks good to me.

ruler_label = memnew(Label);
ruler_label->add_theme_color_override(SceneStringName(font_color), Color(1.0, 0.9, 0.0, 1.0));
ruler_label->add_theme_color_override("font_outline_color", Color(0.0, 0.0, 0.0, 1.0));
ruler_label->add_theme_constant_override("outline_size", 4);
Copy link
Member

@Calinou Calinou Dec 10, 2024

Choose a reason for hiding this comment

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

Scale the outline for hiDPI displays (otherwise, the outline looks thinner relative to the font size):

Suggested change
ruler_label->add_theme_constant_override("outline_size", 4);
ruler_label->add_theme_constant_override("outline_size", 4 * EDSCALE);

Comment on lines 3154 to 3160
if (ruler->is_inside_tree()) {
if (!ruler_start_point->is_visible()) {
ruler_end_point->set_global_position(ruler_start_point->get_global_position());
ruler_start_point->set_visible(true);
ruler_end_point->set_visible(true);
ruler_label->set_visible(true);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (ruler->is_inside_tree()) {
if (!ruler_start_point->is_visible()) {
ruler_end_point->set_global_position(ruler_start_point->get_global_position());
ruler_start_point->set_visible(true);
ruler_end_point->set_visible(true);
ruler_label->set_visible(true);
}
if (ruler->is_inside_tree() && !ruler_start_point->is_visible()) {
ruler_end_point->set_global_position(ruler_start_point->get_global_position());
ruler_start_point->set_visible(true);
ruler_end_point->set_visible(true);
ruler_label->set_visible(true);

@@ -35,9 +35,11 @@
#include "editor/plugins/editor_plugin.h"
#include "editor/plugins/node_3d_editor_gizmos.h"
#include "editor/themes/editor_scale.h"
#include "scene/3d/mesh_instance_3d.h"
Copy link
Member

Choose a reason for hiding this comment

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

Forward declare this

@ryevdokimov
Copy link
Contributor Author

Suggestions applied.

@Repiteo Repiteo merged commit a5cf24c into godotengine:master Dec 11, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 11, 2024

Thanks!

@ryevdokimov ryevdokimov deleted the measure-mode-3d branch December 11, 2024 23:42
@ryevdokimov
Copy link
Contributor Author

Does the target mesh have a collision node associated with it? At the moment the end points snap to only collisions, since snapping to geometry is a different animal.

This PR is a way to solve this: #100415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Ruler Mode for 3D
5 participants