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

Allow higher and lower maximum zoom values in GraphEdit #49437

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jun 8, 2021

Low zoom values result in unreadable text, but it can still be useful for previewing purposes.

Eventually, characters could be replaced by rectangles at very low zoom levels to improve the visual appearance.

Low zoom values result in unreadable text, but it can still be
useful for previewing purposes.

Eventually, characters could be replaced by rectangles at very low
zoom levels to improve the visual appearance.
@Calinou Calinou requested a review from a team as a code owner June 8, 2021 18:33
@Calinou Calinou added cherrypick:3.3 cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:editor usability labels Jun 8, 2021
@Calinou Calinou added this to the 4.0 milestone Jun 8, 2021
@YuriSizov
Copy link
Contributor

I still have a PR that makes those values customizable #38261

@AaronRecord
Copy link
Contributor

Also #47986...

@YuriSizov
Copy link
Contributor

YuriSizov commented Jun 8, 2021

Well my PR is over a year old, I think. So if any of you would like to expose min, max and step for the configuration in yours, be my guest :)

// but this is still useful for previewing purposes.
#define MIN_ZOOM (1 / Math::pow(ZOOM_SCALE, 8))

// Allow zooming 4 times from the default zoom level.

Choose a reason for hiding this comment

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

Might as well add only 1 comment saying which part allows zooming how many times? That might help a bit.

Copy link
Member Author

@Calinou Calinou Jun 9, 2021

Choose a reason for hiding this comment

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

@CertifiedRice I think it's obvious enough from the number in the comment matching the Math::pow() exponent.

@akien-mga
Copy link
Member

Let's merge this as something easy to cherry-pick, but both #38261 and #47986 seem like good improvements to finalize and merge (please cross-review your PRs :)).

@akien-mga akien-mga merged commit e479abe into godotengine:master Jun 15, 2021
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 17, 2021
@akien-mga
Copy link
Member

Cherry-picked for 3.3.3.

@Calinou Calinou deleted the graphedit-allow-higher-lower-zoom-values branch June 17, 2021 16:20
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