[Spice-devel] [PATCH qxl-win] display: apply the fix in fc314927bc48835e to Alpha Bitmaps

Alon Levy alevy at redhat.com
Sun Jul 7 01:07:51 PDT 2013


On Fri, 2013-07-05 at 11:25 -0400, Yonit Halperin wrote:
> rhbz#968050

Looks great to me, ACK.

> 
> In contrast to Microsoft Msdn documentation, the iUniq of a SURFOBJ doesn't
> always change when the surface changes. However, it seems that the
> iUniq of the associated color_trans (XLATEOBJ) changes, while its
> flXlate=XO_TRIVIAL. Since we tried to retrieve the alpha bitmap key
> only by the surface iUniq, we fetched the wrong bitmap, and it looked
> like parts of the screen haven't been rendered.
> 
> The patch modifies QXLGetAlphaBitmap so that it will use GetCacheImage
> instead of duplicating its code. GetCacheImage was already fixed in
> fc314927bc48835e to combine the iUniq of the surace and the
> color_trans.
> ---
>  xddm/display/res.c | 55 ++++++++++++++----------------------------------------
>  xddm/display/res.h |  2 +-
>  xddm/display/rop.c |  2 +-
>  3 files changed, 16 insertions(+), 43 deletions(-)
> 
> diff --git a/xddm/display/res.c b/xddm/display/res.c
> index 6f04475..bfb3571 100644
> --- a/xddm/display/res.c
> +++ b/xddm/display/res.c
> @@ -2070,7 +2070,8 @@ BOOL QXLCheckIfCacheImage(PDev *pdev, SURFOBJ *surf, XLATEOBJ *color_trans)
>      return FALSE;
>  }
>  
> -static CacheImage *GetCacheImage(PDev *pdev, SURFOBJ *surf, XLATEOBJ *color_trans, int high_bits_set, UINT32 *hash_key)
> +static CacheImage *GetCacheImage(PDev *pdev, SURFOBJ *surf, XLATEOBJ *color_trans,
> +                                 BOOL has_alpha, BOOL high_bits_set, UINT32 *hash_key)
>  {
>      CacheImage *cache_image;
>      UINT64 gdi_unique;
> @@ -2085,6 +2086,10 @@ static CacheImage *GetCacheImage(PDev *pdev, SURFOBJ *surf, XLATEOBJ *color_tran
>          return FALSE;
>      }
>  
> +    if (has_alpha) {
> +        ASSERT(pdev, surf->iBitmapFormat == BMF_32BPP);
> +        format = SPICE_BITMAP_FMT_RGBA;
> +    }
>      if (!ImageKeyGet(pdev, surf->hsurf, gdi_unique, &key)) {
>          key = GetHash(surf->pvScan0, surf->sizlBitmap.cx, surf->sizlBitmap.cy, format,
>                        high_bits_set, line_size, surf->lDelta, color_trans);
> @@ -2225,7 +2230,7 @@ BOOL QXLGetBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phys, SU
>                                   &high_bits_set) &&
>              !high_bits_set) {
>              return QXLGetAlphaBitmap(pdev, drawable, image_phys,
> -                                     surf, area, surface_dest);
> +                                     surf, area, surface_dest, color_trans);
>          }
>      }
>  
> @@ -2238,7 +2243,7 @@ BOOL QXLGetBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phys, SU
>                   surf->iBitmapFormat));
>  
>      if (use_cache) {
> -        cache_image = GetCacheImage(pdev, surf, color_trans, high_bits_set, hash_key);
> +        cache_image = GetCacheImage(pdev, surf, color_trans, FALSE, high_bits_set, hash_key);
>          if (cache_image && cache_image->image) {
>              DEBUG_PRINT((pdev, 11, "%s: cached image found %u\n", __FUNCTION__, cache_image->key));
>              internal = cache_image->image;
> @@ -2331,7 +2336,7 @@ BOOL QXLGetBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phys, SU
>  }
>  
>  BOOL QXLGetAlphaBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phys,
> -                       SURFOBJ *surf, QXLRect *area, INT32 *surface_dest)
> +                       SURFOBJ *surf, QXLRect *area, INT32 *surface_dest, XLATEOBJ *color_trans)
>  {
>      Resource *image_res;
>      InternalImage *internal;
> @@ -2347,10 +2352,11 @@ BOOL QXLGetAlphaBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phy
>             area->top >= 0 && area->bottom <= surf->sizlBitmap.cy);
>  
>      DEBUG_PRINT((pdev, 11, "%s: iUniq=%x DONTCACHE=%x w=%d h=%d cx=%d cy=%d "
> -                 "hsurf=%x format=%u\n", __FUNCTION__, surf->iUniq,
> +                 "hsurf=%x format=%u color_trans->iUniq=%x\n", __FUNCTION__, surf->iUniq,
>                   surf->fjBitmap & BMF_DONTCACHE, width, height,
>                   surf->sizlBitmap.cx, surf->sizlBitmap.cy, surf->hsurf,
> -                 surf->iBitmapFormat));
> +                 surf->iBitmapFormat,
> +                 color_trans ? color_trans->iUniq : 0));
>  
>      if (surf->iType != STYPE_BITMAP) {
>          UINT32 alloc_size;
> @@ -2381,30 +2387,9 @@ BOOL QXLGetAlphaBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phy
>      }
>  
>      ASSERT(pdev, surf->iBitmapFormat == BMF_32BPP && surf->iType == STYPE_BITMAP);
> +    cache_image = GetCacheImage(pdev, surf, color_trans, TRUE, FALSE, &key);
>  
> -    //todo: use GetCacheImage
> -
> -    // NOTE: Same BMF_DONTCACHE issue as in QXLGetBitmap
> -    if (!surf->iUniq || (surf->fjBitmap & BMF_DONTCACHE)) {
> -        gdi_unique = 0;
> -    } else {
> -        gdi_unique = surf->iUniq;
> -    }
> -
> -    if (!ImageKeyGet(pdev, surf->hsurf, gdi_unique, &key)) {
> -        key = GetHash(surf->pvScan0, surf->sizlBitmap.cx, surf->sizlBitmap.cy, SPICE_BITMAP_FMT_RGBA,
> -                      FALSE, surf->sizlBitmap.cx << 2, surf->lDelta, NULL);
> -        ImageKeyPut(pdev, surf->hsurf, gdi_unique, key);
> -        DEBUG_PRINT((pdev, 11, "%s: ImageKeyPut %u\n", __FUNCTION__, key));
> -    } else {
> -        DEBUG_PRINT((pdev, 11, "%s: ImageKeyGet %u\n", __FUNCTION__, key));
> -    }
> -
> -    if (cache_image = ImageCacheGetByKey(pdev, key, TRUE, SPICE_BITMAP_FMT_RGBA,
> -                                         surf->sizlBitmap.cx, surf->sizlBitmap.cy)) {
> -        DEBUG_PRINT((pdev, 11, "%s: ImageCacheGetByKey %u hits %u\n", __FUNCTION__,
> -                     key, cache_image->hits));
> -        cache_image->hits++;
> +    if (cache_image) {
>          if (internal = cache_image->image) {
>              DEBUG_PRINT((pdev, 11, "%s: cached image found %u\n", __FUNCTION__, key));
>              *image_phys = PA(pdev, &internal->image, pdev->main_mem_slot);
> @@ -2412,18 +2397,6 @@ BOOL QXLGetAlphaBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phy
>              DrawableAddRes(pdev, drawable, image_res);
>              return TRUE;
>          }
> -    } else if (CacheSizeTest(pdev, surf)) {
> -        CacheImage *cache_image;
> -        cache_image = AllocCacheImage(pdev);
> -        ImageCacheRemove(pdev, cache_image);
> -        cache_image->key = key;
> -        cache_image->image = NULL;
> -        cache_image->format = SPICE_BITMAP_FMT_RGBA;
> -        cache_image->width = surf->sizlBitmap.cx;
> -        cache_image->height = surf->sizlBitmap.cy;
> -        ImageCacheAdd(pdev, cache_image);
> -        RingAdd(pdev, &pdev->cache_image_lru, &cache_image->lru_link);
> -        DEBUG_PRINT((pdev, 11, "%s: ImageCacheAdd %u\n", __FUNCTION__, key));
>      }
>  
>      if (cache_image) {
> diff --git a/xddm/display/res.h b/xddm/display/res.h
> index d69986e..6ce9a68 100644
> --- a/xddm/display/res.h
> +++ b/xddm/display/res.h
> @@ -47,7 +47,7 @@ BOOL QXLGetBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phys, SU
>                    INT32 *surface_dest);
>  BOOL QXLGetBitsFromCache(PDev *pdev, QXLDrawable *drawable, UINT32 hash_key, QXLPHYSICAL *image_phys);
>  BOOL QXLGetAlphaBitmap(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *image_phys, SURFOBJ *surf,
> -                       QXLRect *area, INT32 *surface_dest);
> +                       QXLRect *area, INT32 *surface_dest, XLATEOBJ *color_trans);
>  BOOL QXLCheckIfCacheImage(PDev *pdev, SURFOBJ *surf, XLATEOBJ *color_trans);
>  UINT8 *QXLGetBuf(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *buf_phys, UINT32 size);
>  BOOL QXLGetStr(PDev *pdev, QXLDrawable *drawable, QXLPHYSICAL *str_phys, FONTOBJ *font, STROBJ *str);
> diff --git a/xddm/display/rop.c b/xddm/display/rop.c
> index 9fb3527..a77a912 100644
> --- a/xddm/display/rop.c
> +++ b/xddm/display/rop.c
> @@ -1647,7 +1647,7 @@ BOOL APIENTRY DrvAlphaBlend(SURFOBJ *dest, SURFOBJ *src, CLIPOBJ *clip, XLATEOBJ
>          ASSERT(pdev, src->iBitmapFormat == BMF_32BPP);
>          if (!QXLGetAlphaBitmap(pdev, drawable, &drawable->u.alpha_blend.src_bitmap, src,
>                                 &drawable->u.alpha_blend.src_area,
> -                               &drawable->surfaces_dest[0])) {
> +                               &drawable->surfaces_dest[0], color_trans)) {
>              DEBUG_PRINT((pdev, 0, "%s: QXLGetAlphaBitmap failed\n", __FUNCTION__));
>              ReleaseOutput(pdev, drawable->release_info.id);
>              return FALSE;




More information about the Spice-devel mailing list