[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 119983: Use font metrics to scale icons for high dpi outputs
From: Sebastian_Kügler <sebas () kde ! org>
Date: 2015-04-13 10:38:29
Message-ID: 20150413103829.4623.50406 () mimi ! kde ! org
[Download RAW message or body]
--===============4061085260912066060==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119983/
-----------------------------------------------------------
(Updated April 13, 2015, 10:38 a.m.)
Status
------
This change has been discarded.
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
--===============4061085260912066060==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119983/">https://git.reviewboard.kde.org/r/119983/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been \
discarded.</h1> </td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma, Kai Uwe Broulik, David Edmundson, and Vishesh \
Handa.</div> <div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated April 13, 2015, 10:38 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=337712">337712</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=338308">338308</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-framework
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Use font metrics to scale icons for high dpi \
outputs</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">QScreen, through EDID reports bogus values for \
physicalDotsPerInch*(). This leads to oversized icons on monitors with bogus edid \
information.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">print dpi / sizing in dpitest</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Print \
out some useful information to deduce dpi and pixel sizing.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">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?</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">I've pushed it to plasma-framework[sebas/dpi] for your git \
convenience.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Ran Plasma Desktop, no apparent problems (on this \
hardware, which worked, anyway).</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/declarativeimports/core/units.h <span style="color: \
grey">(ba481781a04a54cb77f99048d3d400fdae617b38)</span></li>
<li>src/declarativeimports/core/units.cpp <span style="color: \
grey">(56c0b55427c128beff5f8d18c37847a435f194c0)</span></li>
<li>tests/dpi/dpitest.cpp <span style="color: \
grey">(c3d2c3e6821fd79fc8b5ed0b3559a5870f88aa36)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119983/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============4061085260912066060==--
[Attachment #3 (text/plain)]
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic