X Tutup
Skip to content

Visualize MarginContainer margins when selected#111095

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
Meorge:feat/margincontainer-viz
Oct 14, 2025
Merged

Visualize MarginContainer margins when selected#111095
Repiteo merged 1 commit intogodotengine:masterfrom
Meorge:feat/margincontainer-viz

Conversation

@Meorge
Copy link
Contributor

@Meorge Meorge commented Sep 30, 2025 •
edited
Loading

Closes godotengine/godot-proposals#8961 .

This PR adds lines depicting the margins for a MarginContainer, when it is selected:

CleanShot 2025-09-30 at 16 36 32@2x

@Meorge Meorge requested a review from a team September 30, 2025 22:26
@KoBeWi KoBeWi added this to the 4.x milestone Sep 30, 2025
@KoBeWi
Copy link
Member

KoBeWi commented Sep 30, 2025

This should be done via a dedicated EditorPlugin for MarginContainer instead of hard-coding it in CanvasItemEditor.

@Meorge
Copy link
Contributor Author

Meorge commented Sep 30, 2025

Ah alrighty, thanks! I'll look into moving the code there 😄

@Meorge Meorge force-pushed the feat/margincontainer-viz branch from 1406401 to ac80478 Compare September 30, 2025 23:34
@Meorge Meorge requested a review from a team as a code owner September 30, 2025 23:34
@AdriaandeJongh
Copy link
Contributor

AdriaandeJongh commented Oct 1, 2025

I think this is a fantastic UX improvement, but the PR needs a bit more work I think? can I suggest to make the margin lines not the same color as the bounding box, perhaps a darker or more transparent orange, and have the lines stop at the margin edge instead of at the edge of the control?
Screenshot 2025-10-01 at 10 13 18

@Meorge
Copy link
Contributor Author

Meorge commented Oct 1, 2025

I can certainly implement that later today!

Something of a side note: I noticed while working on this that the orange color is not theme-based or customizable, but instead hard-coded in. It's out of scope for this PR but I could see people wanting to be able to customize it for different themes. As such, it might be good to have those colors be made theme-dependent.

At the very least, moving them from an inline definition to a constant definition somewhere that various code files could reference would be nice – I'm currently just copying the color definition from its original location, which means the two of them could fall out of sync easily...

@Meorge
Copy link
Contributor Author

Meorge commented Oct 1, 2025

I've changed the rendering of the inner space:

CleanShot 2025-10-01 at 13 24 53@2x CleanShot 2025-10-01 at 13 25 02@2x

Unfortunately I have found that it no longer updates the visualization automatically, when one of the theme properties is changed; the user has to force a redraw by moving their mouse over the viewport, or do something similar. Back in the CanvasItemEditor hardcoded version, it looks like the redraw-queueing was done as a result of the process notification. Would that be the best approach to take here? In retrospect it seems like it could be a bit demanding

KoBeWi
KoBeWi reviewed Oct 3, 2025
View reviewed changes
KoBeWi
KoBeWi reviewed Oct 3, 2025
View reviewed changes
@KoBeWi
Copy link
Member

KoBeWi commented Oct 3, 2025 •
edited
Loading

Modifying margins does not update viewport immediately. You could connect MarginContainer's draw signal to update_viewport(), as changing margins redraws the container.

KoBeWi
KoBeWi reviewed Oct 3, 2025
View reviewed changes
editor/scene/gui/margin_container_editor_plugin.cpp Outdated
return;
}

bool item_locked = margin_container->has_meta("_edit_lock_");
Copy link
Member

@KoBeWi KoBeWi Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if it's worth it to mimic 2D editor colors that much. Maybe you could use a different color altogether, like the one used by Camera2D limits. Then you could ignore locking etc.

Although that might cause margins to appear above the box selection, so not sure 🤔

Copy link
Contributor Author

@Meorge Meorge Oct 3, 2025

Choose a reason for hiding this comment

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

I can certainly give that a try! Admittedly I have found in the past that other editors' lines have been hard to see on my screen, while the bright orange from the bounding box editor is much easier to see. But there's no harm in giving it a go, and perhaps those problems can be addressed in a separate proposal.

KoBeWi
KoBeWi reviewed Oct 3, 2025
View reviewed changes
@Meorge Meorge force-pushed the feat/margincontainer-viz branch from 1916bcd to 8098d31 Compare October 3, 2025 16:58
@Meorge
Copy link
Contributor Author

Meorge commented Oct 3, 2025

Thanks for the code review! I've implemented the changes. It looks pretty good to me, with the minor exception that there is a moment of lag between selecting a MarginContainer and its margins actually being drawn. I haven't had the chance to test if this is also behavior that happens with other editor plugins, though:

Godot.MarginContainer.Visualizer.mp4

@arkology
Copy link
Contributor

arkology commented Oct 3, 2025 •
edited
Loading

I haven't had the chance to test if this is also behavior that happens with other editor plugins, though:

I saw the same thing with the 2D polygon editor plugin about 3 hours ago.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 3, 2025 •
edited
Loading

there is a moment of lag between selecting a MarginContainer and its margins actually being drawn.

If you are using dev build, that's because of inspector loading new node.

KoBeWi
KoBeWi approved these changes Oct 3, 2025
View reviewed changes
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Looks fine. Not sure about the yellow color replacing default the selection box (when margins are 0), but it's probably ok.

Image

@Meorge
Visualize MarginContainer margins when selected
36c7bbb 
This time as an EditorPlugin!

Improve rendering of inner area of MarginContainer

Decrease thickness and opacity of lines, and render margins as a rectangle rather than lines extending to the edges of the bounding box

Apply suggestions from code review

Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>

Use `get_margin_size`, change color of viz border, and trigger redraw on MarginContainer's draw signal
@Meorge Meorge force-pushed the feat/margincontainer-viz branch from 8098d31 to 36c7bbb Compare October 3, 2025 18:42
@Repiteo Repiteo modified the milestones: 4.x, 4.6 Oct 14, 2025
@Repiteo Repiteo merged commit e6768b5 into godotengine:master Oct 14, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 14, 2025

Thanks!

@dalexeev
Copy link
Member

dalexeev commented Oct 14, 2025

I like this change, it reminds me of the functionality found in browser developer tools. However, I'd like to point out that this feature could potentially be extended to other controls (CenterContainer, GridContainer, BoxContainer, FlowContainer, and so on). In that case, we should rename the plugin class, as MarginContainerEditorPlugin is too specific and it's unlikely to make sense to create separate plugins for each control.

@kevinlam508 kevinlam508 mentioned this pull request Nov 12, 2025
Closed
@Meorge Meorge mentioned this pull request Nov 29, 2025
Open
@Meorge Meorge deleted the feat/margincontainer-viz branch February 28, 2026 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KoBeWi KoBeWi KoBeWi approved these changes

Assignees

No one assigned

Labels

enhancement topic:editor topic:gui

Projects

None yet

Milestone

4.6

Development

Successfully merging this pull request may close these issues.

Visually show MarginContainer margin sizes on Editor

6 participants

@Meorge @KoBeWi @AdriaandeJongh @arkology @Repiteo @dalexeev
X Tutup