-
-
Notifications
You must be signed in to change notification settings - Fork 21.2k
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 a zoom speed setting to the 2D editor #95906
Conversation
7f7437c
to
4eb3222
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit doubtful on whether steps of 0.01
within the interval are necessary. Feels needlessly precise, personally. Perhaps there's a magical number that ensures all corresponding zoom values look good enough... Or 0.06
-ish, or 0.1
.
There was a problem hiding this 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. The new zoom sensitivity feels a lot nicer on the mouse wheel 🙂
Regarding the mouse wheel no longer going through integer values by default, I think it's not too much of an issue. You can still use keyboard shortcuts to quickly go to a zoom level preset (1-5 and Shift + 1-5), along with holding Alt while using the mouse wheel to zoom through integer zoom levels only.
70fddcb
to
dfdf7eb
Compare
I checked the other Editor settings and there's only a single float setting (3D/Grid Division Level Bias) that doesn't have I also tried out steps of |
float new_zoom = Math::pow(2.f, new_zoom_index / 12.f); | ||
// Zoom is calculated as pow(zoom_factor, zoom_step). | ||
// This ensures the zoom will always equal 100% when zoom_step is 0. | ||
float zoom_factor = EDITOR_GET("editors/2d/zoom_speed_factor"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a super hot path, but it still seems wasteful to re-query this editor setting every time the user zooms.
I would suggest caching it in a member variable, and re-assigning it in _notification
when EditorSettings::NOTIFICATION_EDITOR_SETTINGS_CHANGED
is received.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has the impact of fetching project/editor settings ever been measured, to really gauge how severe this is? I was wondering if there's any sort of caching system for settings that are continuously fetched, or if it has to always be manual optimization work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it this way because I saw that the integer zoom setting was handled similarly in CanvasItemEditor::_zoom_callback
. Now I also noticed that CanvasItemEditor::_gui_input_viewport
(which gets called every time you move/click the mouse) also uses EDITOR_GET
, plus many other functions. So either it doesn't seem to be a performance issue, or it's something that should be focused on in a separate PR perhaps?
I might still cache the setting however, using CanvasItemEditor::_update_editor_settings
just like ViewPanner
does.
Edit: No I won't, not using that method at least, since it only gets called if specifically the theme or panning settings get changed.
Thanks! |
The old zoom logic (introduced by #37073), used the twelfth root of two as the zoom factor (roughly equaling
1.06
). This had the advantage of visiting all power of two factors (e.g. 25%, 50%, 100%, 200%) which is good for pixel art, but led to quite slow zooming and had many intermediate values you had to scroll through to actually hit those nice values. After #48252 introduced shortcuts for this use case, and the Integer Zoom setting now exists, I don't see much use in preserving this constant zoom factor.This PR instead lets the user configure the zoom factor to be between
1.01
and2.0
. It will still be guaranteed to hit the 100% zoom value, but not any other specific values. However, when setting it to2.0
, it will only hit power of two factors, since it doubles or halves the value every time.I set the default to
1.1
, in order to be slightly faster than the current zooming (and since it's a pretty number).I additionally changed the tooltip for the Integer Zoom setting, since it wrongly stated that default zooming snaps to powers of two (insinuating that each zoom step would be a power of two, without any intermediate values). In any case, that would not be true after this PR, unless setting the factor to
2.0
as previously stated.