![](https://seccdn.libravatar.org/avatar/aa24984c190be65ba9054306350706f9.jpg?s=120&d=mm&r=g)
2009/5/5 Matthias Hopf
On May 04, 09 15:27:00 -0700, Yang Zhao wrote:
0002 is the refactoring. It's almost trivial now that we don't need the logic to keep the cursor enabled on both CRTCs.
diff --git a/src/rhd_cursor.c b/src/rhd_cursor.c index 3a0c7a9..a2b52b1 100644 --- a/src/rhd_cursor.c +++ b/src/rhd_cursor.c @@ -625,10 +625,9 @@ void rhdCrtcShowCursor(struct rhdCrtc *Crtc) { struct rhdCursor *Cursor = Crtc->Cursor; - - lockCursor (Cursor, TRUE); - displayCursor(Crtc, TRUE); - lockCursor (Cursor, FALSE); + lockCursor(Cursor, TRUE); + enableCursor(Cursor, TRUE); + lockCursor(Cursor, FALSE);
I don't like the whitespace changes - IMHO it's much harder to read.
Fixed.
@@ -651,11 +649,27 @@ void rhdCrtcSetCursorPosition(struct rhdCrtc *Crtc, int x, int y) { struct rhdCursor *Cursor = Crtc->Cursor; + int hotx, hoty;
What I don't really like here is the duplication of hotspot calculation. That has been the reason behind displayCursor() in the first place. Not really sure what to do about that, it's probably best to first apply this and then refactor the logic.
displayCursor() currently contains pretty much everything: position setting, size adjustment, and visibility toggle, along with logic to determine if the cursor is within each CRTC's visible area. For RandR cursor, this duplicates a lot of the work the server is already doing. Once the new RandR cursor code is in place, the idea is to refactor the non-RandR cursor code such that we check which CRTC is affected, then simply call the appropriate rhdCrtc*() function.
Also please separate logic and register access, so setCursorPos() should be used instead of direct RHDRegWrite(). It's a small static function, so it will be inlined anyway.
Done. I seem to have forgotten setCursorPos() existed. Revised 0002 and 0003 attached. -- Yang Zhao http://yangman.ca