Differential D10357

Show high-resolution and vector logos properly in HighDPI mode
ClosedPublic
Actions

Authored by ngraham on Feb 7 2018, 5:17 AM.

Details

Reviewers
davidedmundson
Group Reviewers
Plasma
Commits
R102:37624eb6073f: SVN_SILENT made messages (.desktop file) - always resolve ours
R102:82e64226c2e6: Show high-resolution and vector logos properly in HighDPI mode
R102:fdd2fc881157: Show high-resolution and vector logos properly in HighDPI mode
Summary

BUG: 388633
FIXED-IN 5.12.1

Take the device pixel ratio into consideration when rendering the logo, so it looks nice in HighDPI mode.

Test Plan

Tested in NDE Neon

  • 1x scale factor, low-res pixmap: no change, looks fine
  • >1x scale factor, low-res pixmap: no change, looks pixellated
  • 1x scale factor, vector pixmap: no change, looks fine
  • >1x scale factor, vector pixmap: looks great!

Before:

After:

Diff Detail

Repository
R102 KInfoCenter
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
ngraham created this revision.Feb 7 2018, 5:17 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 7 2018, 5:17 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Feb 7 2018, 5:17 AM
ngraham edited the test plan for this revision. (Show Details)Feb 7 2018, 5:19 AM
broulik added a subscriber: broulik.Feb 7 2018, 8:38 AM
broulik added inline comments.
Modules/about-distro/src/Module.cpp
145

Did you check how it behaves with a distro-provided pixmap? I fear it would become tiny. I don't think we have any kind of @2x support for that

main.cpp
2

Unrelated

40

Shouldn't it be Qt::AA_UseHighDpiPixmaps?

From what I understood the Qt::AA_EnableHighDpiScaling flag forces high dpi support regardless of environment variables or platform theme.

davidedmundson added a subscriber: davidedmundson.Feb 7 2018, 11:34 AM
davidedmundson added inline comments.
Modules/about-distro/src/Module.cpp
145

once you have UseHighDpiPixmaps you won't need this code, Qt will do it automatically, including handling your pixmap case.

main.cpp
40

also it would need to be called before the QApplication constructor

broulik added inline comments.Feb 7 2018, 11:56 AM
main.cpp
40

Never did that,

app.setAttribute(Qt::AA_UseHighDpiPixmaps, true);

works just fine and the docs also don't mention it

ngraham updated this revision to Diff 26699.Feb 7 2018, 2:02 PM

Simplify the code and address review comments

ngraham marked 4 inline comments as done.Feb 7 2018, 2:02 PM
ngraham added inline comments.
Modules/about-distro/src/Module.cpp
145

Yes, it still works fine with distro-provided pixmals, at both 1x scale and > 1x scale.

davidedmundson accepted this revision.Feb 7 2018, 2:03 PM
This revision is now accepted and ready to land.Feb 7 2018, 2:03 PM
ngraham edited the test plan for this revision. (Show Details)Feb 7 2018, 2:03 PM
ngraham edited the summary of this revision. (Show Details)
Closed by commit R102:82e64226c2e6: Show high-resolution and vector logos properly in HighDPI mode (authored by ngraham). · Explain WhyFeb 7 2018, 2:08 PM
This revision was automatically updated to reflect the committed changes.
cfeck added a comment.Feb 7 2018, 3:47 PM

Suggesting Qt:AA_EnableHighDpiScaling was my fault; I really should have checked our applications to suggest the correct attribute.

ngraham added a comment.Feb 7 2018, 5:26 PM

No worries, it sent me down the right path, at least!

Revision Contents
Changeset List

PathPackages
MModules/about-distro/src/Module.cpp (4 lines)
Mmain.cpp (2 lines)

Diff 26685

View Options

Modules/about-distro/src/Module.cpp

Loading...
View Options

main.cpp

Loading...