Bug 46261

Created attachment 57248 [details]
screenshot of the clipping errors

Created attachment 57249 [details]
demo application

The crash could well be related to the bad clip, as one side effect of inaccurate damage clipping is reading and writing out-of-bounds. So far I've seen sporadic examples of the unclipped lines, nothing that resembles the crash yet. I've a debug log to read through now and digest.

Thanks for the report.

Try with

commit bbb6794a3b97b1fcf72c8712ab0ec591683b128b
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Feb 20 12:25:31 2012 +0000

    sna: Trim clipped lines to end within bounds
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=46261
    References: https://bugs.freedesktop.org/show_bug.cgi?id=45673
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

commit 805bc3310cd0a13eab8e48e7615bdd42638cfa33
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Feb 20 12:09:19 2012 +0000

    sna: When reversing line-drawing direction, use the clipped endpoint
    
    Make sure we take the clipping into account if we choose to reverse the
    draw direction (to maintain left-to-right inside the box emission).
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=46261
    References: https://bugs.freedesktop.org/show_bug.cgi?id=45673
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I need to look through for unwanted side-effects, but the essence is that I really do need to overhaul the line drawing code so that I can be sure that it is correct.

Thanks, those patches made the behaviour better - but I sometimes still see single-pixel left-overs on the upper clipping boundary - and still experience crashes:

memcpy_blt (src=0xb059f000, dst=0xb6626008, bpp=4, src_stride=4096, dst_stride=2888, src_x=22, src_y=-1, dst_x=22, 
    dst_y=-1, width=1, height=1) at blt.c:195
195				*(uint32_t *)dst_bytes = *(uint32_t *)src_bytes;
(gdb) bt
#0  memcpy_blt (src=0xb059f000, dst=0xb6626008, bpp=4, src_stride=4096, dst_stride=2888, src_x=22, src_y=-1, dst_x=22, 
    dst_y=-1, width=1, height=1) at blt.c:195
#1  0x002a9050 in read_boxes_inplace (kgem=<optimized out>, bo=<optimized out>, src_dx=0, src_dy=0, pixmap=0xa23aa60, 
    dst_dx=0, dst_dy=0, box=0xa2903c8, n=582) at sna_io.c:103
#2  0x002a996d in sna_read_boxes (sna=0x9e4bd50, src_bo=0xa294fb8, src_dx=0, src_dy=0, dst=0xa23aa60, dst_dx=0, dst_dy=0, 
    box=0xa2903c8, nbox=582) at sna_io.c:156
#3  0x002846e3 in _sna_pixmap_move_to_cpu (pixmap=0xa23aa60, flags=2) at sna_accel.c:1037
#4  0x002861b7 in sna_drawable_move_region_to_cpu (drawable=0xa23aa60, region=0xbf82603c, flags=2) at sna_accel.c:1240
#5  0x0028b2cf in sna_copy_boxes (src=0xa23aa60, dst=0xa2f7198, gc=0xa2f7fb8, box=0xbf826164, n=1, dx=0, dy=0, reverse=0, 
    upsidedown=0, bitplane=0, closure=0x0) at sna_accel.c:3582
#6  0x081aa4bb in miCopyRegion (pSrcDrawable=0xa23aa60, pDstDrawable=0xa2f7198, pGC=0xa2f7fb8, pDstRegion=0xbf826164, dx=0, 
    dy=0, copyProc=0x28abb0 <sna_copy_boxes>, bitPlane=0, closure=0x0) at micopy.c:137
#7  0x081aa9a0 in miDoCopy (pSrcDrawable=0xa23aa60, pDstDrawable=0xa2f7198, pGC=0xa2f7fb8, xIn=0, yIn=0, widthSrc=722, 
    heightSrc=544, xOut=0, yOut=0, copyProc=0x28abb0 <sna_copy_boxes>, bitPlane=0, closure=0x0) at micopy.c:334
#8  0x0028a91b in sna_copy_area (src=0xa23aa60, dst=0xa2f7198, gc=0xa2f7fb8, src_x=0, src_y=0, width=722, height=544, 
    dst_x=0, dst_y=0) at sna_accel.c:3766
#9  0x08159c57 in damageCopyArea (pSrc=0xa23aa60, pDst=0xa2f7198, pGC=0xa2f7fb8, srcx=0, srcy=0, width=722, height=544, 
    dstx=0, dsty=0) at damage.c:864
#10 0x08071c86 in ProcCopyArea (client=0xa1ff2e8) at dispatch.c:1651
#11 0x08076195 in Dispatch () at dispatch.c:439
#12 0x0806439a in main (argc=6, argv=0xbf826444, envp=0xbf826460) at main.c:287

Created attachment 57343 [details]
screenshot of small single-pixel leftovers

Closer, that crash is definitely related to the single pixel above the clip box so hopefully down to the last bug in that loop. ;-)

just to be curious: I saw those loops operate on single-pixel-data only and only fallback to memcpy for unknown formats.
Wouldn't it be more efficient to call memcpy always except for small areas - as it could use all the hand-optimized assembler tricks there?

Since the compiler can not perform constant folding on the memcpy length, it always emits a PLT call for memcpy(), which is why special casing the common tiny copies is actually a win.

commit 1a65e2b8a2ebfb4d736efb7631515babad75faf2
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Feb 21 08:09:52 2012 +0000

    sna: Split up/down edge walking in order to handle endpoint clipping
    
    In order to prevent walking upwards off the top of the pixmap when
    rendering a clipped vertical edge, we need to tweak the boundary
    conditions for the vertical edge walker.
    
    Reported-by: Clemens Eisserer <linuxhippy@gmail.com>
    References: https://bugs.freedesktop.org/show_bug.cgi?id=46261
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks - crash and rendering problems fixed :)

Fingers crossed that's the last time I have to look at that function... For at least a day. ;-)

If I can solve the mystery of the white glyphs, then I can be reasonably happy.