Review Request 119983: Use font metrics to scale icons for high dpi outputs

[Martin Klapetek]

> On Aug. 29, 2014, 1:11 a.m., David Edmundson wrote:
> > src/declarativeimports/core/units.cpp, lines 197-198
> > <https://git.reviewboard.kde.org/r/119983/diff/1/?file=308044#file308044line197>
> >
> >     I'm confused.
> >     
> >     gridUnit is based on  QGuiApplication::font()
> >     
> >     and we generate a ratio comparing it against QGuiApplication::font()
> >     
> >     won't that always result in the same constant?

Not quite, the grid unit uses boundingRect.height(), which is in pixels, while this uses pointSize(), that's not in pixels. If I understand it correctly, same point size can have different pixel sizes depending on things.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119983/#review65459
-----------------------------------------------------------


On Aug. 29, 2014, 1:03 a.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119983/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 1:03 a.m.)
> 
> 
> Review request for Plasma, Kai Uwe Broulik, David Edmundson, and Vishesh Handa.
> 
> 
> Bugs: 337712 and 338308
>     http://bugs.kde.org/show_bug.cgi?id=337712
>     http://bugs.kde.org/show_bug.cgi?id=338308
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Use font metrics to scale icons for high dpi outputs
> 
> QScreen, through EDID reports bogus values for physicalDotsPerInch*().
> This leads to oversized icons on monitors with bogus edid information.
> 
> This patch changes the ratio underlying to the icon sizing for displays
> with a DPI > 96 * 1.5 to use the ratio between the font as rendered (its
> effective pixelSize, essentially) to scale the icon sizes up.
> 
> As we rely on proper font metrics throughout already, this should bring
> sizing in line with the font, which is something that makes sense as it
> means we're sharing the underlying mechanism (font metrics) for sizing
> in different areas.
> 
> The downside of this patch is that we're essentially working around an
> issue that should be fixed in the hardware, the monitor's edid.
> Unrealistic.
> 
> print dpi / sizing in dpitest
> 
> Print out some useful information to deduce dpi and pixel sizing.
> 
> David, and Kai Uwe, Vishesh (since I know you have a high DPI displays), 
> could you run this patch for a bit and check if it works for you, too?
> 
> I've pushed it to plasma-framework[sebas/dpi] for your git convenience.
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/units.h ba481781a04a54cb77f99048d3d400fdae617b38 
>   src/declarativeimports/core/units.cpp 56c0b55427c128beff5f8d18c37847a435f194c0 
>   tests/dpi/dpitest.cpp c3d2c3e6821fd79fc8b5ed0b3559a5870f88aa36 
> 
> Diff: https://git.reviewboard.kde.org/r/119983/diff/
> 
> 
> Testing
> -------
> 
> Ran Plasma Desktop, no apparent problems (on this hardware, which worked, anyway).
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140829/303e4b48/attachment-0001.html>