![](https://seccdn.libravatar.org/avatar/129c5b7244f2b7b09a1348ad3d488c57.jpg?s=120&d=mm&r=g)
On Apr 29, 09 19:50:52 -0700, Yang Zhao wrote:
Attached patch is a refactoring of the RandR-based cursor code to be cleaner and faster. Fix for the cursor curruption issue is also included. (http://bugs.freedesktop.org/show_bug.cgi?id=13405)
Thanks for working on this, Yang!
The non-RandR code path has not been touched, and will still exhibit the bug. The plan is to then rewrite that code path to use the new, cleaner code, which should make everything easier to understand and reduce the number of unnecessary register writes and conditional branches. I don't think a lot of people are using a non-RandR setup, so this should satisfy almost everybody that's currently experiencing cursor issues.
Fair enough :)
- Code was added to correctly set cursor dimensions. This is disabled as there are apparently no benefits on doing so.
I would actually remove that (unused) code. Yes, I stumbled over the Xserver loading MAX by MAX only as well.
@@ -636,11 +644,26 @@ rhdCrtcShowCursor(struct rhdCrtc *Crtc) [...] + /* Move cursor off-screen so it won't be visible*/ + RHDRegWrite(Cursor, Cursor->RegOffset + D1CUR_POSITION, + (Crtc->X + Crtc->Width) << 16 | (Crtc->Y + Crtc->Height)); + RHDRegWrite(Cursor, Cursor->RegOffset + D1CUR_HOT_SPOT, 0); +
Is this consistent with the solution Alex found for how to fix part of the corruptions (never move outside Crtc boundaries + don't let end at multiples of 128)? One way to circumvent this would be to use (0, Crtc->Y+Crtc->Height) as coordinates. What do you think?
rhdCrtcSetCursorPosition(struct rhdCrtc *Crtc, int x, int y) [...] - displayCursor(Crtc, TRUE); + RHDRegWrite(Cursor, Cursor->RegOffset + D1CUR_POSITION, x << 16 | y); + RHDRegWrite(Cursor, Cursor->RegOffset + D1CUR_HOT_SPOT, hotx << 16 | hoty);
This doesn't change cursor width any more if necessary (see above). That
has been the reason for the change to use displayCursor everywhere. It
does fix the cursor corruption issues (while it introduced other issues ;)
on a machine here, so I'm positive that this is actually necessary.
Matthias
--
Matthias Hopf