>
> Potential changes to amend with previous commit per suggestions by Uri,
> Frediano.
> ---
> server/display-channel.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 4ee842a..5f4caf9 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -592,7 +592,7 @@ static bool current_add_equal(DisplayChannel *display,
> DrawItem *item, TreeItem
> *
> * If there is overlap between @rgn and the @item region, remove the
> * overlapping intersection from both @rgn and the item's region (NOTE: it's
> - * not clear to me why this is done)
> + * not clear to me why this is done - jjongsma)
> *
> * 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
> when
> @@ -626,8 +626,8 @@ static bool current_add_equal(DisplayChannel *display,
> DrawItem *item, TreeItem
> * @ring: a fallback toplevel ring???
> * @item: the tree item to exclude from @rgn
> * @rgn: the region to exclude
> - * @top_ring: ?
> - * @frame_candidate: ?
> + * @top_ring: ???
> + * @frame_candidate: ???
> */
> static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem
> *item, QRegion *rgn,
> Ring **top_ring, Drawable *frame_candidate)
> @@ -683,7 +683,7 @@ static void __exclude_region(DisplayChannel *display,
> Ring *ring, TreeItem *item
> }
> }
> } else {
> - /* NOTE: It's unclear to me why this code is necessary here
> */
> + /* TODO: document the purpose of this code */
> if (frame_candidate) {
> Drawable *drawable = SPICE_CONTAINEROF(draw, Drawable,
> tree_item);
> stream_maintenance(display, frame_candidate, drawable);
> @@ -700,13 +700,14 @@ static void __exclude_region(DisplayChannel *display,
> Ring *ring, TreeItem *item
> Shadow *shadow;
>
> /* exclude the intersection from the 'rgn' argument as well,
> - * but only if the item is now empty... WHY??? */
> + * but only if the item is now empty.
> + * TODO: explain why this is necessary */
> 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)) {
> - /* NOTE: it's unclear why top_ring is set here */
> + /* TODO: document why top_ring is set here */
> *top_ring = tree_item_container_items(&shadow->base,
> ring);
> }
> }
> @@ -737,8 +738,7 @@ static void __exclude_region(DisplayChannel *display,
> Ring *ring, TreeItem *item
> * Any items that have an empty region as a result of the __exclude_region()
> * call are removed from the tree.
> *
> - * NOTE: I still don't have a great conceptual understanding of this
> function's
> - * intended use.
> + * TODO: What is the intended use of 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.
> @@ -751,7 +751,7 @@ static void __exclude_region(DisplayChannel *display,
> Ring *ring, TreeItem *item
> * 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?
> + * that's being added to the 'current' ring. TODO: What is its purpose?
> */
> static void exclude_region(DisplayChannel *display, Ring *ring, RingItem
> *ring_item,
> QRegion *rgn, TreeItem **last, Drawable
> *frame_candidate)
> @@ -954,9 +954,9 @@ static bool current_add(DisplayChannel *display, Ring
> *ring, Drawable *drawable)
> * 'on_hold' regions from the shadows of previous
> * iterations
> *
> - * NOTE: it's unclear to me why we only only call
> + * TODO: it's unclear to me why we only only call
> * exclude_region() for the previous item if the
> next
> - * item is obscured and has a shadow.
> + * item is obscured and has a shadow. -jjongsma
> */
> TreeItem *next = sibling;
> exclude_region(display, ring, exclude_base,
> &exclude_rgn, &next, NULL);
> @@ -989,7 +989,7 @@ static bool current_add(DisplayChannel *display, Ring
> *ring, Drawable *drawable)
> if (shadow || skip) {
> /* 'now' is currently set to the the item immediately
> AFTER
> * the obscured sibling that we just removed.
> - * NOTE: it's unclear to me Why this item is used as an
> + * TODO: document why this item is used as an
> * 'exclude_base' */
> exclude_base = now;
> }
Ack with this change
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel