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

HiDPI Skin Scaling #1204

Merged
merged 40 commits into from
Mar 30, 2017
Merged

HiDPI Skin Scaling #1204

merged 40 commits into from
Mar 30, 2017

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Mar 5, 2017

This is the PR original started by @devananda to allow to build skins using @2x suffixed images for HiDpi screens.

I have picked it up, because we have an increasing amount of reports complaining about missing HiDPI support on Win and Linux. Mac has already "some" HiDpi support by the OS, for font rendering, and upscaling. so there seams to be less pressure.

I have now completed his work with floating point scaling for almost all skin widgets.
For now, I have kept the checkbox in preferences for double only. I consider fractional scaling as an experimental option, that can be enabled by editing the cfg file. ScaleFactor 0.5
This is usefull for example to test big skins on small displays.
It is fun to test exotic scaling values B-)

I have failed to scale the combobox which is heavy styled by a style sheet.
Qt4 has no nice interface to hock between style sheet ans widget painting.
So other styled elements are effected as well.
I think a complete solution for this requires Qt5.

I have no HiDPI screen, so I cannot decide if this is already usable.
Assuming this does not break 1x scaling, this is a candidate to include it into 2.1 marked as experimental.

In any case this is a step in a right direction, because it allows high resolution cover arts and waveforms.

@daschuer
Copy link
Member Author

daschuer commented Mar 5, 2017

Deere @0.5x
bildschirmfoto vom 2017-03-05 01 55 48

@daschuer
Copy link
Member Author

daschuer commented Mar 5, 2017

@devananda: I have removed the upscaled @2x versions, because the on-the-fly generated version are quite similar. To improve the @2x quality some @2x images with removed aliasing would be nice.
An other option would be to switch to svg versions. Unfortunately this comes with a penalty of skin loading time.

@@ -489,6 +489,23 @@
</property>
</widget>
</item>
<item row="12" column="1" colspan="2">
<widget class="QCheckBox" name="checkBoxDoubleWidgetSize">
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be a QDoubleSpinBox?

Copy link
Member Author

Choose a reason for hiding this comment

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

For now this is a checkbox by design, because we "only" have the request for double the size.
And I want to avoid to have to much degrees of freedom for now.

If you thing it is useful to allow free scaling, we can consider to go for it. Today almost all is calculated on pixel base so that 1x and 2x works nice and fractional scaling is kind of blurry.
It now supports @2x suffix to avoid this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know other ratios are common on smartphone screens, but Mixxx does not run on those. I am not sure about laptops.

Copy link
Contributor

Choose a reason for hiding this comment

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

The desired scale factor may vary depending on screen resolution, physical screen size, and personal taste, so I think it would be good to make it more flexible.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 7, 2017

Build error with gcc-6.3.1-1.fc25.x86_64:

g++ -o lin64_build/skin/skincontext.o -c -std=c++11 -pipe -Wall -Wextra -g -O3 -ffast-math -funroll-loops -fomit-frame-pointer -march=native -pthread -Dx86_64 -DMIXXX_BUILD_DEBUG -D__LINUX__ -D__UNIX__ -DSETTINGS_PATH=\".mixxx/\" -DSETTINGS_FILE=\"mixxx.cfg\" -DUNIX_SHARE_PATH=\"/home/be/local/share/mixxx\" -DUNIX_LIB_PATH=\"/home/be/local/lib/mixxx\" -D__PORTAUDIO__ -DQT_TABLET_SUPPORT -DQT_SHARED -DQT_CORE_LIB -DQT_GUI_LIB -DQT_OPENGL_LIB -DQT_XML_LIB -DQT_SVG_LIB -DQT_SQL_LIB -DQT_SCRIPT_LIB -DQT_NETWORK_LIB -DQT_SHARED -D__SNDFILE__ -D__MAD__ -D__HID__ -D__BULK__ -D__VINYLCONTROL__ -D__BROADCAST__ -D__OPUS__ -D__VAMP__ -Dkiss_fft_scalar=double -DHAVE_FFTW3 -D__SQLITE3__ -D__BATTERY__ -Ilin64_build -Isrc -Ilib/soundtouch-1.9.2 -Ilib/replaygain -Ilib/libebur128-1.1.0/ebur128 -I/usr/include/QtOpenGL -I/usr/include/QtXml -I/usr/include/QtSvg -I/usr/include/QtSql -I/usr/include/QtNetwork -I/usr/include/QtTest -I/usr/include/QtScriptTools -I/usr/include/QtGui -I/usr/include/QtScript -I/usr/include/QtCore -Ilib/gtest-1.7.0/include -Ilib/fidlib-0.9.10 -I/usr/include/taglib -Ilib/qtscript-bytearray -Ilib/reverb -Ilib/portaudio -I/usr/include/libusb-1.0 -Ilib/hidapi-0.8.0-rc1/hidapi -Ilib/xwax -Ilib/scratchlib -I/usr/include/opus -I/usr/include/libupower-glib -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include src/skin/skincontext.cpp
src/skin/skincontext.cpp: In member function ‘int SkinContext::scaleToWidgetSize(QString&) const’:
src/skin/skincontext.cpp:268:60: error: ‘round’ was not declared in this scope
         return static_cast<int>(round(dSize * m_scaleFactor));
                                                            ^
