Bug 1030

Summary: New damage sprite is duplicated when scrolling (XCopyArea)
Product: xorg Reporter: Kristian Høgsberg <krh>
Component: Server/GeneralAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: high CC: keithp, kem
Version: git   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 269, 351    
Attachments:
Description Flags
patch to report src+dst as damage arae
none
add SourceValidate() call to damageCopyArea() and damageCopyPlane() none

Description Kristian Høgsberg 2004-08-10 17:59:13 UTC
With the new damage based sprite, sometimes when scrolling a document under the
sprite, the sprite image is duplicated.  To reproduce, scroll almost to the end
of a document in e.g. firefox.  Leave the last half screen of the document below
the window.  Place the cursor in the lower half of the document window and press
page-down.  The point is that the cursor should be above a part of the document
that will still be visible when the document is scrolled, and after scrolling,
the cursor should be over a part of the document that was not previously visible.

The problem must be that the sprite code does not receive a damage callback for
the source area of a XCopyArea().  This makes sense, but then it is not able to
remove the sprite before the area is copied, and the sprite is copied with the
area.  If the sprite also intersects to destination area, it works fine because
the sprite code receives a callback in this case; this explains the behaviour
described above.
Comment 1 Kristian Høgsberg 2004-08-10 18:49:59 UTC
Created attachment 597 [details] [review]
patch to report src+dst as damage arae

The attached patch changes the XCopyArea() handling in damage to report the
union of the source and the destination area as the damaged area.  This fixes
the problem, but it doesn't feel right...
Comment 2 Keith Packard 2004-08-10 19:44:28 UTC
No, that's not the right fix.  The 'SourceValidate' function needs to be called
sometime along the CopyArea path so that the sprite code can pull down the cursor.

That's guaranteed to be called by the driver when copying between different
drawables, but is never called when copying between the same drawable.  I hacked
the kdrive-based servers to just always call it, but we can't depend on that
behaviour now.  It used to be "ok" in this case because the misprite code would
see every CopyArea on the screen.

I suspect the easiest fix is to hack the Damage code to call SourceValidate
from the CopyArea and CopyPlane functions when the source and dest are the same
and leave the different-drawable call to the DDX itself.  Yes, this is a kludge.

Suggestions are welcome.
Comment 3 Kristian Høgsberg 2004-08-11 12:21:16 UTC
These files call ValidateSource

  afb/afbbitblt.c:199
  cfb/cfbbitblt.c:117
  cfb/cfbbitblt.c:380
  cfb16/cfbbitblt.c:117
  cfb16/cfbbitblt.c:380
  cfb24/cfbbitblt.c:117
  cfb24/cfbbitblt.c:380
  cfb32/cfbbitblt.c:117
  cfb32/cfbbitblt.c:380
  fb/fbcopy.c:420
  hw/xfree86/xaa/xaaBitBlt.c:57
  hw/xfree86/xf4bpp/ppcCpArea.c:267
  ilbm/ilbmbitblt.c:204
  iplan2p4/iplbitblt.c:94
  mfb/mfbbitblt.c:169

What would be the problem in changing those callers to always call
ValidateSource, regardless of wether src != dst?  

Is it also called from closed source drivers that we can't change?  Or is it a
problem to change the semantics of ValidateSource?
Comment 4 Kristian Høgsberg 2004-08-11 12:29:07 UTC
Hmm... changing blocked bug from #315 to #351 ;-)
Comment 5 Kristian Høgsberg 2004-08-11 18:53:28 UTC
Created attachment 612 [details] [review]
add SourceValidate() call to damageCopyArea() and damageCopyPlane()

As discussed with keithp, I'm committing this patch and closing the bug.  It
may be better to require the drivers to call SourceValidate in both == and !=
cases, but for now this is the fix.
Comment 6 Kristian Høgsberg 2004-08-11 18:53:56 UTC
Closing bug.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.