-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Enable HiDPI on Windows #2469
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
Enable HiDPI on Windows #2469
Conversation
|
Looks good to me but I can't really test it. Maybe @justinclift can make a build for @chrisjlocke to check this doesn't change anything for non-HiDPI monitors? |
|
Yep, I can make a special build that includes this, so we can make sure it doesn't break standard resolution displays too. I'll get around it to after figuring out the macOS 3.12.1 release build problem, probably tonight some time. 😄 |
|
Are icons/resources available in higher resolution? |
|
Good question. I think we use one of the more common OSS icon sets that's around, I'm just not personally sure which one(s). Someone will know though. Hopefully that icon set has some SVG's too, which might be useful for solving that problem. 😄 |
|
If you mean the button icons, then they're all derived from 16x16 PNGs.... so no high resolution variants are available. |
|
@sandman7920 Oh cool, that looks promising. 😄 |
|
Excellent, thanks @sandman7920 - useful link! I use that icon set a lot in my own projects, so having larger versions is handy. |
|
This is a build of our latest
No icon adjustments in it though. 😉 @chrisjlocke Would you be ok to see how it looks on your (non HiDPI?) setup? 😄 |
|
Runs without any issues at all. 👍 |
|
Excellent, thanks @chrisjlocke and @sandman7920! 😄 |
|
Ok, this is merged now. We still need to figure out updated icons for HiDPI users though. |
|
I assume you'd need some 'resources' directory where DB4S pulls the resources as necessary. If it detects HiDPI (or command-switch?) it uses an alternative directory which contains SVGs or larger PNGs. |
|
@chrisjlocke one could add Edit <file alias="open_data_in_app">application_go.png</file>
<file alias="open_data_in_app@2x">[email protected]</file> |
|
For the avoidance of doubt (that and I'm a dummy) if the normal 16x16 icon is application_go.png, then [email protected] would be 32x32, @3x.png would be 48x48, etc. |
|
I think @3x.png will be enough. You can test for yourself (windows) |
|
Excellent, thanks! |
|
Arggh not all icons exists in the hi-res set |
|
Might be able to use variants and alternatives until they fill the gaps... there are also some ico to svg converters .... not perfect, but better than nothing.... maybe..... ;) |
|
I don't think project is still alive. |
|
Digging into Qt documentation iconSize-prop This means iconSize it's a platform dependant, or the iconSize for all toolbars must be set manually. |
|
I have made build with SVG resources and everything looks great on my side (4K display). @chrisjlocke may you test this build sqlitebrowser.zip @justinclift maybe it's better HiDPI to be enabled on all platforms? Some of UI files had to be changed, hard link to PNG files (not aliasing) Replacements TODO |
|
This looks like it's going down a pretty awesome path. 😄
No objection here. Seems like we could drop the That would at least let people on other platforms try it out, report problems, etc. 😄 |
|
@justinclift could you try to build https://github.com/sandman7920/sqlitebrowser/tree/SVG on Mac OS Replacement I couldn't find replacement for Someone else should try to find replacement, I am pretty bad with those kind of things. |
|
@justinclift someone should test this, I don't have Mac at home and won't be going to the office for two weeks. |
|
@revolter Would you have time to test if the icons in either of those two builds looks better on Retina / HiDPI screens than the standard (eg 3.12.1) build? |
|
Toolbar icons are platform dependent. Which means iconSize it's tuned to the platform Guidelines, from the screenshots above one could see that height of the Toolbar is the same as before (only icons are scaled up), Icons of smaller size will not be scaled up (before was 16px). We could use something like this: class DB4SProxyStyle : public QProxyStyle {
public:
int pixelMetric(QStyle::PixelMetric metric,
const QStyleOption *option = nullptr,
const QWidget *widget = nullptr) const override {
if (metric == QStyle::PM_ToolBarIconSize) {
return 16; // or maybe 22
}
return QProxyStyle::pixelMetric(metric, option, widget);
}
};
...
int main( int argc, char ** argv )
{
// Create application object. All the initialisation stuff happens in there
Application a(argc, argv);
a.setStyle(new DB4SProxyStyle);
This will override default style with custom value. @justinclift , @revolter what do you think |
|
So we're not specifying their size at all, not directly nor indirectly? I suppose we're using the icons in |
|
Yes, default size is set by platform style. On Windows is 30 on Linux 22, but if supplied resource is less than theme (platform default style) value Qt doesn't perform upscaling. |
|
I think that 16 would be perfect on macOS. For example, Safari uses 16pt icons too. |
|
Screenshots with 16px on Windows, everything looks OK. |
|
@sandman7920 Yep, can do. Are Windows or macOS or both needed? 😄 |
|
On Windows everything looks OK. @justinclift what is the next step, make new pull request? |
|
Meh, I'll do both. macOS ones are building atm. They should be online in about 45 mins or so... |
|
Ahhh. Yeah, I'd say make a new PR. Since the new macOS build is already running though, we might as well check that just to make sure too. 😄 |
|
In master branch? |
|
Yep. |
|
@revolter New macOS build to try, if you have the time. 😄 |
|
@revolter before on Mac OS iconSize was set 22 (i think), which means height of the ToolBar was 22 and icons were drawn in the middle. Now height is 16, I also think it's looks better now.
I don't think so, but you can try to start application with |
Awesome. Seems like an all-around win then. 😄 Thanks heaps for checking those builds @revolter. 😄 @sandman7920 You up for creating an appropriate PR (against |
Meaning QT_AUTO_SCREEN_SCALE_FACTOR=0 open "~/Downloads/DB Browser for SQLite.app"? |
|
@revolter yes |
|
Then it doesn't fix that issue. |
I don't think 'open' will set the environment variable when it runs the .app. Instead, run the executable in the |
|
Oh, true. But it still doesn't fix the issue. |
|
@justinclift is this a Qt 5.15.x build |
|
@sandman7920 Yep. Qt 5.15.1. |
|
As a data point, the test build has also been confirmed as working by someone on Twitter a few weeks ago: (Just noticed now, as I haven't been keeping an eye on Twitter) |










Enable HiDPI on Windows by default #2468