[Spice-devel] [PATCH] fixup! DisplayChannel: document exclude_region() functions

[Jonathon Jongsma]

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;
                 }
-- 
2.9.3