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