[Spice-devel] [PATCH] RFC: Document display channel drawable trees, etc
Jonathon Jongsma
jjongsma at redhat.com
Fri Jan 20 20:34:16 UTC 2017
---
I started looking into a bug related to surfaces and drawing, but after a
little initial investigation, I realized that I didn't really understand any of
the code in this part of spice server. I discussed it a little bit with several
others who also didn't have a good understanding of this part of the code. So I
decided it needed to be documented so that we can understand it and work on it
effectively. I've spent several long days trying to understand the details and
trying to document them as well as I can. There is a lot of confusing
terminology, and there are things I still don't fully understand, but I wanted
to send this out to get feedback from others. Is it helpful? Do you see any
cases where I misinterpreted things? Can anybody solve any pieces of the puzzle
that I didn't quite figure out? etc. etc.
I know it's rather dense and will probably require others to spend some serious
time understanding the code, but I'd love to get some feedback on it.
server/display-channel-private.h | 5 +-
server/display-channel.c | 256 +++++++++++++++++++++++++++++++++++++++
server/display-channel.h | 10 +-
server/tree.c | 12 +-
server/tree.h | 6 +
5 files changed, 285 insertions(+), 4 deletions(-)
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 6e299b9..dc4d605 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -32,7 +32,10 @@ struct DisplayChannelPrivate
int enable_jpeg;
int enable_zlib_glz_wrap;
- Ring current_list; /* Drawable */
+ /* A ring of pending drawables for this DisplayChannel, regardless of which
+ * surface they're associated with. This list is mainly used to flush older
+ * drawables when we need to make room for new drawables. */
+ Ring current_list;
uint32_t current_size;
uint32_t drawable_count;
diff --git a/server/display-channel.c b/server/display-channel.c
index 6069883..83e76c1 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -393,6 +393,8 @@ static void current_add_drawable(DisplayChannel *display,
drawable->refs++;
}
+/* Unrefs the drawable and removes it from any rings that it's in, as well as
+ * removing any associated shadow item */
static void current_remove_drawable(DisplayChannel *display, Drawable *item)
{
/* todo: move all to unref? */
@@ -422,6 +424,7 @@ static void drawable_remove_from_pipes(Drawable *drawable)
}
}
+/* This function should never be called for Shadow TreeItems */
static void current_remove(DisplayChannel *display, TreeItem *item)
{
TreeItem *now = item;
@@ -439,22 +442,38 @@ static void current_remove(DisplayChannel *display, TreeItem *item)
} else {
Container *now_as_container = CONTAINER(now);
+ /* This is a questionable assert. It relies several assumptions
+ * to ensure that this function is never called for a
+ * TREE_ITEM_TYPE_SHADOW item. */
spice_assert(now->type == TREE_ITEM_TYPE_CONTAINER);
if ((ring_item = ring_get_head(&now_as_container->items))) {
+ /* descend into the container's child ring and continue
+ * iterating and removing those children */
now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
continue;
}
+ /* This item is a container but it has no children, so reset our
+ * iterator to the item's previous sibling and free this empty
+ * container */
ring_item = now->siblings_link.prev;
container_free(now_as_container);
}
if (now == item) {
+ /* This is true if the initial @item was a DRAWABLE, or if @item
+ * was a container and we've finished iterating over all of that
+ * container's children and returned back up to the parent and
+ * freed it (see below) */
return;
}
+ /* Increment the iterator to the next sibling. Note that if an item was
+ * removed above, ring_item will have been reset to the item before the
+ * item that was removed */
if ((ring_item = ring_next(&container_of_now->items, ring_item))) {
now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
} else {
+ /* there is no next sibling, so move one level up the tree */
now = &container_of_now->base;
}
}
@@ -467,10 +486,23 @@ static void current_remove_all(DisplayChannel *display, int surface_id)
while ((ring_item = ring_get_head(ring))) {
TreeItem *now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
+ /* NOTE: current_remove() should never be called on Shadow type items
+ * or we will hit an assert. Fortunately, the 'current' ring is ordered
+ * in such a way that a DrawItem will always be placed before its
+ * associated Shadow in the tree. Since removing a DrawItem will also
+ * result in the associated Shadow item being removed from the tree,
+ * this loop will never call current_remove() on a Shadow item unless
+ * we change the order that items are inserted into the tree */
current_remove(display, now);
}
}
+/* Replace an existing Drawable in the tree with a new drawable that is
+ * equivalent. The new drawable is also added to the pipe.
+ *
+ * This function can fail if the items aren't actually equivalent (e.g. either
+ * item has a shadow, they have different effects, etc)
+ */
static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *other)
{
DrawItem *other_draw_item;
@@ -490,6 +522,9 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
other_drawable = SPICE_CONTAINEROF(other_draw_item, Drawable, tree_item);
if (item->effect == QXL_EFFECT_OPAQUE) {
+ /* check whether the new item can safely replace the other drawable at
+ * the same position in the pipe, or whether it should be added to the
+ * end of the queue */
int add_after = !!other_drawable->stream &&
is_drawable_independent_from_surfaces(drawable);
stream_maintenance(display, drawable, other_drawable);
@@ -553,6 +588,47 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
return FALSE;
}
+/* This function excludes the region from a single TreeItem
+ *
+ * If there is overlap between @rgn and the @item region, remove the
+ *
+ * overlapping intersection from both @rgn and the item's region (XXX WHY???)
+ *
+ * However, if the item is a DrawItem that has a shadow, we add an additional
+ * region to @rgn: the intersection of the shadow item's region with @rgn if
+ * @rgn was shifted over by the delta between the DrawItem and the Shadow.
+ * [WORKING THEORY: since the destination region for a COPY_BITS operation was
+ * excluded, we no longer need the source region that corresponds with that
+ * copy operation, so we can also exclude any drawables that affect that
+ * region. Not sure if that actually makes sense... ]
+ *
+ * If the item is a Shadow, we store the intersection between @rgn and the
+ * Shadow's region in Shadow::on_hold and remove that region from @rgn. This is
+ * done since a Shadow represents the source region for a COPY_BITS operation,
+ * and we need to make sure that this source region stays up-to-date until the
+ * copy operation is executed.
+ *
+ * Consider the following case:
+ * 1) the surface is fully black at the beginning
+ * 2) we add a new item to the tree which paints region A white
+ * 3) we add a new item to the tree which copies region A to region B
+ * 4) we add another new item to teh tree painting region A blue.
+ *
+ * After all operations are completed, region A should be blue, and region B
+ * should be white. If there were no copy operation (step 3), we could simply
+ * eliminate step 2 when we add item 4 to the tree, since step 4 overwrites the
+ * same region with a different color. However, if we did eliminate step 2,
+ * region B would be black after all operations were completed. So these
+ * regions that would normally be excluded are put "on hold" if they are part
+ * of a source region for a copy operation.
+ *
+ * @display: the display channel
+ * @ring: a fallback toplevel ring???
+ * @item: the tree item to exclude from @rgn
+ * @rgn: the region to exclude (?)
+ * @top_ring: ?
+ * @frame_candidate: ?
+ */
static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem *item, QRegion *rgn,
Ring **top_ring, Drawable *frame_candidate)
{
@@ -560,51 +636,79 @@ static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem *item
stat_start(&display->priv->__exclude_stat, start_time);
region_clone(&and_rgn, rgn);
+ /* find intersection of the @rgn argument with the region of the @item arg */
region_and(&and_rgn, &item->rgn);
if (!region_is_empty(&and_rgn)) {
if (IS_DRAW_ITEM(item)) {
DrawItem *draw = DRAW_ITEM(item);
if (draw->effect == QXL_EFFECT_OPAQUE) {
+ /* remove the intersection from the original @rgn */
region_exclude(rgn, &and_rgn);
}
if (draw->shadow) {
+ /* this item represents the destination of a COPY_BITS
+ * operation. 'shadow' represents the source item for the copy
+ * operation */
Shadow *shadow;
int32_t x = item->rgn.extents.x1;
int32_t y = item->rgn.extents.y1;
+ /* remove the intersection from the item's region */
region_exclude(&draw->base.rgn, &and_rgn);
shadow = draw->shadow;
+ /* shift the intersected region by the difference between the
+ * source and destination regions */
region_offset(&and_rgn, shadow->base.rgn.extents.x1 - x,
shadow->base.rgn.extents.y1 - y);
+ /* remove the shifted intersection region from the source
+ * (shadow) item's region. If the destination is excluded, we
+ * can also exclude the corresponding area from the source */
region_exclude(&shadow->base.rgn, &and_rgn);
+ /* find the intersection between the shifted intersection
+ * region and the Shadow's 'on_hold' region. This represents
+ * the portion of the Shadow's region that we just removed that
+ * is currently stored in on_hold. */
region_and(&and_rgn, &shadow->on_hold);
if (!region_is_empty(&and_rgn)) {
+ /* Since we removed a portion of the Shadow's region, we
+ * can also remove that portion from on_hold */
region_exclude(&shadow->on_hold, &and_rgn);
+ /* Since this region is no longer "on hold", add it back to
+ * the @rgn argument */
region_or(rgn, &and_rgn);
// in flat representation of current, shadow is always his owner next
+ // XXX: what does that mean?
if (!tree_item_contained_by(&shadow->base, *top_ring)) {
*top_ring = tree_item_container_items(&shadow->base, ring);
}
}
} else {
+ /* XXX: ??? */
if (frame_candidate) {
Drawable *drawable = SPICE_CONTAINEROF(draw, Drawable, tree_item);
stream_maintenance(display, frame_candidate, drawable);
}
+ /* Remove the intersection from the DrawItem's region */
region_exclude(&draw->base.rgn, &and_rgn);
}
} else if (item->type == TREE_ITEM_TYPE_CONTAINER) {
+ /* excludes the intersection between 'rgn' and item->rgn from the
+ * item's region */
region_exclude(&item->rgn, &and_rgn);
if (region_is_empty(&item->rgn)) { //assume container removal will follow
Shadow *shadow;
+ /* exclude the intersection from the 'rgn' argument as well,
+ * but only if the item is now empty... WHY??? */
region_exclude(rgn, &and_rgn);
if ((shadow = tree_item_find_shadow(item))) {
+ /* add the shadow's on_hold region back to the 'rgn' argument */
region_or(rgn, &shadow->on_hold);
if (!tree_item_contained_by(&shadow->base, *top_ring)) {
+ /* what is top_ring used for??? */
*top_ring = tree_item_container_items(&shadow->base, ring);
}
}
@@ -614,14 +718,43 @@ static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem *item
spice_assert(item->type == TREE_ITEM_TYPE_SHADOW);
shadow = SHADOW(item);
+ /* Since a Shadow represents the source region for a COPY_BITS
+ * operation, we need to make sure that we don't remove existing
+ * drawables that draw to this source region. If we did, it would
+ * affect the copy operation. So we remove the intersection between
+ * @rgn and item->rgn from the @rgn argument to avoid excluding
+ * these drawables */
region_exclude(rgn, &and_rgn);
+ /* adds this intersection to on_hold */
region_or(&shadow->on_hold, &and_rgn);
}
}
+ /* clean up memory */
region_destroy(&and_rgn);
stat_add(&display->priv->__exclude_stat, start_time);
}
+/* This function iterates through the given @ring starting at @ring_item and
+ * continuing until reaching @last, and calls __exclude_item() on each item.
+ * Any items that have an empty region as a result of the __exclude_region()
+ * call are removed from the tree.
+ *
+ * XXX: I still don't have a great conceptual understanding of what we're
+ * trying to do by calling this function.
+ *
+ * @ring: every time this function is called, @ring is a Surface's 'current'
+ * ring, or to the ring of children of a container within that ring.
+ * @ring_item: callers usually call this argument 'exclude_base'. We will
+ * iterate through the tree starting at this item
+ * @rgn: callers usually call this 'exclude_rgn' -- it appears to be the region
+ * we want to exclude from existing items in the tree. It is an in/out
+ * parameter and it may be modified as the result of calling this function
+ * @last: We will stop iterating at this item, and the function will return the
+ * next item after iteration is complete (which may be different than the
+ * passed value if that item was removed from the tree
+ * @frame_candidate: usually callers pass NULL, sometimes it's the drawable
+ * that's being added to the 'current' ring. What is its purpose?
+ */
static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_item,
QRegion *rgn, TreeItem **last, Drawable *frame_candidate)
{
@@ -640,40 +773,60 @@ static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_i
spice_assert(!region_is_empty(&now->rgn));
+ /* check whether the ring_item item intersects the passed-in region */
if (region_intersects(rgn, &now->rgn)) {
+ /* remove the overlapping portions of region and now->rgn, among
+ * other things. See documentation for __exclude_region() */
__exclude_region(display, ring, now, rgn, &top_ring, frame_candidate);
if (region_is_empty(&now->rgn)) {
+ /* __exclude_region() does not remove the region of shadow-type
+ * items */
spice_assert(now->type != TREE_ITEM_TYPE_SHADOW);
ring_item = now->siblings_link.prev;
+ /* if __exclude_region() removed the entire region for this
+ * sibling item, remove it from the 'current' tree */
current_remove(display, now);
if (last && *last == now) {
+ /* the caller wanted to stop at this item, but this item
+ * has been removed, so we set @last to the next item */
SPICE_VERIFY(SPICE_OFFSETOF(TreeItem, siblings_link) == 0);
*last = (TreeItem *)ring_next(ring, ring_item);
}
} else if (now->type == TREE_ITEM_TYPE_CONTAINER) {
+ /* if this sibling is a container type, descend into the
+ * container's child ring and continue iterating */
Container *container = CONTAINER(now);
if ((ring_item = ring_get_head(&container->items))) {
ring = &container->items;
spice_assert(SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link)->container);
continue;
}
+ /* container had no children, so reset ring_item to the
+ * container itself */
ring_item = &now->siblings_link;
}
if (region_is_empty(rgn)) {
+ /* __exclude_region() removed the entire region from 'rgn', so
+ * no need to continue checking further items in the tree */
stat_add(&display->priv->exclude_stat, start_time);
return;
}
}
SPICE_VERIFY(SPICE_OFFSETOF(TreeItem, siblings_link) == 0);
+ /* if this is the last item to check, or if the current ring is
+ * completed, don't go any further */
while ((last && *last == (TreeItem *)ring_item) ||
!(ring_item = ring_next(ring, ring_item))) {
+ /* we're currently iterating the top ring, so we're done */
if (ring == top_ring) {
stat_add(&display->priv->exclude_stat, start_time);
return;
}
+ /* we're iterating through a container child ring, so climb one
+ * level up the heirarchy and continue iterating that ring */
ring_item = &container->base.siblings_link;
container = container->base.container;
ring = (container) ? &container->items : top_ring;
@@ -681,6 +834,8 @@ static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_i
}
}
+/* Add a @drawable (with a shadow) to the current ring.
+ * The return value indicates whether the new item should be added to the pipe */
static int current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawable *item)
{
stat_start(&display->priv->add_stat, start_time);
@@ -707,11 +862,19 @@ static int current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawable
stream_detach_behind(display, &shadow->base.rgn, NULL);
}
+ /* Prepend the shadow to the beginning of the current ring */
ring_add(ring, &shadow->base.siblings_link);
+ /* Prepend the draw item to the beginning of the current ring. NOTE: this
+ * means that the drawable is placed *before* its associated shadow in the
+ * tree. Changing this order will violate several unstated assumptions */
current_add_drawable(display, item, ring);
if (item->tree_item.effect == QXL_EFFECT_OPAQUE) {
QRegion exclude_rgn;
region_clone(&exclude_rgn, &item->tree_item.base.rgn);
+ /* Since the new drawable is opaque, remove overlapped regions from all
+ * items already in the tree. Start iterating through the tree
+ * starting with the shadow item to avoid excluding the new item
+ * itself */
exclude_region(display, ring, &shadow->base.siblings_link, &exclude_rgn, NULL, NULL);
region_destroy(&exclude_rgn);
streams_update_visible_region(display, item);
@@ -724,6 +887,8 @@ static int current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawable
return TRUE;
}
+/* Add a @drawable (without a shadow) to the current ring.
+ * The return value indicates whether the new item should be added to the pipe */
static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
{
DrawItem *item = &drawable->tree_item;
@@ -736,54 +901,116 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
region_init(&exclude_rgn);
now = ring_next(ring, ring);
+ /* check whether the new drawable region intersects any of the items
+ * already in the 'current' ring */
while (now) {
TreeItem *sibling = SPICE_CONTAINEROF(now, TreeItem, siblings_link);
int test_res;
if (!region_bounds_intersects(&item->base.rgn, &sibling->rgn)) {
+ /* the bounds of the two items are totally disjoint, so no need to
+ * check further. check the next item */
now = ring_next(ring, now);
continue;
}
+ /* bounds overlap, but check whether the regions actually overlap */
test_res = region_test(&item->base.rgn, &sibling->rgn, REGION_TEST_ALL);
if (!(test_res & REGION_TEST_SHARED)) {
+ /* there's no overlap of the regions between these two items. Move
+ * on to the next one. */
now = ring_next(ring, now);
continue;
} else if (sibling->type != TREE_ITEM_TYPE_SHADOW) {
+ /* there is an overlap between the two regions */
+ /* NOTE: Shadow types represent a source region for a COPY_BITS
+ * operation, they don't represent a region that will be drawn.
+ * Therefore, we don't check for overlap between the new
+ * DrawItem and any shadow items */
if (!(test_res & REGION_TEST_RIGHT_EXCLUSIVE) &&
!(test_res & REGION_TEST_LEFT_EXCLUSIVE) &&
current_add_equal(display, item, sibling)) {
+ /* the regions were equivalent, so we just replaced the other
+ * drawable with the new one */
stat_add(&display->priv->add_stat, start_time);
+ /* Caller doesn't need to add the new drawable to the pipe,
+ * since current_add_equal already added it to the pipe */
return FALSE;
}
if (!(test_res & REGION_TEST_RIGHT_EXCLUSIVE) && item->effect == QXL_EFFECT_OPAQUE) {
+ /* the new drawable is opaque and entirely contains the sibling item */
Shadow *shadow;
int skip = now == exclude_base;
if ((shadow = tree_item_find_shadow(sibling))) {
+ /* The sibling item was the destination of a COPY_BITS operation */
if (exclude_base) {
+ /* During a previous iteration through this loop, an
+ * obscured sibling item was removed from the tree, and
+ * exclude_base was set to the item immediately after
+ * the removed item (see below). This time through the
+ * loop, we encountered another sibling that was
+ * completely obscured, so we call exclude_region()
+ * using the previously saved item as our starting
+ * point. @exlude_rgn will be the union of any previous
+ * 'on_hold' regions from the shadows of previous
+ * iterations
+ *
+ * XXX: why do we only only call exclude_region() for
+ * the previous item if the next item is obscured and
+ * has a shadow???
+ */
TreeItem *next = sibling;
+ /* XXX: What is the intent of this call? */
exclude_region(display, ring, exclude_base, &exclude_rgn, &next, NULL);
if (next != sibling) {
+ /* the @next param is only changed if the given item
+ * was removed as a side-effect of calling
+ * exclude_region(), so update our loop variable */
now = next ? &next->siblings_link : NULL;
exclude_base = NULL;
continue;
}
}
+ /* Since the destination item (sibling) of the COPY_BITS
+ * operation is fully obscured, we no longer need the
+ * source item (shadow) anymore. shadow->on_hold represents
+ * a region that would normally have been excluded by a
+ * previous call to __exclude_region() (see documentation
+ * for that function), but was put on hold to make sure we
+ * kept the source region up to date. Now that we no longer
+ * need this source region, this "on hold" region can be
+ * safely excluded again. */
region_or(&exclude_rgn, &shadow->on_hold);
}
now = now->prev;
+ /* remove the obscured sibling from the 'current' tree, which
+ * will also remove its shadow (if any) */
current_remove(display, sibling);
+ /* advance the loop variable */
now = ring_next(ring, now);
if (shadow || skip) {
+ /* XXX: don't understand this exclude_base stuff
+ * 'now' is currently set to the the item immediately AFTER
+ * the obscured sibling that we just removed. Why is this
+ * item used as an 'exclude_base'??
+ */
exclude_base = now;
}
continue;
}
if (!(test_res & REGION_TEST_LEFT_EXCLUSIVE) && is_opaque_item(sibling)) {
+ /* the sibling item is opaque and entirely contains the new drawable */
Container *container;
+ /* XXX: still don't understand this exclude_base stuff. The
+ * first time through, @exclude_base will be NULL, but
+ * subsequent loops may set it to something.
+ * In addition, @exclude_rgn starts out empty, but previous
+ * iterations of this loop may have added various
+ * Shadow::on_hold regions to it.
+ */
if (exclude_base) {
exclude_region(display, ring, exclude_base, &exclude_rgn, NULL, NULL);
region_clear(&exclude_rgn);
@@ -791,13 +1018,20 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
}
if (sibling->type == TREE_ITEM_TYPE_CONTAINER) {
container = CONTAINER(sibling);
+ /* NOTE: here, ring is reset to the ring of the container's children */
ring = &container->items;
+ /* if the sibling item is a container, place the new
+ * drawable into that container */
item->base.container = container;
+ /* Start iterating over the container's children to see if
+ * any of them intersect this new drawable */
now = ring_next(ring, ring);
continue;
}
spice_assert(IS_DRAW_ITEM(sibling));
if (!DRAW_ITEM(sibling)->container_root) {
+ /* Create a new container to hold the sibling and the new
+ * drawable */
container = container_new(DRAW_ITEM(sibling));
if (!container) {
spice_warning("create new container failed");
@@ -805,17 +1039,32 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
return FALSE;
}
item->base.container = container;
+ /* reset 'ring' to the container's children ring, so that
+ * we can add the new drawable to this ring below */
ring = &container->items;
}
}
}
+ /* If we've gotten here, that means that:
+ * - the new item is not opaque
+ * - We just created a container to hold the new drawable and the
+ * sibling that encloses it
+ * - ??? */
if (!exclude_base) {
exclude_base = now;
}
break;
}
+ /* we've removed any obscured siblings and figured out which ring the new
+ * drawable needs to be added to, so let's add it. */
if (item->effect == QXL_EFFECT_OPAQUE) {
+ /* @exclude_rgn may contain the union of on_hold regions from any
+ * Shadows that were associated with DrawItems that were removed from
+ * the tree. Add the new item's region to that */
region_or(&exclude_rgn, &item->base.rgn);
+ /* XXX: Why do we need to call exclude_region() here again? I thought
+ * we had just iterated the whole list above to check for region
+ * intersections between this new item and existing items... */
exclude_region(display, ring, exclude_base, &exclude_rgn, NULL, drawable);
stream_trace_update(display, drawable);
streams_update_visible_region(display, drawable);
@@ -1623,6 +1872,8 @@ static void surface_update_dest(RedSurface *surface, const SpiceRect *area)
canvas->ops->read_bits(canvas, dest, -stride, area);
}
+/* Draws all drawables associated with @surface, starting from the tail of the
+ * ring, and stopping after it draws @last */
static void draw_until(DisplayChannel *display, RedSurface *surface, Drawable *last)
{
RingItem *ring_item;
@@ -1646,6 +1897,11 @@ static void draw_until(DisplayChannel *display, RedSurface *surface, Drawable *l
} while (now != last);
}
+/* Find the first Drawable in the @current ring that intersects the given
+ * @area, starting at item @from (or the head of the ring if @from is NULL).
+ *
+ * NOTE: this function expects @current to be a ring of Drawables, and more
+ * specifically an instance of Surface::current_list (not Surface::current) */
static Drawable* current_find_intersects_rect(Ring *current, RingItem *from,
const SpiceRect *area)
{
diff --git a/server/display-channel.h b/server/display-channel.h
index ba83e8c..d7184a7 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -162,8 +162,14 @@ typedef struct DrawContext {
typedef struct RedSurface {
uint32_t refs;
- Ring current; /* TreeItem */
- Ring current_list; /* Drawable */
+ /* A Ring representing a hierarchical tree structure. This tree includes
+ * DrawItems, Containers, and Shadows. It is used to efficiently determine
+ * which drawables overlap, and to exclude regions of drawables that are
+ * obscured by other drawables */
+ Ring current;
+ /* A ring of pending Drawables associated with this surface. This ring is
+ * actually used for drawing */
+ Ring current_list;
DrawContext context;
Ring depend_on_me;
diff --git a/server/tree.c b/server/tree.c
index ea11000..cc3a96f 100644
--- a/server/tree.c
+++ b/server/tree.c
@@ -185,6 +185,9 @@ void tree_item_dump(TreeItem *item)
tree_foreach(item, dump_item, &di);
}
+/* Creates a new Shadow item for the given DrawItem, with an offset of @delta.
+ * A shadow represents a source region for a COPY_BITS operation, while the
+ * DrawItem represents the destination region for the operation */
Shadow* shadow_new(DrawItem *item, const SpicePoint *delta)
{
spice_return_val_if_fail(item->shadow == NULL, NULL);
@@ -205,6 +208,11 @@ Shadow* shadow_new(DrawItem *item, const SpicePoint *delta)
return shadow;
}
+/* Create a new container to hold @item and insert @item into this container.
+ *
+ * NOTE: This function assumes that @item is already inside a different Ring,
+ * so it removes @item from that ring before inserting it into the new
+ * container */
Container* container_new(DrawItem *item)
{
Container *container = spice_new(Container, 1);
@@ -268,6 +276,8 @@ Shadow* tree_item_find_shadow(TreeItem *item)
return DRAW_ITEM(item)->shadow;
}
+/* return the Ring containing siblings of item, falling back to @ring if @item
+ * does not have a container */
Ring *tree_item_container_items(TreeItem *item, Ring *ring)
{
return (item->container) ? &item->container->items : ring;
@@ -281,7 +291,7 @@ int tree_item_contained_by(TreeItem *item, Ring *ring)
if (now == ring) {
return TRUE;
}
- } while ((item = &item->container->base));
+ } while ((item = &item->container->base)); /* move up one level */
return FALSE;
}
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 */
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
+ */
QRegion on_hold;
};
--
2.9.3
More information about the Spice-devel
mailing list