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

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

2013-07-07 Thread Alon Levy

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);
 -

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

2013-07-07 Thread Uri Lublin


On 07/05/2013 06:25 PM, Yonit Halperin wrote:

rhbz#968050

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.


Ack.


---
  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,

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

2013-07-05 Thread Yonit Halperin

rhbz#968050

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));
-}
-
-

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

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

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

3 matches

Site Navigation

Mail list logo

Footer information