Re: [Qemu-devel] [PATCH] Missing cirrus_invalidate_region() in cirrus_do_copy()

[Brian Kress]

Alexander Graf wrote:
> On 16.02.2009, at 23:09, Brian Kress <address@hidden> wrote:
>
> > After doing a video to video copy, cirrus_do_copy() in cirrus_vga.c 
> > does a conditional
> > call to cirrus_invalidate_region() with an "if (!notify)" test.  
> > However at this point the blt
> > has already been done so it seems like this call should be made 
> > unconditionally.  The patch
> > below fixes some display update problems when dragging windows under 
> > both X (fedora
> > 10 guest) and a Windows XP guest.
>
> Hm - I've run into this too, exposed the most using a gnome guest on 
> vnc using copyrect.
>
> Basically from what I understood, the conditional is
>
> If display supports copyrect
>   tell it to copy
> Else
>   draw yourself
   That logic is done in vnc.c and is still being done.  The vnc
viewer is still being sent the copyrect, it's just getting overwritten
on the next update because the display surface is not updated.  The
data in s->ds needs to be updated, either by vnc_copy() or by
vga_hw_update() (which is what this patch does).  An alternate way to
fix this would be to do a memmove from the source rectangle to the
destination rectangle in vnc_copy(), which I think is what previous
versions of the copyrect code did, but it got taken out for some
reason, which is what triggered this bug.
> And with this patch you're basically reverting to 'draw yourself' 
> always, no?
>
> I boiled things far enough to see that the blt addresses were 
> completely off. The next idea on my list was to check if this is a 
> Linux driver issue by running the same image on a real cirrus logic.
>
> Alex
   The blt addresses being off seemed to be a separate issue caused by
assuming pitch == width * depth.  I already fixed that in another patch,
which has already been applied.  That one did show up only on Linux,
the one this patch is targeted at affects both.