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

Improve UV editor zoom behavior #83731

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

aXu-AP
Copy link
Contributor

@aXu-AP aXu-AP commented Oct 21, 2023

Implements partially godotengine/godot-proposals#4118

  • Scrolling focuses towards mouse like in canvas editor (main 2D editor view).
  • Center UV editor view on opening.
  • Adjust UV editor scroll area to include polygons larger than texture.

Also throughout the code zoom level was get by 2 different methods: uv_zoom->get_value() and uv_draw_zoom. I changed them all to the latter for consistency (single source of truth).

@aXu-AP aXu-AP requested a review from a team as a code owner October 21, 2023 14:28
@AThousandShips AThousandShips added this to the 4.x milestone Oct 21, 2023
@aXu-AP aXu-AP marked this pull request as draft November 2, 2023 19:59
@aXu-AP aXu-AP marked this pull request as ready for review November 3, 2023 16:06
@aXu-AP
Copy link
Contributor Author

aXu-AP commented Nov 3, 2023

Updated with lessons I learned when making #84353, mainly moved some stuff around to avoid some edge cases and also replaced the zoom slider with EditorZoomWidget (see godotengine/godot-proposals#8294).

@@ -1194,40 +1249,6 @@ void Polygon2DEditor::_uv_draw() {
//draw paint circle
uv_edit_draw->draw_circle(bone_paint_pos, bone_paint_radius->get_value() * EDSCALE, Color(1, 1, 1, 0.1));
}

rect.position = -uv_edit_draw->get_size();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified and moved this scroll updating logic to _update_zoom_and_pan, since there's no reason to update it on every redraw, only when zoom gets changed. Also helps solving problem of setting scroll position to value which gets clamped by scrollbar, then updating scrollbar limits.
Logic for hiding scrollbars isn't needed, even in this original implementation it never triggers.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 4, 2023

For some reason the editor always opens partially off-screen:

2FTMgYlulN.mp4

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Nov 5, 2023

For some reason the editor always opens partially off-screen

Hmm, this is confusing. I'm not touching any of the window layout logic and didn't encounter such behavior while making this. Can you confirm this is a regression from this pr? I made some testing and noticed that there's a regression from 3.x to 4.0, that UV editor doesn't recover last window placement (maybe related to the fact that it's a native window now?).

And lastly, if godotengine/godot-proposals#8209 gets approved, there'll be no window at all 😛

@KoBeWi
Copy link
Member

KoBeWi commented Nov 5, 2023

Ok it's not caused by this PR.

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Nov 5, 2023

Thanks for the great feedback, updated 👌 I'll make similiar changes to #84353 soonish.

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

editor/plugins/polygon_2d_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/polygon_2d_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/polygon_2d_editor_plugin.cpp Outdated Show resolved Hide resolved
@akien-mga
Copy link
Member

Needs rebase, then should be ready to merge.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 4, 2024
Scrolling focuses towards mouse like in canvas editor.
Center view on opening.
Adjust scroll area to include polygons larger than texture.
Change zoom slider to EditorZoomWidget.
@aXu-AP
Copy link
Contributor Author

aXu-AP commented Jan 10, 2024

Rebased now, sorry for taking some time.

@akien-mga akien-mga merged commit 7ce8a8f into godotengine:master Jan 11, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@aXu-AP aXu-AP deleted the uv-edit-zoom-improvements branch January 13, 2024 07:47
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.

4 participants