On Fri, 2017-02-10 at 06:56 -0500, Frediano Ziglio wrote:
> >
> >
[snip]
> > +/* Add a @drawable (with a shadow) to the current ring.
> > + * The return value indicates whether the new item should be added
> > to the
> > pipe */
>
> it's @drawable or @item ?
I guess it should be @item.
>
> > static int current_add_with_shadow(DisplayChannel *display, Ring
> > *ring,
> > Drawable *item)
> > {
[snip]
>
> > diff --git a/server/tree.h b/server/tree.h
> > index 35b78ab..e579dd0 100644
> > --- a/server/tree.h
> > +++ b/server/tree.h
> > @@ -43,12 +43,18 @@ struct TreeItem {
> > RingItem siblings_link;
> > uint32_t type;
> > Container *container;
> > + /* rgn holds the region of the item. As additional items get
> > added to
> > the
> > + * tree, this region may be modified to represent the portion
> > of the
> > item
> > + * that is not obscured by other items */
>
> I'm a bit confused by the "to represent the portion...". Does it mean
> that
> the portion is removed, right? Maybe "this region may be modified to
> exclude
> the portion of the item ..." ?
I think my original comment is accurate but perhaps a little unclear.
If I changed to your wording, I would also have to change "not
obscured" to "obscured". For example:
"this region may be modified to exclude the portion of the item that is
obscured..."
vs.
"this region may be modified to represent the portion of the item that
is *not* obscured"
They are basically equivalent I think. I can use your wording though
if you think it's clearer.
>
> > QRegion rgn;
> > };
> >
> > /* A region "below" a copy, or the src region of the copy */
> > struct Shadow {
> > TreeItem base;
> > + /* holds the union of all parts of this source region that
> > have been
> > + * obscured by a drawable item that has been subsequently
> > added to the
> > tree
> > + */
>
> Not entirely sure of this.
Yes, it's complicated, but I think the description is accurate.
>
> There are 2 region of code that modify on_hold:
> - excluding a region from an item with a shadow
> region_and(&and_rgn, &shadow->on_hold);
> if (!region_is_empty(&and_rgn)) {
> region_exclude(&shadow->on_hold, &and_rgn);
> this remove part of the region
This part of the code is removing that part of the on_hold region
because we just removed that same region from the Shadow itself. In
other words, the on_hold region must always be a subset of the
Shadow's region. It doesn't make sense to have a region stored in
Shadow's on_hold unless it's actually a part of the Shadow's region.
> - excluding a region from a shadow
> region_or(&shadow->on_hold, &and_rgn);
> this add a region.
This is the part of the region that has been obscured by a new item, so
we add it to on_hold.
>
> Both in __exclude_region, a function we are not sure what's doing
> and for which purpose.
>
> > QRegion on_hold;
> > };
> >
>
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel