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;
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel