-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Q scale factor from preferences #3960
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some typos.
will test this soon.
Co-authored-by: ronso0 <[email protected]>
Done. |
What is the use case for using a scale factor different from the rest of the system? |
To bee able using the Deere skin on a full HD 15'' device. Windows scales such a device to 150% by default. That works nice for most Applications, but does not work for Mixxx. You can also consider the other way around. |
adjust the GUI to increased eye/screen distance when djing. |
Done. |
Pull Request Test Coverage Report for Build 1155856999
💛 - Coveralls |
checkBoxStartFullScreen->setChecked( | ||
m_pConfig | ||
->getValueString( | ||
ConfigKey(kConfigGroup, kStartInFullscreenKey)) | ||
.toInt() == 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are some ugly linebreaks now.
what about this, precommit agrees
checkBoxStartFullScreen->setChecked( | |
m_pConfig | |
->getValueString( | |
ConfigKey(kConfigGroup, kStartInFullscreenKey)) | |
.toInt() == 1); | |
m_pConfig->getValueString( | |
ConfigKey(kConfigGroup, kStartInFullscreenKey)) | |
.toInt() == 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not make the whole config key a constant? Then the line would be shorter.
Line breaks improved |
@uklotzde: merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the conflicts.
Done |
src/main.cpp
Outdated
QString strScaleFactor; | ||
QString line = in.readLine(); | ||
while (!line.isNull()) { | ||
if (line.startsWith(kScaleFactorConfigKey)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using QSettings
instead of hacking a new config parser here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, unfortunately the attempt failed because QSettings expects a "=" between key and value, which is not used in our ini file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. It looks like only the constructor depends on QApplication to construct file paths. Can you just add a ConfigObject static method like ConfigObject::fromFilePath(path)
?
src/main.cpp
Outdated
@@ -1,11 +1,13 @@ | |||
#include <QApplication> | |||
#include <QDir> | |||
#include <QSettings> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QSettings seems to be unused and a left-over from previous experiments?
src/main.cpp
Outdated
if (cfgFile.open(QFile::ReadOnly | QFile::Text)) { | ||
QTextStream in(&cfgFile); | ||
QString strScaleFactor; | ||
QString line = in.readLine(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better use in.readLine().trimmed()
(here an below) in case users edited their config file manually.
Done. Now it uses the ConfigObject parser. |
@uklotzde: Please have another look. |
0f87cb0
to
46182d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Time will tell if this fixes the scaling issues that have been reported frequently. Thank you! LGTM
This allows to set the environment variable QT_SCALE_FACTOR from preferences.
Unfortunately it is required to parse mixxx.cfg with custom code before initializing the QApp.
A workaround would be to use a private QT API for HID scaling which is also not nice.
This allows also to scale Mixxx by 50 % on a 200 % System while avoiding to go below 100 % for the overall scaling.