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

Set a minimum zoom of 1% on the tileset editor #57329

Merged
merged 1 commit into from
Feb 15, 2022

Conversation

Ev01
Copy link
Contributor

@Ev01 Ev01 commented Jan 27, 2022

Fixes #57292 where zooming out too much stops you from being able to zoom back in.

@Ev01 Ev01 force-pushed the tileset-editor-minimum-zoom branch from a9c91aa to e481866 Compare January 28, 2022 00:06
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.

Works alright (not sure about the EDSCALE part though).

@Ev01
Copy link
Contributor Author

Ev01 commented Jan 28, 2022

Works alright (not sure about the EDSCALE part though).

Yeah, I put the EDSCALE part there because it's what EditorZoomWidget::_update_zoom_label() does to get the percentage. I'm not exactly sure what EDSCALE is and the bug could still happen if EDSCALE is 1000 or higher.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 28, 2022

EDSCALE is the scale of the editor (you can set it in Editor Settings). 1000 is insane value, so it won't be a problem.

@Ev01
Copy link
Contributor Author

Ev01 commented Jan 28, 2022

CanvasItemEditor does it like this:

// At top of file
#define MIN_ZOOM 1. / 128
#define MAX_ZOOM 128

// At top of zoom function
p_zoom = CLAMP(p_zoom, MIN_ZOOM, MAX_ZOOM);

Maybe this would be a better/more consistent way to do it?

@Ev01
Copy link
Contributor Author

Ev01 commented Jan 28, 2022

Setting a max zoom would also prevent reaching insane 40-digit zoom levels.

@Calinou
Copy link
Member

Calinou commented Jan 28, 2022

I'd probably the clamp the zoom to a factor of 100 (that is, 10000%).

… editor

Fixes issue godotengine#57292 where zooming out too much stops
you from being able to zoom back in
@Ev01 Ev01 force-pushed the tileset-editor-minimum-zoom branch from e481866 to c66ab56 Compare January 30, 2022 02:48
@Ev01
Copy link
Contributor Author

Ev01 commented Jan 30, 2022

After testing with different editor scales I discovered dividing by EDSCALE was unnecessary. I set the minimum zoom to 1% and the maximum to 10000%.

@Ev01 Ev01 requested a review from KoBeWi January 30, 2022 02:53
@akien-mga akien-mga merged commit 6d708df into godotengine:master Feb 15, 2022
@akien-mga
Copy link
Member

Thanks!

We discussed this in a PR review meeting and found that this is fine as an ad hoc solution, but it would be good to rework things in EditorZoomWidget directly so that it can prevent reaching 0 and being unable to zoom in. Then existing editor plugins that use EditorZoomWidget could have their ad hoc clamping removed as this should be part of the EditorZoomWidget behavior.

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.

Cant zoom in after zooming out to 0% in tileset editor
4 participants