IJHack / QtPass

QtPass is a multi-platform GUI for pass, the standard unix password manager.
https://qtpass.org/
GNU General Public License v3.0
1.03k stars 162 forks source link

Fix High Dpi Support. Works now under Windows and KDE/Plasma. #392

Closed hgraeber closed 6 years ago

hgraeber commented 6 years ago

Commit 4a530820 was incomplete...

Qts scaling environment variables shall never be changed by application. That way the user hasn't any chance to fix things himself. With setting QT_AUTO_SCREEN_SCALE_FACTOR to 1 QtPass does not work under KDE/Plasma and cannot even be fixed by setting Qts scaling environment variables.

FiloSpaTeam commented 6 years ago

I suggest this code:

#if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)
QApplication::setAttribute(Qt::AA_EnableHighDpiScaling);
#else
qputenv("QT_AUTO_SCREEN_SCALE_FACTOR", "1");
#endif
hgraeber commented 6 years ago

@FiloSpaTeam Why. The only reason can be that Qt 5.6s HighDPI is broken.

By forcing the QT_AUTO_SCALE_FACTOR environment variable to a fixed value you will break it even more, because it is not possible anymore to use the variable outside of QtPass to fix the behavior on some platform where HighDPI misbehaves.

So I can only repeat: It is better to never change those environment variables inside of the program.

FiloSpaTeam commented 6 years ago

https://doc.qt.io/qt-5/qt.html#ApplicationAttribute-enum

FiloSpaTeam commented 6 years ago

This is introduced in Qt 5.6 and Debian uses Qt 5.2/3 so they cannot compile. Se have not defined a minimal Qt version so we should be compatible

hgraeber commented 6 years ago

@FiloSpaTeam With Qt before 5.6 HighDPI is broken anyways. For distributions using those old version I would expect that they patch it themself. I think they already have many other patches of this type inside of their packages and HighDPI still doesn't work completely right anyways.

codecov[bot] commented 6 years ago

Codecov Report

Merging #392 into master will increase coverage by <.01%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #392      +/-   ##
=========================================
+ Coverage    6.49%   6.49%   +<.01%     
=========================================
  Files          39      39              
  Lines        2557    2556       -1     
=========================================
  Hits          166     166              
+ Misses       2391    2390       -1
Impacted Files Coverage Δ
main/main.cpp 0% <ø> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3f6f753...59adb7c. Read the comment docs.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 6.422% when pulling 59adb7cff367a2914a574214d853322dccc25d20 on hgraeber:master into 3f6f7533a6802a40b60fba8aad27f3781f033675 on IJHack:master.