<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/122086/">https://git.reviewboard.kde.org/r/122086/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On Januar 21st, 2015, 9:07 vorm. UTC, <b>Martin Gräßlin</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="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;">ping - do we want this approach to go in?</p></pre>
 </blockquote>







</blockquote>

<pre style="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;">I've two concerns about this.
1. the name "icons" implies "several" (it's unfortunate that ::icon isn't ::iconPixmap) - maybe "windowIcon()" or similar
2. the behavior how an icon is build up from multiple sources (isn't)
   right now, you could end up w/ one 16x16 NETWM icon pixmap.
   Considerations: NETWM and WM_HINTS could be more specific than an icon from the appname, the appname could though be more size complete and finally WM_HINTS would likely be "ugly" compared to the other two (bitmasked). Overmore it might be undesirable to end up w/ an icon that has completely different looks for different sizes.
   As we don't have a specific hint on the required target size, it gets harder to ever take an informed decision on what to return here, but one might want to at least check the sizes provided by NEWTM before returning early (if other sources are allowed by flags) - yes, I know that this is not a straight suggestion for improvement :-(</p></pre>
<br />










<p>- Thomas</p>


<br />
<p>On Januar 21st, 2015, 9:06 vorm. UTC, Martin Gräßlin wrote:</p>









<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 KDE Frameworks and kwin.</div>
<div>By Martin Gräßlin.</div>


<p style="color: grey;"><i>Updated Jan. 21, 2015, 9:06 vorm.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kwindowsystem
</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;">::icons tries to retrieve all available icon sizes and combine them
into a single QIcon. On the X11 platform this can significantly
reduce the time needed to fetch all available icons compared to the
already existing ::icon methods which return a QPixmap of a specific
size.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The change includes a new test application "icontest" which shows all
available icons in all available sizes for a window id passed as
command line argument.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags)</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/kwindowsystem.h <span style="color: grey">(322322f12dda7279567be8420533ed22cffffd52)</span></li>

 <li>src/kwindowsystem.cpp <span style="color: grey">(65d215b6dfbf4df22e871fd7187fface75abb61b)</span></li>

 <li>src/kwindowsystem_p.h <span style="color: grey">(1f01145b5c7efe925fcb8242f92af17b704e01c9)</span></li>

 <li>src/kwindowsystem_p_x11.h <span style="color: grey">(0d4b6ba551776d2fa08030f78226ecdb3c80c889)</span></li>

 <li>src/kwindowsystem_x11.cpp <span style="color: grey">(bf958ae63b48424fc412405259f082b740928f9a)</span></li>

 <li>tests/CMakeLists.txt <span style="color: grey">(ce68cc505a69ea9a3cf645e9ae587bd89abe1648)</span></li>

 <li>tests/icontest.cpp <span style="color: grey">(PRE-CREATION)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/122086/diff/" style="margin-left: 3em;">View Diff</a></p>






  </td>
 </tr>
</table>








  </div>
 </body>
</html>