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

Yonit Halperin yhalperi at redhat.com
Mon Feb 22 06:15:45 PST 2010


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.


More information about the Spice-devel mailing list