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 2D editor zoom logic #48252

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Apr 27, 2021

master version of #50490.

  • Add 1-5 shortcuts to zoom between 100% and 1600% quickly (similar to GIMP).
  • When holding down Alt, go through integer zoom values if above 100% or fractional zoom values with integer denominators if below 100% (50%, ~33.3%, 25%, …).

This closes godotengine/godot-proposals#2658.

@Calinou Calinou added cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:2d topic:editor usability labels Apr 27, 2021
@Calinou Calinou added this to the 4.0 milestone Apr 27, 2021
@mathias17
Copy link

OK this is even better - simply pressing 1-9, without using Control, to alter zoom.

Two questions:

  1. Will these 9 new shortcuts be added to the Shortcuts tab in Editor Settings?
  2. Will these 9 new shortcuts work no matter which panel currently has focus?

@Calinou
Copy link
Member Author

Calinou commented Apr 28, 2021

  1. Will these 9 new shortcuts be added to the Shortcuts tab in Editor Settings?

Yes, this is already done 🙂

  1. Will these 9 new shortcuts work no matter which panel currently has focus?

No, these only work if the 2D editor viewport is focused. Making them work even if the 2D editor viewport isn't focused might be possible in master, but it's almost guaranteed to open a can of worms with regards to shortcut conflicts when this is cherry-picked for 3.x.

@akien-mga akien-mga requested a review from a team June 17, 2021 10:28
@groud
Copy link
Member

groud commented Jun 17, 2021

I am not really sold on the proposal. This is a lot of shortcuts for something that simply requires pressing Ctrl+'+' or Ctrl+'-' otherwise. And I'm not sure we need that amount of different values with one key each.

I think it would make more sense to make those 1-9 keys switch between scenes for example.

@mathias17
Copy link

mathias17 commented Jun 17, 2021

I am not really sold on the proposal. This is a lot of shortcuts for something that simply requires pressing Ctrl+'+' or Ctrl+'-' otherwise. And I'm not sure we need that amount of different values with one key each.

I think it would make more sense to make those 1-9 keys switch between scenes for example.

Interesting idea for number row keys 1 through 0 to be mapped to scene tabs. I think I would actually use that.
This type of shortcut functionality doesn't seem like the user would be able to modify it. I guess it would just be an arbitrary feature that users wouldn't be able to modify/turn off - which is fine.

If the consensus is for 1 - 0 to be mapped to scene tabs, I still suggest CTRL+[number] for specific zoom shortcuts.

And at the very least, as part of this proposal - ALT+SCROLLWHEEL will snap to integer zoom values. A big improvement, imo.

@KoBeWi
Copy link
Member

KoBeWi commented Jul 8, 2021

I think it would make more sense to make those 1-9 keys switch between scenes for example.

The difference is that there's no way to jump to zoom values, while you have all scene tabs available at once. No reason to switch them with number if you can just click them.

That said, the "1" shortcut is the same as clicking zoom label, but personally I also use it to reset zoom 🤔

EDIT:
I know noticed that there is both "Reset Zoom" and "Zoom 100%". I think the former should be removed.

@Calinou
Copy link
Member Author

Calinou commented Jul 8, 2021

I know noticed that there is both "Reset Zoom" and "Zoom 100%". I think the former should be removed.