scons: *** [lin64_build/skin/skincontext.o] Error 1
scons: building terminated because of errors.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 10, 2017

I tried this on my 1366 x 768 laptop screen and it seems to work as advertised. Setting ScaleFactor to 2.0 made the GUI very laggy for me. I'm guessing that's because of my old integrated GPU, so I'm guessing it wouldn't be an issue on a newer computer that has a screen resolution high enough to need this feature. Scaling down to 0.5 did not make the GUI lag.

Thanks for taking up this bug!

@Be-ing
Copy link
Contributor

Be-ing commented Mar 10, 2017

I have failed to scale the combobox which is heavy styled by a style sheet.
Qt4 has no nice interface to hock between style sheet ans widget painting.
So other styled elements are effected as well.
I think a complete solution for this requires Qt5.

Unfortunately the QDoubleSpinBox subclass introduced in #1187 suffers the same problem. :/

@esbrandt
Copy link
Contributor

esbrandt commented Mar 12, 2017

Thanks for working on it.

Tested the current state of the PR on macOS with latest master.

  • Pushbutton images look jagged, even if unscaled. Deere uses *.svg, Latenight *.png see screenshot below.
    deere 1 hidpi default-1
    deere 2 hidpi
    latenight 1 hidpi default
    latenight 2 hidpi
  • When switching from a skin that supports color schemes, back to another skin, somehow the color scheme processor does not reset. In the example below, i changed to Dark Metal, selected another color scheme, than switched to Latenight. Some Pushbuttons, sliders and background colors are tinted.
    mixxx-117
  • Naming of the preferences option. Lend the wording from Windows 10, here as i find it pretty spot on, and usable even if we decide to support other resolutions then x2 (e.g. by a slider)
    HiDPI / Retina scaling: Change the size of text, buttons, and other items
    Having the HiDPI/Retina buzzwords helps to catch the eye, and describe in user centric terms what the option does.
  • Just for the case someone comes up with a HiDPI skin, and the user has the x2 option enabled, is there a way to handle that? Like scanning the skin manifest for a (not yet existing) HiDPI node, determined the resolution, and defaulting to x1?

@Be-ing
Copy link
Contributor

Be-ing commented Mar 12, 2017

Pushbutton images look jagged, even if unscaled

I noticed this too on GNU/Linux (X11).

HiDPI / Retina scaling: Change the size of text, buttons, and other items

SGTM

Just for the case someone comes up with a HiDPI skin, and the user has the x2 option enabled, is there a way to handle that? Like scanning the skin manifest for a (not yet existing) HiDPI node, determined the resolution, and defaulting to x1?

I don't think we should have skins that only work on HiDPI displays.

@esbrandt
Copy link
Contributor

I don't think we should have skins that only work on HiDPI displays.

Why not, doesn't have to be the default...
I do not have a HiDPI screen, but imagine developing a x0.5 skin on a HiDPI screen is no fun. I would rather take any good skin than booting it out. But this is a non-issue, if we allow for scaling x0.5.

@daschuer
Copy link
Member Author

I think I have discovered the rule:
You can set the font size by pixel (px) or by point (pt) size.
The point size fond depends on the Screen DPI, which changes with the OS Desktop scaling (verified with Ubuntu Unity and Windows 10)
Original the WEffectSelector has no font size set, and defaults to the default point sized font.
I have changed it now to a px size fond, and removed all mixxx scaling from point size fonts to avoid double scaling.

