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

opensimplexnoise: Fix persistence, tweak documentation URL and layout #791

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

pseidemann
Copy link
Contributor

@pseidemann pseidemann commented Nov 18, 2022

persistence's spinbox step size was set to 1, breaking the default of 0.5.

changed step size to 0.05 because small changes in this value make a big different (only values 0 to ~2 are relevant).

other fixes I did:

  • typo persistance -> persistence
  • documentation button led to 404, changed to permanent 3.6 url (I think this is fair since the demo is made with this version and future godot versions might work differently)
  • decreased spinbox widths
  • increased button sizes to include "3.6" in the doc button name
  • increased vbox separation
  • centered texture
  • disabled high dpi since it doesn't actually properly scale on high dpi

before
Bildschirmfoto 2022-11-18 um 10 58 55

after
Bildschirmfoto 2022-11-18 um 10 59 36

ps: how is the release on the website handled?

@Calinou Calinou added the bug label Nov 18, 2022
@Calinou
Copy link
Member

Calinou commented Nov 18, 2022

disabled high dpi since it doesn't actually properly scale on high dpi

Please look into adding support for multiple resolutions instead. We shouldn't disable hiDPI support as operating systems' lowDPI fallbacks tend to behave badly (on top of looking blurry).

@pseidemann
Copy link
Contributor Author

hi @Calinou,
I was confused for a second, because as far as I know, there is no proper way to support high dpi with godot.

I read the article you linked and indeed it says this:

Note:
Godot doesn't support manually overriding the 2D scale factor yet, so it is not possible to have hiDPI support in non-game applications. Due to this, it is recommended to leave Allow Hidpi disabled in non-game applications to allow for the OS to use its low-DPI fallback.

this is a non-game application, as far as I can tell, since it's only using control nodes. so disabling the flag like I did is following the guide, if I'm not mistaken.

further, it states

By default, Godot projects aren't considered DPI-aware by the operating system.

but also

The Godot editor itself is always marked as DPI-aware.

how does the godot editor support high dpi?

how should this be handled if I have a game and I render controls on top of it?

@pseidemann
Copy link
Contributor Author

to be complete, following this guide, we should also set stretch mode to disabled and stretch aspect to ignore.
and indeed, right now, when I resize the window, controls will be scaled, which is bad since they can get very small/big, making it unreadable or harder to use.

@Calinou
Copy link
Member

Calinou commented Nov 18, 2022

this is a non-game application, as far as I can tell, since it's only using control nodes. so disabling the flag like I did is following the guide, if I'm not mistaken.

You can use the 2d stretch mode here, as many other GUI-only demo projects are already doing. It's not a huge problem for demo projects if text becomes too large at high resolutions, as long as the text remains readable on high DPI screens. We aim the demos to be usable when run on mobile devices too 🙂

Also, the documentation is outdated as overriding the 2D scale factor is possible since Godot 3.5.

how does the godot editor support high dpi?

The editor features a custom scale factor system that modifies the theme elements to suit the defined editor scale. This system has been in place since Godot 2.1 and predates the new 2D scale factor override.

@pseidemann
Copy link
Contributor Author

hi @Calinou,
allow hidpi + 2d stretch mode as it was before doesn't work for me.
the gui is too small and almost not readable on my system. I'm on mac os with a 27" 4k display (3840x2160 native) with 2304x1296 scaled resolution.

screenshot with godot website for size reference:
Bildschirmfoto 2022-11-19 um 14 29 19

however, with the recommended settings from the guide which are just the defaults, the gui size is correct:
Bildschirmfoto 2022-11-19 um 14 30 57

am I missing something?

@Calinou
Copy link
Member

Calinou commented Nov 19, 2022

allow hidpi + 2d stretch mode as it was before doesn't work for me.

You need to manually resize the window to make it larger on hiDPI displays, as Godot doesn't do this automatically if you've enabled Allow hiDPI. In comparison, the OS' lowDPI fallback manages this automatically, but it often messes up with fullscreen applications (especially on Windows).

Eventually, Godot should be made to automatically change the initial window size depending on the OS' scale factor (when Allow hiDPI is enabled). This is something that should be done at the engine level though. After doing this, we can automatically adjust the base 2D scale factor based on the OS-reported scale factor. The application would effectively look the same as if you disabled Allow hiDPI, except it'd be crisper.

Mobile devices don't have a concept of window size (by default), so the app is always made fullscreen there.

@pseidemann
Copy link
Contributor Author

hi @Calinou,

You need to manually resize the window to make it larger on hiDPI displays [...]

sorry, how is this better than just using the defaults? then no resize would be needed.

[...] it often messes up with fullscreen applications (especially on Windows).

isn't this a separate bug and should be fixed within the engine?

[...] Godot should be made to automatically change the initial window size [...]

hmm this doesn't make sense for me. I, as a user of a gui application, should be able to choose a arbitrary window size, by resizing the window to my liking (of course their might be a minimum size but this doesn't matter here) and the gui elements should always have the same size, no matter how big or small I want to make the window.

I'm fine with changing it back to "allow hidpi", also for consistency sake with other demos, but I'm interested in your reasoning, because it seems like we have very different understandings how gui applications should behave. I'm just comparing here with other existing applications like blender or even the godot editor itself. they behave very differently from what you describe as a desired behavior.

@Calinou
Copy link
Member

Calinou commented Nov 19, 2022

isn't this a separate bug and should be fixed within the engine?

No, this is a Windows bug that's been around ever since it's had a lowDPI fallback. Plenty of AAA games still run into this issue nowadays because their executable doesn't mark the game window as DPI-aware. This can be worked around by the user by explicitly marking it as DPI-aware in the executable's properties, but it's not something most people are aware of. (In my experience, most people aren't even aware that they're using a hiDPI display in the first place.)

For reference, in 4.0, Allow hiDPI is enabled by default due to all the issues caused by lowDPI fallbacks on both Windows and macOS. I'd prefer if the demo projects in 3.x aligned with the new defaults in 4.0 to make porting the demos easier.

@pseidemann
Copy link
Contributor Author

independent of the new convention in 4.0, don't you think there is a kind of surprising discrepancy?

@pseidemann
Copy link
Contributor Author

reverted the allow hidpi change and moved the discussion regarding scaling to godotengine/godot#42085.

good to merge from my side

@pseidemann
Copy link
Contributor Author

cc @Calinou

@pseidemann
Copy link
Contributor Author

hi @Calinou could you merge this please?
thanks! 😃

Copy link
Member

@Calinou Calinou left a 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. Thanks!

@Calinou Calinou changed the title opensimplexnoise: fix persistence, doc url + layout tweaks opensimplexnoise: Fix persistence, tweak documentation URL and layout Feb 22, 2023
@Calinou Calinou merged commit 1849640 into godotengine:master Feb 22, 2023
@Calinou
Copy link
Member

Calinou commented Feb 22, 2023

Please check if some of the updates are relevant for the 4.0-dev branch's Noise Viewer, which uses FastNoiseLite (it replaces OpenSimplexNoise in 4.0): https://github.com/godotengine/godot-demo-projects/tree/4.0-dev/misc/noise_viewer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants