[Spice-devel] [PATCH 25/30] Replace custom region implementation with pixman_region32_t

[Yonit Halperin]

On 02/18/2010 10:58 PM, Alexander Larsson wrote:
> +int region_test(const QRegion *rgn, const QRegion *other_rgn, int query)
>   {
>   }
>


The new implementation is not efficient comparing to the previous 
implementation.
The previous one returns immediately once the requested query is 
satisfied (e.g., if it is
REGION_TEST_SHARED, it returns after it encounters the first two 
rectangles that intersect).
However, the suggested implementation calculates the whole intersection.
In addition, in case we query for REGION_TEST_RIGHT/LEFT_EXCLUSIVE it 
goes over the region again
and compares it to the intersection. The previous implementation makes 
only one pass over the region.

region_test is called many times. For example, it is called at least 
once when enter a drawable to the dependency tree (see 
red_red_current_add in red_worker). It is called when an opaque drawable 
hides other drawables (see exclude_region in red_worker). In the latter, 
the intersection itself is needed, so we can change it to calculate the 
intersection without pre-checking if it exists.
There are more calls to region_intersects (which calls region_test) that 
should be examined.

>
> @@ -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);
>       }
>       display_begin_send_massage(display_channel, item);
>   }
Tabs instead of spaces.

One more thing: as Izik mentioned, in many places you transform pixman 
bboxes to SpiceRect. Can't we work only with one type and perform the 
transformation only once more upstream?


Thanks,
Yonit.