<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113185/">http://git.reviewboard.kde.org/r/113185/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 10th, 2013, 11:21 a.m. UTC, <b>Thomas Lübking</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;">https://bugs.kde.org/show_bug.cgi?id=54359
If Fredrik's right, the tutorial used by most cursor creators would be wrong (great...)
I guess to be absolutely sure, we'll have to XCreateFontCursor and then somehow get the bitmaps hash (XGetImage and on the bits? No idea...)
For the moment i'll trust Fredrik more ;-)</pre>
</blockquote>
<p>On October 10th, 2013, 11:32 a.m. UTC, <b>Wolfgang Bauer</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;">Well, according to https://bugs.kde.org/show_bug.cgi?id=248599#c6, size_fdiag in QCursor does have the hash c7088f0f3e6c8088236ef8e1e3e70000, so my patch would be correct.
I haven't verified that, though.</pre>
</blockquote>
<p>On October 10th, 2013, 11:40 a.m. UTC, <b>Thomas Lübking</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;">The cursor in cur_fdiag_bits displays the top_left_corner bits (nw->se) what Fredrik claims to be wrong (and for "forward" i'd indeed expect sw->ne)
The output for the provided applet is:
size_fdiag hash: c7088f0f3e6c8088236ef8e1e3e70000
size_bdiag hash: fcf1c3c7cd4491d801f1e1c78f100000
So the top_left_corner hash would be c7088f0f3e6c8088236ef8e1e3e70000, what should also be size_bdiag.</pre>
</blockquote>
<p>On October 10th, 2013, noon UTC, <b>Wolfgang Bauer</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;">You mean the arrows from the top-left to the bottom-right corner?
But then it is wrong in the Oxygen themes and KDE_Classic as shipped with KDE, size_bdiag is from bottom-left to top-right in both themes (and size_fdiag from top-left to bottom-right)</pre>
</blockquote>
<p>On October 10th, 2013, 12:21 p.m. UTC, <b>Thomas Lübking</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;">Yes, and that also matches the preview image for Qt::SizeBDiagCursor ...
So, given size_fdiag is indeed top_left_corner, ie. nw -> se and size_bdiag is sw -> ne ie. top_right_corner, the present hashes are wrong and the fix correct.
Given that we read nw -> se that actually makes sense and matches Qt's coordinate system (where forward = sw -> ne is more the Math/GL coordinate system)
However, Qt announces Qt::SizeBDiagCursor as sw -> ne and that is what users should get.
Wait 24h to see whether Fredrik has an additional comment on it and otherwise "ShipIt!"</pre>
</blockquote>
<p>On October 10th, 2013, 12:36 p.m. UTC, <b>Wolfgang Bauer</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;">> However, Qt announces Qt::SizeBDiagCursor as sw -> ne and that is what users should get.
Sorry, I don't get that comment, why the "However"?
That's the same statement as in the other sentences. Or is there a typo somewhere?
But, of course, I will wait those 24h hours before committing.
Thanks for your review!</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;">"However [that may be]" -> "Whatsoever"
Sidenote: neutral++ has fd_double_arrow -> top_right_corner but size_fdialog -> top_left_corner
This is a complete mess =D</pre>
<br />
<p>- Thomas</p>
<br />
<p>On October 10th, 2013, 9:51 a.m. UTC, Wolfgang Bauer wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking.</div>
<div>By Wolfgang Bauer.</div>
<p style="color: grey;"><i>Updated Oct. 10, 2013, 9:51 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=325837">325837</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</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;">Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for "size_bdiag" and "size_fdiag" are swapped, so for themes not containing "size_fdiag" the wrong resize cursor is shown in the preview.
This patch fixes that long standing bug. (there has been no change to that function since 2007!)
This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session).</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;">- Enter systemsettings->Workspace Appearance->Cursor Theme
- Select a theme without "size_fdiag", f.e.: crystalwhite, DMZ, Adwaita
- Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g.
See atached screenshots</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>kcontrol/input/xcursor/xcursortheme.cpp <span style="color: grey">(010c9ad)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113185/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png">KCM without the patch</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png">KCM with the patch</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>