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

Adds a scale_gizmo_handles entry to the Touchscreen editor settings #75718

Merged
merged 1 commit into from
May 8, 2023

Conversation

m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented Apr 5, 2023

When enabled, this scales the editor icons to improve usability on touchscreen devices. In addition this commit fixes touch detection for the collision_shape_2d_editor_plugin so it scales with the icons size.

Fixes the Resize a CollisionShape2D and the Move Path2D Nodes issues in #73707

  • Non touchscreen UI
    Screenshot_20230405_145030_Godot Editor 4 (dev)

  • Touchscreen UI
    Screenshot_20230405_144946_Godot Editor 4 (dev)

3.x version (commit 445f5ea87d32172b6a01afd2df0fd9a44696874e)

editor/editor_node.cpp Outdated Show resolved Hide resolved
editor/editor_themes.cpp Outdated Show resolved Hide resolved
@YuriSizov
Copy link
Contributor

I'm not sure about the approach. I think that for the majority of controls in the editor the default scale increase doesn't improve the situation much. We should probably increase the overall scale + extra spacing.

So I would limit this PR to only apply extra scale to the gizmo handles (and rename the setting accordingly).

@m4gr3d m4gr3d force-pushed the add_scale_editor_icons_main branch 2 times, most recently from e1d26f7 to b7b5549 Compare April 8, 2023 05:06
@MewPurPur
Copy link
Contributor

This doesn't address #75144 I believe. QbieShay was referring to the Curve resource, not the Curve2D one.

@m4gr3d m4gr3d requested review from KoBeWi and groud April 24, 2023 01:31
@KoBeWi
Copy link
Member

KoBeWi commented Apr 26, 2023

Ye this PR does not fix #75144 The Curve editor is not even affected.

editor/editor_themes.cpp Outdated Show resolved Hide resolved
editor/editor_themes.cpp Outdated Show resolved Hide resolved
@KoBeWi

This comment was marked as resolved.

@m4gr3d m4gr3d force-pushed the add_scale_editor_icons_main branch 4 times, most recently from 974cfd7 to 47870a3 Compare May 6, 2023 14:19
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.

I left some minor comments, but it seems fine otherwise.

When enabled, this scales the editor icons to improve usability on touchscreen devices.
In addition this commit fixes touch detection for the collision_shape_2d_editor_plugin so it scales with the icons size.
@m4gr3d m4gr3d force-pushed the add_scale_editor_icons_main branch from 47870a3 to 30824e9 Compare May 7, 2023 22:56
@m4gr3d
Copy link
Contributor Author

m4gr3d commented May 7, 2023

I left some minor comments, but it seems fine otherwise.

@KoBeWi Thanks!

Can you also take a look at the 3.x version - #73692

@m4gr3d m4gr3d changed the title Adds a scale_editor_icons entry to the Touchscreen editor settings Adds a scale_gizmo_handles entry to the Touchscreen editor settings May 8, 2023
@m4gr3d m4gr3d merged commit bd1bc68 into godotengine:master May 8, 2023
@m4gr3d m4gr3d deleted the add_scale_editor_icons_main branch May 8, 2023 00:05
@Zireael07
Copy link
Contributor

Yay, this should massively improve Android port usability

@YuriSizov
Copy link
Contributor

This is more of a usability enhancement than a bug fix, so I think it's not worth a cherry-pick at this point. The Android editor is experimental, so it's fine to require updating to Godot 4.1 if you want all of the improvements done to it.

@YuriSizov
Copy link
Contributor

After an additional discussion we decided to cherry-pick this after all.

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.4.

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.

5 participants