[Spice-devel] [PATCH 3/5] DisplayChannel: start documenting drawable tree
Jonathon Jongsma
jjongsma at redhat.com
Fri Feb 3 22:55:12 UTC 2017
The code that manages pending QXL Drawable operations is fairly complex
and difficult to understand. This is an attempt to start documenting
that code to save time when we have to work on this code in the future.
---
server/display-channel.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++
server/tree.c | 12 +++++++-
server/tree.h | 6 ++++
3 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/server/display-channel.c b/server/display-channel.c
index 7d3c6e4..e12751b 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -392,6 +392,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? */
@@ -420,6 +422,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;
@@ -440,19 +443,32 @@ static void current_remove(DisplayChannel *display, TreeItem *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;
}
}
@@ -465,10 +481,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;
@@ -488,6 +517,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);
@@ -679,6 +711,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);
@@ -705,7 +739,11 @@ 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;
@@ -722,6 +760,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;
@@ -734,31 +774,49 @@ 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) {
TreeItem *next = sibling;
exclude_region(display, ring, exclude_base, &exclude_rgn, &next, NULL);
@@ -771,7 +829,10 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
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) {
exclude_base = now;
@@ -780,6 +841,7 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
}
if (!(test_res & REGION_TEST_LEFT_EXCLUSIVE) && is_opaque_item(sibling)) {
+ /* the sibling item is opaque and entirely contains the new drawable */
Container *container;
if (exclude_base) {
@@ -789,13 +851,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");
@@ -803,6 +872,8 @@ 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;
}
}
@@ -812,6 +883,8 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
}
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) {
region_or(&exclude_rgn, &item->base.rgn);
exclude_region(display, ring, exclude_base, &exclude_rgn, NULL, drawable);
@@ -1621,6 +1694,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;
@@ -1644,6 +1719,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/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