The style @2x hack could be used to tweak for example margin and the combobox handle.
Do you think keeping this will introduce future issues? If we do not make use of it for 2.1 I should remove or disable it.

There is still an issue with the Track table header. It seams that this is a pixel size font, because it is not OS scaled. Do you know where it is define? This has to be a point size font.

We can also enable the Auto feature for Windows by query the widget DPI.

@esbrandt
Copy link
Contributor

esbrandt commented Mar 25, 2017

There is still an issue with the Track table header. It seams that this is a pixel size font, because it is not OS scaled. Do you know where it is define? This has to be a point size font.

In Deere: https://github.com/mixxxdj/mixxx/blob/master/res/skins/Deere/style.qss#L178

@daschuer
Copy link
Member Author

Ups .. that easy :-) Thank you!

Copy link
Contributor

@esbrandt esbrandt left a comment

Choose a reason for hiding this comment

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

Originally, we had all font sizes given in pt, but switched to px sizes. I can not remember the details, but there was possible a cross-platform issue.

So we need to test if this change does not have unwanted consequences.

Edit: Found 98eec1e

@Be-ing
Copy link
Contributor

Be-ing commented Mar 25, 2017

There is still an issue with the Track table header

It's not just the table headers; it's also the text in the table cells that is not scaled.

@daschuer
Copy link
Member Author

We have to distinguish between the skin and the library region. In the skin region, we have to use pixel size fonds in our custom widgets, to not have cross platform issues and to make the new scaling option from this PR work. The text on buttons should not resize with the OSs font setting.

In the library area, we have the "Library Font" preference option to scale this area separate. This is a point size font, which is scaled by the OSs DPI setting. The same is true for other dialogs like preferences.

If we now look at the header bar of the track table we have two options:
Use pixel size fonts and override the header bar font code to resize them annual. Or just use point size fonts and rely on the OSs font size settings.
In this case it is convenient that the header follows the OSs setting like the table content.

@daschuer
Copy link
Member Author

now with auto HiDpi for windows as well

@daschuer
Copy link
Member Author

Ready for merge ?!

@Be-ing
Copy link
Contributor

Be-ing commented Mar 29, 2017

How does this behave with Qt5? Should the scale factor be hardcoded as 1.0 with Qt5 and the option hidden from the preferences?

Conflicts:
	src/preferences/dialog/dlgprefcontrols.h
	src/preferences/dialog/dlgprefcontrolsdlg.ui
@daschuer
Copy link
Member Author

How does this behave with Qt5? Should the scale factor be hardcoded as 1.0 with Qt5 and the option hidden from the preferences?

I have not tested it with QT5 and I would like to avoid the work on QT5 for now since my last effort produces a crashing Mixxx. So it would be OK for me to hide the option for now.

@daschuer
Copy link
Member Author

Done.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 29, 2017

LGTM! Thank you for making Mixxx usable on newer computers. After this is merged I will adopt the font scaling strategy used for WEffectSelector in this PR for WBeatSpinBox in #1187.

@tristan-k
Copy link

How do I test this? I'm using a surface device (3000x2000 resolution). I downloaded the latest build from http://downloads.mixxx.org/builds/master/release/mixxx-2.1.0-alpha-pre-master-git6233-release-x64.msi, but I can't find any option for high dpi displays in the preferences.

@daschuer
Copy link
Member Author

daschuer commented Mar 30, 2017

You should try this:
https://ci.appveyor.com/api/buildjobs/hakiak1wqjuomd32/artifacts/mixxx-2.1.0-alpha-pre-master-PR1204-git6234-release-x64.msi

Please check if Mixxx scales with Windows when changing from 100 to 200 % in the windows preferences and Auto Scaling enabled (default) in Mixxx preferences.

Could you provide a screen shot? This was developed on a SD screen.

@tristan-k
Copy link

Sure. After a quick look it seems to scale properly.

mixxx_highdpi

@daschuer
Copy link
Member Author

Thank you! For testing and review and @devananda for a solid start! This is finally even better than I had hoped initially.

@daschuer daschuer merged commit 1447df7 into mixxxdj:master Mar 30, 2017
@esbrandt esbrandt mentioned this pull request Jun 24, 2017
37 tasks
@daschuer daschuer deleted the LateNight branch September 26, 2021 17:35
This was referenced Aug 22, 2022
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.

5 participants