Bug 389826 - Elisa looks bad with non-96 DPI
Summary: Elisa looks bad with non-96 DPI
Status: RESOLVED FIXED
Alias: None
Product: Elisa
Classification: Applications
Component: general (show other bugs)
Version: 0.0.81
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Matthieu Gallien
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-03 11:47 UTC by Mykola Krachkovsky
Modified: 2018-02-24 14:13 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Elisa at 144 DPI (310.19 KB, image/png)
2018-02-03 11:47 UTC, Mykola Krachkovsky
Details
To make theme sizes depend on DPI (2.90 KB, patch)
2018-02-11 10:38 UTC, Mykola Krachkovsky
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mykola Krachkovsky 2018-02-03 11:47:50 UTC
Created attachment 110318 [details]
Elisa at 144 DPI

I have 144 DPI monitor and Elisa looks not good (Now playing and lower).

1. Shuffle and Repeat checkboxes are cut
2. Album cover/Singer title and first song title are intersected.
3. Control buttons are cut.
4. Buttons in bottom toolbar are cut too.

Elisa is built from git (20180203T101741.75ceb39) in OBS.
openSUSE Tumbleweed x86_64, KDE Plasma 5.12 Beta (5.11.95).
Comment 1 Matthieu Gallien 2018-02-04 20:00:52 UTC
Currently, I am lacking proper hardware to test and fix issues.
A second important thing is that we are planning to move to Qt Quick Controls v2 only. Some of the fix for HighDPI would have be different in this case.
If someone from the Elisa team produces patches, could you test them ?
Comment 2 Lim Yuen Hoe 2018-02-09 05:47:40 UTC
I have a high dpi display and am seeing similar issues. I can test any related patches - though maybe not necessarily promptly :)
Comment 3 Mykola Krachkovsky 2018-02-09 09:13:11 UTC
Sorry, for response delay. Surely I can test patches. I could try even to make it if I'll get some hint where to start to look.
Comment 4 Mykola Krachkovsky 2018-02-11 10:38:06 UTC
Created attachment 110530 [details]
To make theme sizes depend on DPI

Hi, I've made something like a patch, not sure is it considered ok, but at least it makes Elisa great again ;) for HiDPI
Comment 5 Nate Graham 2018-02-11 14:50:02 UTC
Thank you for the patch! However, patches in bug report tend to get lost. Please submit it with Phabricator instead. See:

- https://community.kde.org/Get_Involved/Bug_Reporting#Submit_patches_using_Phabricator.2C_not_the_bug_tracker
- https://community.kde.org/Infrastructure/Phabricator
Comment 6 Mykola Krachkovsky 2018-02-12 10:32:10 UTC
Yes, ok I'll do it. But before I need to make patch correct. As I've found Screen.pixelDensity is physical DPI (or DPmm). But when logical DPI != physical DPI that's bad. And more, it's changed when screen is changed. And I can't find logical DPI in QML Screen object. So I'm not sure what's best option, to export logical dpi to QML: root context setContextProperty or call qmlRegisterType for wrapper around primary screen. Or maybe there is a simpler way to export function into QML.
Comment 7 Mykola Krachkovsky 2018-02-12 12:36:12 UTC
Hope, I've done it ok:
https://phabricator.kde.org/D10460
Comment 8 Mykola Krachkovsky 2018-02-12 13:16:16 UTC
PS for testing xrdb may be used:
xrdb -merge <<< 'Xft.dpi: 96'
xrdb -merge <<< 'Xft.dpi: 144'
or any other value.
As this value is system wide, it's better to revert to original after changes.
Comment 9 Nate Graham 2018-02-12 19:58:29 UTC
Thanks, you did great! The Elisa team will take a look at it as soon as they're able.
Comment 10 Matthieu Gallien 2018-02-12 21:22:00 UTC
Thanks a lot for your review on Phabricator.
Comment 11 Alexander Stippich 2018-02-22 08:39:56 UTC
Git commit 157c9f04f4f99b5591980fe97052682a3ca6963b by Alexander Stippich.
Committed on 21/02/2018 at 19:16.
Pushed by astippich into branch 'master'.

To make theme size depend on DPI

Summary:

Added logicalDpi property to use for scaling UI elements size in Theme.qml

Reviewers: #elisa, astippich

Reviewed By: astippich

Subscribers: astippich, asn, mgallien

Tags: #elisa

Maniphest Tasks: T7530

Differential Revision: https://phabricator.kde.org/D10460

M  +2    -0    src/main.cpp
M  +28   -23   src/qml/Theme.qml

https://commits.kde.org/elisa/157c9f04f4f99b5591980fe97052682a3ca6963b
Comment 12 Alexander Stippich 2018-02-24 14:13:18 UTC
Git commit ab81facebd458c515087ba7f3e734b9b2efde955 by Alexander Stippich, on behalf of Mykola Krachkovsky.
Committed on 24/02/2018 at 14:06.
Pushed by astippich into branch 'master'.

To make theme size depend on DPI

Summary:

Added logicalDpi property to use for scaling UI elements size in Theme.qml

Reviewers: #elisa, astippich

Reviewed By: astippich

Subscribers: astippich, asn, mgallien

Tags: #elisa

Maniphest Tasks: T7530

Differential Revision: https://phabricator.kde.org/D10460

M  +2    -0    src/main.cpp
M  +28   -23   src/qml/Theme.qml

https://commits.kde.org/elisa/ab81facebd458c515087ba7f3e734b9b2efde955