Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called "size_fdiag"

[Wolfgang Bauer]

> On Oct. 10, 2013, 1:21 p.m., Thomas Lübking wrote:
> > 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 ;-)
> 
> Wolfgang Bauer wrote:
>     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.
> 
> Thomas Lübking wrote:
>     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.
> 
> Wolfgang Bauer wrote:
>     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)
> 
> Thomas Lübking wrote:
>     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!"

> 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!


- Wolfgang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113185/#review41484
-----------------------------------------------------------


On Oct. 10, 2013, 11:51 a.m., Wolfgang Bauer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113185/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2013, 11:51 a.m.)
> 
> 
> Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking.
> 
> 
> Bugs: 325837
>     http://bugs.kde.org/show_bug.cgi?id=325837
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> 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).
> 
> 
> Diffs
> -----
> 
>   kcontrol/input/xcursor/xcursortheme.cpp 010c9ad 
> 
> Diff: http://git.reviewboard.kde.org/r/113185/diff/
> 
> 
> Testing
> -------
> 
> - 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
> 
> 
> File Attachments
> ----------------
> 
> KCM without the patch
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png
> KCM with the patch
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20131010/153f6269/attachment.htm>
-------------- next part --------------
_______________________________________________
kwin mailing list
kwin at kde.org
https://mail.kde.org/mailman/listinfo/kwin