On Thu, 18 Feb 2010 21:58:51 +0100
Alexander Larsson <al...@redhat.com> wrote:

void red_pipe_item_init(PipeItem *item, int type)
> @@ -1904,13 +1904,13 @@ static inline void __exclude_region(RedWorker 
> *worker, TreeItem *item, QRegion *
>  
>              if (draw->shadow) {
>                  Shadow *shadow;
> -                int32_t x = item->rgn.bbox.left;
> -                int32_t y = item->rgn.bbox.top;
> +                int32_t x = item->rgn.extents.x1;
> +                int32_t y = item->rgn.extents.y1;
>  
>                  region_exclude(&draw->base.rgn, &and_rgn);
>                  shadow = draw->shadow;
> -                region_offset(&and_rgn, shadow->base.rgn.bbox.left - x,
> -                              shadow->base.rgn.bbox.top - y);
> +                region_offset(&and_rgn, shadow->base.rgn.extents.x1 - x,
> +                              shadow->base.rgn.extents.y1 - y);
>                  region_exclude(&shadow->base.rgn, &and_rgn);
>                  region_and(&and_rgn, &shadow->on_hold);
>                  if (!region_is_empty(&and_rgn)) {
> @@ -4304,8 +4304,7 @@ static void red_draw_drawable(RedWorker *worker, 
> Drawable *drawable)
>  #ifdef UPDATE_AREA_BY_TREE
>      //todo: add need top mask flag
>      worker->draw_funcs.set_top_mask(worker->surface.context.canvas,
> -                                    drawable->tree_item.base.rgn.num_rects,
> -                                    drawable->tree_item.base.rgn.rects);
> +                                    &drawable->tree_item.base.rgn);
>  #endif
>      red_draw_qxl_drawable(worker, drawable);
>  #ifdef UPDATE_AREA_BY_TREE
> @@ -6965,6 +6964,9 @@ static void red_display_send_upgrade(DisplayChannel 
> *display_channel, UpgradeIte
>      RedChannel *channel;
>      QXLDrawable *qxl_drawable;
>      SpiceMsgDisplayDrawCopy *copy = &display_channel->send_data.u.copy;
> +    SpiceRect *rects;
> +    int num_rects;
> +    uint32_t num_rects32;
>  
>      ASSERT(display_channel && item && item->drawable);
>      channel = &display_channel->base;
> @@ -6980,8 +6982,11 @@ static void red_display_send_upgrade(DisplayChannel 
> *display_channel, UpgradeIte
>      copy->base.box = qxl_drawable->bbox;
>      copy->base.clip.type = SPICE_CLIP_TYPE_RECTS;
>      copy->base.clip.data = channel->send_data.header.size;
> -    add_buf(channel, BUF_TYPE_RAW, &item->region.num_rects, 
> sizeof(uint32_t), 0, 0);
> -    add_buf(channel, BUF_TYPE_RAW, item->region.rects, sizeof(SpiceRect) * 
> item->region.num_rects, 0, 0);
> +    rects = region_dup_rects(&item->region, &num_rects);
> +    num_rects32 = num_rects;
> +    add_buf(channel, BUF_TYPE_RAW, &num_rects32, sizeof(uint32_t), 0, 0);
> +    add_buf(channel, BUF_TYPE_RAW, rects, sizeof(SpiceRect) * num_rects, 0, 
> 0);
> +    free(rects);


So this is wrong, add_buf() will add the address of what rects is pointing to 
the
sending vector, but then you free() it before the server send it.

The way to deal with this would be freeing it at 
red_display_release_stream_clip()
and allocating the memory at the callers of __new_stream_clip()


>      copy->data = qxl_drawable->u.copy;
>      fill_bits(display_channel, &copy->data.src_bitmap, item->drawable);
>  
> @@ -7034,6 +7039,9 @@ static void red_display_send_stream_clip(DisplayChannel 
> *display_channel,
>  
>      StreamAgent *agent = item->stream_agent;
>      Stream *stream = agent->stream;
> +    SpiceRect *rects;
> +    int num_rects;
> +    uint32_t num_rects32;
>  
>      ASSERT(stream);
>  
> @@ -7046,9 +7054,11 @@ static void 
> red_display_send_stream_clip(DisplayChannel *display_channel,
>      } else {
>          ASSERT(stream_clip->clip.type == SPICE_CLIP_TYPE_RECTS);
>          stream_clip->clip.data = channel->send_data.header.size;
> -        add_buf(channel, BUF_TYPE_RAW, &item->region.num_rects, 
> sizeof(uint32_t), 0, 0);
> -        add_buf(channel, BUF_TYPE_RAW, item->region.rects,
> -                item->region.num_rects * sizeof(SpiceRect), 0, 0);
> +     rects = region_dup_rects(&item->region, &num_rects);
> +     num_rects32 = num_rects;
> +        add_buf(channel, BUF_TYPE_RAW, &num_rects32, sizeof(uint32_t), 0, 0);
> +        add_buf(channel, BUF_TYPE_RAW, rects, num_rects * sizeof(SpiceRect), 
> 0, 0);
> +     free(rects);

The same problem.

>      }
>      display_begin_send_massage(display_channel, item);
>  }



So about this patch just one more question:
It look to me like there are some cases we translate rects from one type into 
another
where we could have use one type in the begining?,
(I did see many cases that it is a most, But some looked like we could avoid 
this?)

Thanks

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to