[Spice-devel] [PATCH 08/11] spicec: add sw canvas diff check

Alon Levy alevy at redhat.com
Tue Sep 24 06:16:49 PDT 2013


On 09/24/2013 02:42 PM, Marc-André Lureau wrote:


Looks good, but can you explain why you disable diff check for fill, text?
> ---
>   client/display_channel.cpp | 88 +++++++++++++++++++++++++++++++++-------------
>   client/display_channel.h   |  2 ++
>   client/red_pixmap.h        | 19 ++++++++++
>   client/utils.h             |  3 ++
>   4 files changed, 88 insertions(+), 24 deletions(-)
>
> diff --git a/client/display_channel.cpp b/client/display_channel.cpp
> index 49a4c6a..c9ce3d8 100644
> --- a/client/display_channel.cpp
> +++ b/client/display_channel.cpp
> @@ -611,6 +611,7 @@ DisplayChannel::DisplayChannel(RedClient& client, uint32_t id,
>   #endif
>       , _interrupt_update (*this)
>       , _mig_wait_primary (false)
> +    , _check_diff (getenv("CHECKDIFF") != NULL)
>   {
>       DisplayHandler* handler = static_cast<DisplayHandler*>(get_message_handler());
>   
> @@ -1276,6 +1277,22 @@ void DisplayChannel::create_canvas(int surface_id, const std::vector<int>& canva
>       if (i == canvas_types.size()) {
>           THROW("create canvas failed");
>       }
> +
> +    try {
> +        if (!_check_diff)
> +            return;
> +
> +        SCanvas *canvas = new SCanvas(surface_id == 0, width, height, format,
> +                                      screen()->get_window(),
> +                                      _pixmap_cache, _palette_cache, _glz_window,
> +                                      _surfaces_cache);
> +        _swsurfaces_cache[surface_id] = canvas;
> +        if (surface_id == 0) {
> +            LOG_INFO("display %d: check sw", get_id());
> +        }
> +    } catch (...) {
> +        spice_warn_if_reached();
> +    }
>   }
>   
>   void DisplayChannel::handle_mode(RedPeer::InMessage* message)
> @@ -1620,29 +1637,52 @@ void DisplayChannel::handle_surface_destroy(RedPeer::InMessage* message)
>       }
>   }
>   
> -#define PRE_DRAW
> -#define POST_DRAW
> +#define DRAW(type, can_diff) {                                          \
> +    canvas->draw_##type(*type, message->size());                        \
> +    if (type->base.surface_id == 0) {                                   \
> +        invalidate(type->base.box, false);                              \
> +    }                                                                   \
> +    if ((can_diff) && _check_diff) {                                    \
> +        Canvas *swcanvas = _swsurfaces_cache[type->base.surface_id];    \
> +        swcanvas->draw_##type(*type, message->size());                  \
> +        check_diff(swcanvas, canvas, type->base.box);                   \
> +    }                                                                   \
> +}
> +
> +#include "red_pixmap_sw.h"
> +
> +void check_diff(Canvas *a, Canvas *b, const SpiceRect& rect)
> +{
> +    QRegion region;
> +    RedPixmapSw *ap = new RedPixmapSw(rect.right, rect.bottom, RedDrawable::RGB32, true, 0);
> +    RedPixmapSw *bp = new RedPixmapSw(rect.right, rect.bottom, RedDrawable::RGB32, true, 0);
> +
> +    region_init(&region);
> +    region_add(&region, &rect);
> +    a->copy_pixels(region, *ap);
> +    b->copy_pixels(region, *bp);
> +    ap->equal(*bp, rect);
>   
> -#define DRAW(type) {                                \
> -    PRE_DRAW;                                       \
> -    canvas->draw_##type(*type, message->size());    \
> -    POST_DRAW;                                      \
> -    if (type->base.surface_id == 0) {               \
> -        invalidate(type->base.box, false);          \
> -    }                                               \
> +    delete ap;
> +    delete bp;
>   }
>   
>   void DisplayChannel::handle_copy_bits(RedPeer::InMessage* message)
>   {
>       Canvas *canvas;
>       SpiceMsgDisplayCopyBits* copy_bits = (SpiceMsgDisplayCopyBits*)message->data();
> -    PRE_DRAW;
>       canvas = _surfaces_cache[copy_bits->base.surface_id];
> +    Canvas *swcanvas = _swsurfaces_cache[copy_bits->base.surface_id];
> +
>       canvas->copy_bits(*copy_bits, message->size());
> -    POST_DRAW;
>       if (copy_bits->base.surface_id == 0) {
>           invalidate(copy_bits->base.box, false);
>       }
> +
> +    if (_check_diff) {
> +        swcanvas->copy_bits(*copy_bits, message->size());
> +        check_diff(swcanvas, canvas, copy_bits->base.box);
> +    }
>   }
>   
>   void DisplayChannel::handle_draw_fill(RedPeer::InMessage* message)
> @@ -1650,7 +1690,7 @@ void DisplayChannel::handle_draw_fill(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawFill* fill = (SpiceMsgDisplayDrawFill*)message->data();
>       canvas = _surfaces_cache[fill->base.surface_id];
> -    DRAW(fill);
> +    DRAW(fill, FALSE);
>   }
>   
>   void DisplayChannel::handle_draw_opaque(RedPeer::InMessage* message)
> @@ -1658,7 +1698,7 @@ void DisplayChannel::handle_draw_opaque(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawOpaque* opaque = (SpiceMsgDisplayDrawOpaque*)message->data();
>       canvas = _surfaces_cache[opaque->base.surface_id];
> -    DRAW(opaque);
> +    DRAW(opaque, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_copy(RedPeer::InMessage* message)
> @@ -1666,7 +1706,7 @@ void DisplayChannel::handle_draw_copy(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawCopy* copy = (SpiceMsgDisplayDrawCopy*)message->data();
>       canvas = _surfaces_cache[copy->base.surface_id];
> -    DRAW(copy);
> +    DRAW(copy, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_blend(RedPeer::InMessage* message)
> @@ -1674,7 +1714,7 @@ void DisplayChannel::handle_draw_blend(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawBlend* blend = (SpiceMsgDisplayDrawBlend*)message->data();
>       canvas = _surfaces_cache[blend->base.surface_id];
> -    DRAW(blend);
> +    DRAW(blend, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_blackness(RedPeer::InMessage* message)
> @@ -1682,7 +1722,7 @@ void DisplayChannel::handle_draw_blackness(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawBlackness* blackness = (SpiceMsgDisplayDrawBlackness*)message->data();
>       canvas = _surfaces_cache[blackness->base.surface_id];
> -    DRAW(blackness);
> +    DRAW(blackness, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_whiteness(RedPeer::InMessage* message)
> @@ -1690,7 +1730,7 @@ void DisplayChannel::handle_draw_whiteness(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawWhiteness* whiteness = (SpiceMsgDisplayDrawWhiteness*)message->data();
>       canvas = _surfaces_cache[whiteness->base.surface_id];
> -    DRAW(whiteness);
> +    DRAW(whiteness, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_invers(RedPeer::InMessage* message)
> @@ -1698,7 +1738,7 @@ void DisplayChannel::handle_draw_invers(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawInvers* invers = (SpiceMsgDisplayDrawInvers*)message->data();
>       canvas = _surfaces_cache[invers->base.surface_id];
> -    DRAW(invers);
> +    DRAW(invers, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_rop3(RedPeer::InMessage* message)
> @@ -1706,7 +1746,7 @@ void DisplayChannel::handle_draw_rop3(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawRop3* rop3 = (SpiceMsgDisplayDrawRop3*)message->data();
>       canvas = _surfaces_cache[rop3->base.surface_id];
> -    DRAW(rop3);
> +    DRAW(rop3, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_stroke(RedPeer::InMessage* message)
> @@ -1714,7 +1754,7 @@ void DisplayChannel::handle_draw_stroke(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawStroke* stroke = (SpiceMsgDisplayDrawStroke*)message->data();
>       canvas = _surfaces_cache[stroke->base.surface_id];
> -    DRAW(stroke);
> +    DRAW(stroke, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_text(RedPeer::InMessage* message)
> @@ -1722,7 +1762,7 @@ void DisplayChannel::handle_draw_text(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawText* text = (SpiceMsgDisplayDrawText*)message->data();
>       canvas = _surfaces_cache[text->base.surface_id];
> -    DRAW(text);
> +    DRAW(text, FALSE);
>   }
>   
>   void DisplayChannel::handle_draw_transparent(RedPeer::InMessage* message)
> @@ -1730,7 +1770,7 @@ void DisplayChannel::handle_draw_transparent(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawTransparent* transparent = (SpiceMsgDisplayDrawTransparent*)message->data();
>       canvas = _surfaces_cache[transparent->base.surface_id];
> -    DRAW(transparent);
> +    DRAW(transparent, TRUE);
>   }
>   
>   void DisplayChannel::handle_draw_alpha_blend(RedPeer::InMessage* message)
> @@ -1738,7 +1778,7 @@ void DisplayChannel::handle_draw_alpha_blend(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawAlphaBlend* alpha_blend = (SpiceMsgDisplayDrawAlphaBlend*)message->data();
>       canvas = _surfaces_cache[alpha_blend->base.surface_id];
> -    DRAW(alpha_blend);
> +    DRAW(alpha_blend, FALSE);
>   }
>   
>   void DisplayChannel::handle_draw_composite(RedPeer::InMessage* message)
> @@ -1746,7 +1786,7 @@ void DisplayChannel::handle_draw_composite(RedPeer::InMessage* message)
>       Canvas *canvas;
>       SpiceMsgDisplayDrawComposite* composite = (SpiceMsgDisplayDrawComposite*)message->data();
>       canvas = _surfaces_cache[composite->base.surface_id];
> -    DRAW(composite);
> +    DRAW(composite, TRUE);
>   }
>   
>   void DisplayChannel::streams_time()
> diff --git a/client/display_channel.h b/client/display_channel.h
> index 197ae73..8a30289 100644
> --- a/client/display_channel.h
> +++ b/client/display_channel.h
> @@ -187,6 +187,7 @@ private:
>   
>   private:
>       SurfacesCache _surfaces_cache;
> +    SurfacesCache _swsurfaces_cache;
>       PixmapCache& _pixmap_cache;
>       PaletteCache _palette_cache;
>       GlzDecoderWindow& _glz_window;
> @@ -226,6 +227,7 @@ private:
>       InterruptUpdate _interrupt_update;
>   
>       bool _mig_wait_primary;
> +    bool _check_diff;
>       friend class SetModeEvent;
>       friend class CreatePrimarySurfaceEvent;
>       friend class DestroyPrimarySurfaceEvent;
> diff --git a/client/red_pixmap.h b/client/red_pixmap.h
> index 240ed76..3f12855 100644
> --- a/client/red_pixmap.h
> +++ b/client/red_pixmap.h
> @@ -36,6 +36,25 @@ public:
>       bool is_big_endian_bits();
>       virtual RedDrawable::Format get_format() { return _format; }
>   
> +    bool equal(const RedPixmap &other, const SpiceRect &rect) const {
> +        spice_debug("l:%d r:%d t:%d b:%d", rect.left, rect.right, rect.top, rect.bottom);
> +        for (int x = rect.left; x < rect.right; ++x)
> +            for (int y = rect.top; y < rect.bottom; ++y) {
> +                for (int i = 0; i < 3; i++) { // ignore alpha
> +                    int p = x * 4 + y * _stride + i;
> +                    if (other._data[p] != _data[p]) {
> +                        spice_printerr("equal fails at (+%d+%d) +%d+%d:%d in %dx%d",
> +                                       rect.left, rect.top, x-rect.left, y-rect.top, i,
> +                                       _width-rect.left, _height-rect.top);
> +                        if (getenv("DIFFBP"))
> +                            SPICE_BREAKPOINT();
> +                        return false;
> +                    }
> +                }
> +            }
> +        return true;
> +    }
> +
>   protected:
>       Format _format;
>       int _width;
> diff --git a/client/utils.h b/client/utils.h
> index 4657279..d8d6962 100644
> --- a/client/utils.h
> +++ b/client/utils.h
> @@ -50,6 +50,9 @@ private:
>       throw Exception(exption_string, err);                       \
>   }
>   
> +#define SPICE_BREAKPOINT() do{                  \
> +    __asm__ __volatile__ ("int $03");           \
> +}while(0)
>   
>   template <class T>
>   class AutoRef {



More information about the Spice-devel mailing list