We need both shortcuts until support for multiple editor shortcuts is implemented. (Otherwise, I have to hardcode one of them which isn't great.)

@KoBeWi
Copy link
Member

KoBeWi commented Jul 8, 2021

I mean, if Reset Zoom and Zoom 100% is the same thing, we don't need both. I'd just remove Ctrl+0, as 1 is easier to use anyways.

@Calinou
Copy link
Member Author

Calinou commented Jul 8, 2021

I'd just remove Ctrl+0, as 1 is easier to use anyways.

Most applications such as web browsers and code editors support Ctrl + 0 to reset the zoom. Removing that shortcut would break user expectations, but at the same time, removing 1 would also break user expectations for those coming from GIMP.

Edit: Rebased and tested again, it works successfully.

@KoBeWi
Copy link
Member

KoBeWi commented Jul 8, 2021

Wow, that Alt-zoom is so nice to use.

I still think that having two functionally identical shortcuts is redundant. We could remove it for now and possibly re-add when the multi-event shortcuts are added.

Also, this is inspired by GIMP, but GIMP has actually 5 zoom shortcuts. 1 for 100%, 2 for 200%, 3 for 400%, 4 for 800 and 5 for 1600%. I tried the numerical zooms and, while this is handy, having 9 of them, with 100% increment isn't very useful (especially when Alt+wheel does exactly that). Maybe it could be changed to something closer to GIMP, i.e. less shortcuts and more exponential scale?

@Calinou
Copy link
Member Author

Calinou commented Jul 8, 2021

Also, this is inspired by GIMP, but GIMP has actually 5 zoom shortcuts. 1 for 100%, 2 for 200%, 3 for 400%, 4 for 800 and 5 for 1600%. I tried the numerical zooms and, while this is handy, having 9 of them, with 100% increment isn't very useful (especially when Alt+wheel does exactly that). Maybe it could be changed to something closer to GIMP, i.e. less shortcuts and more exponential scale?

I thought about doing this at first, but I feel like zoom values above 900% aren't often useful in game development. (This is different when editing pixel art, for instance.)

@KoBeWi
Copy link
Member

KoBeWi commented Jul 8, 2021

Then it could stop at 4 / 800%.

@Calinou
Copy link
Member Author

Calinou commented Jul 8, 2021

Then it could stop at 4 / 800%.

Adding 5 / 1600% isn't a problem, it's just that being able to jump to any 100% increment quickly feels better overall. But at this point, I think anything is an upgrade over what we have currently.

@KoBeWi
Copy link
Member

KoBeWi commented Jul 8, 2021

But Alt + wheel already gives 100% increment. Having 9 shortcuts for that would be only useful if you use keyboard only, which is unlikely in the editor. I use GIMP quite a bit and IMO its zoom shortcut scheme is enough.

Adding 5 / 1600% isn't a problem

I know. That's why we could just have 4 shortcuts. Or also 5, but with different increments, e.g. 100/200/400/600/800.

- Add 1-5 shortcuts to zoom between 100% and 1600% quickly
  (similar to GIMP).
- When holding down Alt, go through integer zoom values if above 100%
  or fractional zoom values with integer denominators if below 100%
  (50%, ~33.3%, 25%, …).
@Calinou
Copy link
Member Author

Calinou commented Jul 10, 2021

I changed the zoom shortcuts to use power-of-two integer factors between 100% and 1600%. It now works the same way it does in GIMP.

@akien-mga akien-mga merged commit 51b0aed into godotengine:master Jul 13, 2021
@akien-mga
Copy link
Member

Thanks!

Comment on lines +5633 to +5637
ED_SHORTCUT("canvas_item_editor/zoom_100_percent", TTR("Zoom To 100%"), KEY_1);
ED_SHORTCUT("canvas_item_editor/zoom_200_percent", TTR("Zoom To 200%"), KEY_2);
ED_SHORTCUT("canvas_item_editor/zoom_400_percent", TTR("Zoom To 400%"), KEY_3);
ED_SHORTCUT("canvas_item_editor/zoom_800_percent", TTR("Zoom To 800%"), KEY_4);
ED_SHORTCUT("canvas_item_editor/zoom_1600_percent", TTR("Zoom To 1600%"), KEY_5);
Copy link
Member

Choose a reason for hiding this comment

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

I think the correct capitalization would be "Zoom to 100%", etc.

https://titlecaseconverter.com/blog/is-to-capitalized/

Copy link
Member Author

Choose a reason for hiding this comment

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

Done: #50418

@akien-mga
Copy link
Member

Could probably use a dedicated PR for 3.x, it's not trivial to cherry-pick.

@Calinou Calinou removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jul 15, 2021
@Calinou Calinou deleted the improve-2d-editor-zoom branch July 15, 2021 15:18
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.

Improve 2D editor zoom controls
5 participants