ToolBoxWidget: Improve high DPI scenarios and improve layout flow.#1374
Conversation
|
Having second thoughts, I think it makes sense to just simply remove any element of variation here. Pinta doesn't have enough tools for any complexity to be needed here, and plenty of space to use the ones it has. New smallest window New smallest window without layers and history Kinda the same arbitrarily larger as the other one Kinda the same arbitrarily larger larger as the other one
1440p 150% display scaling |
|
Last set of tweaks, I think this last one's the general winner, in dropping the override of Valign such that the default flex mode is preferred.
The results above make me think there's more work to be done potentially, as that was setting the OS level scaling to 2x and looked a load better. However the native scaling of this monitor is 100%, so those results make everything else on the system look worse. |
|
I've marked this as draft for now. I'm convinced it's an improvement, but I'm going to explore if there's any other ways of approaching it too, which is difficult, because I don't know GTK at all! The last commit just simplifies this to it's purest form. Let GTK handle everything, all we care about is it generates a 2 column grid, and less here is working better than more IMO. If another tool gets added to the default view, then every time there's a second pair, force this back to being 2 columns. Meanwhile if plugins add then tools, they're guaranteed to be separated away from Pinta's main tool's apart from if there's an odd amount in the base set. So, small but impactful I guess.
|
|
Current updates involving just injecting some CSS to force the images to be bigger:
By far the best attempt yet. |
|
This pull request got pretty noisy but hopefully it shows I've evaluated all the options and the one that's ready for review seems to be the best user experience whilst not a massive amount of new complexity in the code.
|
|
And the final screenshot from myself, before letting someone else use this PR 😆
|
|
Thanks for investigating this! I won't have time to test it for another couple days, but will try it out on macOS too |
|
The strange thing is, whilst I don't think the top few screenshots "look" good. They absolutely performed better in real life in the maximised screen. There's a lot going on here that depends upon the monitors being used and the people using them; but I think what's going on in this specific commit is that |
|
So this essentially just increases the size of the toolbox icons and makes the new size proportional to the font size. This works around GTK's broken fractional scaling on non-Wayland platforms for these icons but not for the rest of the UI. Here are the cases from #1365 (comment) with the code from this PR, all of these are on a 14" 2240x1400 monitor where I normally use 150%: X11 100%: X11 150%: X11 200%: Wayland 150%: Windows 100%: Windows 150%: Windows 200%: Additionally here's a comparison with Pinta 3.0 at 100% on a 22" 1920x1080 monitor. Notice how the icons get bigger also here. This PR: Overall this seems like an improvement. For the X11/Windows 150% cases I think it solves the most major scaling issue, and even in the other cases I slightly prefer the larger icons. It doesn't fully fix the fractional scaling problems in #1365 but that is really a GTK issue which may be too complex to work around in Pinta. |
|
Looks pretty good on macOS too with the larger icons 
|
| public void AddItem (ToolBoxButton item) | ||
| { | ||
| // Despite .Flat already being set, if you use .AddCSSClass("ToolBoxWidget"), stuff doesn't work, so we set both at once. | ||
| item.Tool.ToolItem.SetCssClasses (["ToolBoxWidget", AdwaitaStyles.Flat]); |
There was a problem hiding this comment.
Perhaps this could move into the ToolBoxButton constructor to replace its AddCssClass(AdwaitaStyles.Flat) call? (which doesn't do anything now since it's being overridden here)
Organization-wise, it feels cleaner to have the button set up its own style correctly
In the long run I think we'll also likely add an application-wide css file (related to #1370) but loading the custom css in the ToolBoxWidget constructor is fine as-is since this keeps it to a simple change that's safer to backport to a 3.0.x minor release
|
Hopefully My only concern with moving to the ToolBoxButton constructor, rather than the ToolBoxWidget constructor, is that you're effectively reloading the CSS 22 times for each button, rather than once in the Widget. Despite this, everything works basically the same, and given there's other open issues where moving CSS into the main loop might be the correct fix, long term it seems more appropriate to move the CSS wherever that ends up being in the future so the ToolBoxButton itself just adds the class rather than attempts to define it. For now though, everything works and it's probably a penalty of a microsecond once per initialisation so whatever really :) |
This avoids a specific issue where the icons get smaller as the application canvas gets larger. However, this also increases the physical size of the icons even at lower resolution and DPI. While the high resolution case was the primary motivator, the other cases seem improved visually too. Related discussion #1365
|
That last one should be good to go, since a friend introduced me to static constructors as a concept to keep things modern. |
|
Thanks! The latest version looks good 👍 |
…1374) This avoids a specific issue where the icons get smaller as the application canvas gets larger. However, this also increases the physical size of the icons even at lower resolution and DPI. While the high resolution case was the primary motivator, the other cases seem improved visually too. Related discussion #1365 (cherry picked from commit c9cf7d4)
…1374) This avoids a specific issue where the icons get smaller as the application canvas gets larger. However, this also increases the physical size of the icons even at lower resolution and DPI. While the high resolution case was the primary motivator, the other cases seem improved visually too. Related discussion #1365 (cherry picked from commit c9cf7d4)
Related discussion #1365
In summary, I believe that there's no practical benefit of the current behaviour which allows for a single vertical toolbar, if the expense is that the toolbar itself begins to shrink too.
Based on the following scales below, I'd like to imagine that this change is an overall win, since having personally tested before and after this patch being applied, the overall space used ends up being about the same; but on larger monitors, there's less mouse movement as well as bigger icons.
Smallest possible window size in Windows:
Arbitrarily larger:
Larger still
Full screen 1440p 100% display scaling
