don't multiply svg sizes by devicepixelratio
ClosedPublic

Authored by mart on Dec 11 2017, 1:05 PM.

Details

Summary

when the native high dpi scaling is enabled, some things gets scaled two times,
as sizes taken from svgs should *not* be manually scaled by devicePixelRatio

Test Plan

works correctly on wayland, svg.elementSize will have to be tweaked in x11 to
return a value scaled by scalefactor, to homogenize the behavior compared to
native qt high dpi scaling.
to return a value scaled by scalefactor for this qml code to work there too.
I advise everybody with high dpi laptops to test this on both wayland and x11

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Dec 11 2017, 1:05 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptDec 11 2017, 1:05 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
mart requested review of this revision.Dec 11 2017, 1:05 PM
broulik added a subscriber: broulik.Jan 4 2018, 3:20 PM

This fixes the gigantig scroll bars I had in plasmashell on X with 2x scaling.

hein added a subscriber: hein.

I think this needs David

I handle scaling everyewhere except for Plasma which does it's own thing.


I'm not convinced by this patch:

If this was Qt's devicePixelRatio this would definitely 100% make sense as there we write all UI code in logical sizes.

But in Plasma it's up to the developer to convert everything from logical pixles to devicePixels themselves, so it does makes sense to multiply surely?

On wayland we have both systems going at once.
Which sounds super broken, but it just about works because Units.devicePixelRatio is (implicitly) divided through by Qt's devicePixelRatio.

Units.devicePixelRatio should be exactly 1, because (unless explicitly overridden) our logical DPI is set to 96. (96/96 =1)

So this patch shouldn't be affecting anything?

davidedmundson accepted this revision.Jan 23 2018, 11:57 AM

I'm 100% convinced Plasma needs to multiply it as it does everything in device pixels.

However:

void FrameSvgItem::updateDevicePixelRatio()

m_frameSvg->setScaleFactor(qMax<qreal>(1.0, floor(Units::instance().devicePixelRatio())));

this changes elementSize

So with this code and that we're doing it twice. You're right one of them should die.

This revision is now accepted and ready to land.Jan 23 2018, 11:57 AM
This revision was automatically updated to reflect the committed changes.