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

Add uiScale option to the Graph Editor #1586

Conversation

virokannas
Copy link
Contributor

@virokannas virokannas commented Oct 26, 2023

The branch has an additional command line variable for MaterialXGraphEditor, --uiScale, that will override the automatic calculation and application of DPI scaling.

Using this flag on MacOS with a retina display, will result in a normal-sized UI.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 26, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@kwokcb
Copy link
Contributor

kwokcb commented Oct 27, 2023

Hi @simpassi ,
I think it would be better if the dpi detection was corrected instead to return the appropriate value for retina displays.
Would you have time to look at this? Thanks.

@virokannas
Copy link
Contributor Author

virokannas commented Oct 27, 2023

Sure @kwokcb , I can check what all would be involved, but the underlying mechanism causing this is that GLFW needs a special hint to enable full-resolution backing, and at that point DPI scaling will need to be handled per monitor. Also, the situation where a window is moved from one monitor to another needs to be caught and handled.

Right now as the hint isn't enabled, GLFW will just use a logical, not physical, resolution for everything and as a side effect the UI is correct size on any monitor you move it to, as the OS is handling the viewport scaling.

I'm sure there'll still be situations where disabling scaling is desired. Another option is to have a variable to set the scaling factor manually - some UI frameworks like QT make this possible through environment variables.

@kwokcb
Copy link
Contributor

kwokcb commented Oct 27, 2023

If this get's too complicated (even just for the single monitor case) than I think your suggestion of allowing a scale factor override is a good alternative and more flexible than an on/off switch.

@virokannas
Copy link
Contributor Author

I changed it to take an optional --scaleFactor float argument. On a retina Mac, 1.0 will yield an acceptable result, but of course now it can be used to simply scale the UI up or down too.

@virokannas
Copy link
Contributor Author

@kwokcb forgot to tag you in the above message; let me know if there's anything you'd like changed in the merge req.

Copy link
Contributor

@kwokcb kwokcb left a comment

Choose a reason for hiding this comment

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

Looks good to me. Sorry for the delay in review as I missed the notification.

source/MaterialXGraphEditor/Main.cpp Outdated Show resolved Hide resolved
@virokannas
Copy link
Contributor Author

@jstone-lucasfilm now it's changed to uiScale

Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
@jstone-lucasfilm jstone-lucasfilm changed the title Fix double HiDPI scaling in GraphEditor by adding a command line variable Add uiScale option to the Graph Editor Jun 28, 2024
@jstone-lucasfilm jstone-lucasfilm changed the title Add uiScale option to the Graph Editor Add uiScale command-line option to the Graph Editor Jun 28, 2024
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks @virokannas!

@jstone-lucasfilm jstone-lucasfilm changed the title Add uiScale command-line option to the Graph Editor Add uiScale option to the Graph Editor Jun 28, 2024
@jstone-lucasfilm jstone-lucasfilm merged commit 8e050bf into AcademySoftwareFoundation:main Jun 28, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants