[gnome-builder/wip/chergert/gutter] document gutter renderer and cleanup

[Christian Hergert <chergert src gnome org>]

commit b6cab7b8418b0aa1fcbf407dbd9cf01a4358bedc
Author: Christian Hergert <chergert redhat com>
Date:   Mon Sep 18 02:11:45 2017 -0700

    document gutter renderer and cleanup

 libide/sourceview/ide-omni-gutter-renderer.c |  452 +++++++++++++++++++-------
 1 files changed, 335 insertions(+), 117 deletions(-)
---
diff --git a/libide/sourceview/ide-omni-gutter-renderer.c b/libide/sourceview/ide-omni-gutter-renderer.c
index 2d8d112..6486888 100644
--- a/libide/sourceview/ide-omni-gutter-renderer.c
+++ b/libide/sourceview/ide-omni-gutter-renderer.c
@@ -33,6 +33,24 @@
 #include "sourceview/ide-omni-gutter-renderer.h"
 #include "sourceview/ide-source-view.h"
 
+/**
+ * SECTION:ide-omni-gutter-renderer
+ * @title: IdeOmniGutterRenderer
+ * @short_description: A featureful gutter renderer for the code editor
+ *
+ * This is a #GtkSourceGutterRenderer that knows how to render many of
+ * our components necessary for Builder. Because of the complexity of
+ * Builder, using traditional gutter renderers takes up a great deal
+ * of horizontal space.
+ *
+ * By overlapping some of our components, we can take up less space and
+ * be easier for the user with increased hit-targets.
+ *
+ * Additionally, we can render faster because we can coalesce work.
+ *
+ * Since: 3.28
+ */
+
 #define DIAGNOSTICS_SIZE 16
 #define ARROW_WIDTH      5
 #define CHANGE_WIDTH     2
@@ -51,6 +69,10 @@ struct _IdeOmniGutterRenderer
   DzlSignalGroup *view_signals;
   DzlSignalGroup *buffer_signals;
 
+  /* TODO: It would be nice to use some basic caching here
+   *       so we don't waste 6Kb-12Kb of data on these surfaces.
+   *       But that can be done later after this patch set merges.
+   */
   cairo_surface_t *note_surface;
   cairo_surface_t *warning_surface;
   cairo_surface_t *error_surface;
@@ -58,6 +80,12 @@ struct _IdeOmniGutterRenderer
   cairo_surface_t *warning_selected_surface;
   cairo_surface_t *error_selected_surface;
 
+  /*
+   * We cache various colors we need from the style scheme to avoid
+   * looking them up very often, as it is CPU time consuming. We also
+   * use these colors to prime the symbolic colors for the icon surfaces
+   * to they look appropriate for the style scheme.
+   */
   struct {
     GdkRGBA fg;
     GdkRGBA bg;
@@ -70,15 +98,59 @@ struct _IdeOmniGutterRenderer
     GdkRGBA change;
   } changes;
 
-  PangoLayout   *layout;
+  /*
+   * We need to reuse a single pango layout while drawing all the lines
+   * to keep the overhead low. We don't have pixel caching on the gutter
+   * data so keeping this stuff fast is critical.
+   */
+  PangoLayout *layout;
+
+  /*
+   * We reuse a simple bold attr list for the current line number
+   * information.  This way we don't have to do any pango markup
+   * parsing.
+   */
   PangoAttrList *bold_attrs;
 
+  /* We stash a copy of how long the line numbers could be. 1000 => 4. */
   guint n_chars;
+
+  /* While processing the lines, we track what our first line number is
+   * so that differential calculation for each line is cheap by avoiding
+   * accessing GtkTextIter information.
+   */
   guint begin_line;
-  gint  stopped_line;
+
+  /*
+   * While starting a render, we check to see what the current
+   * breakpoint line is (so we can draw the proper background.
+   *
+   * TODO: Add a callback to the debug manager to avoid querying this
+   *       information on every draw cycle.
+   */
+  gint stopped_line;
+
+  /*
+   * To avoid doing multiple line recalculations inline, we defer our
+   * changed handler until we've re-entered teh main loop. Otherwise
+   * we could handle lots of small changes during automated processing
+   * of the underlying buffer.
+   */
   guint resize_source;
-  gint  number_width;
 
+  /*
+   * The number_width field contains the maximum width of the text as
+   * sized by Pango. It is in pixel units in the scale of the widget
+   * as the underlying components will automatically deal with scaling
+   * for us (as necessary).
+   */
+  gint number_width;
+
+  /*
+   * Some users might want to toggle off individual features of the
+   * omni gutter, and these boolean properties provide that. Other
+   * components map them to GSettings values to be toggled.
+   */
   guint show_line_changes : 1;
   guint show_line_numbers : 1;
   guint show_line_diagnostics : 1;
@@ -99,32 +171,55 @@ enum {
 
 typedef struct
 {
+  /* The line contains a regular breakpoint */
   guint is_breakpoint : 1;
+
+  /* The line contains a countpoint styl breakpoint */
   guint is_countpoint : 1;
+
+  /* The line contains a watchpoint style breakpoint */
   guint is_watchpoint : 1;
+
+  /* The line is an addition to the buffer */
   guint is_add : 1;
+
+  /* The line has changed in the buffer */
   guint is_change : 1;
+
+  /* The line is part of a deleted range in the buffer */
   guint is_delete : 1;
+
+  /* The line contains a diagnostic error */
+  guint is_error : 1;
+
+  /* The line contains a diagnostic warning */
   guint is_warning : 1;
+
+  /* The line contains a diagnostic note */
   guint is_note : 1;
-  guint is_error : 1;
 } LineInfo;
 
 G_DEFINE_TYPE (IdeOmniGutterRenderer, ide_omni_gutter_renderer, GTK_SOURCE_TYPE_GUTTER_RENDERER)
 
 static GParamSpec *properties [N_PROPS];
 
+/*
+ * style_get_is_bold:
+ *
+ * This helper is used to extract the "bold" field from a GtkSourceStyle
+ * within a GtkSourceStyleScheme.
+ *
+ * Returns; %TRUE if @val was set to a trusted value.
+ */
 static gboolean
-get_style_boolean (GtkSourceStyleScheme *scheme,
+style_get_is_bold (GtkSourceStyleScheme *scheme,
                    const gchar          *style_name,
-                   const gchar          *property,
                    gboolean             *val)
 {
   GtkSourceStyle *style;
 
   g_assert (!scheme || GTK_SOURCE_IS_STYLE_SCHEME (scheme));
   g_assert (style_name != NULL);
-  g_assert (property != NULL);
   g_assert (val != NULL);
 
   *val = FALSE;
@@ -132,24 +227,31 @@ get_style_boolean (GtkSourceStyleScheme *scheme,
   if (scheme == NULL)
     return FALSE;
 
-  style = gtk_source_style_scheme_get_style (scheme, style_name);
-
-  if (style != NULL)
+  if (NULL != (style = gtk_source_style_scheme_get_style (scheme, style_name)))
     {
-      g_autofree gchar *property_set = g_strdup_printf ("%s-set", property);
-      gboolean set = FALSE;
-
+      gboolean bold_set = FALSE;
       g_object_get (style,
-                    property, val,
-                    property_set, &set,
+                    "bold-set", &bold_set,
+                    "bold", val,
                     NULL);
-
-      return set;
+      return bold_set;
     }
 
   return FALSE;
 }
 
+/*
+ * get_style_rgba:
+ *
+ * Gets a #GdkRGBA for a particular field of a style within @scheme.
+ *
+ * @type should be set to BACKGROUND or FOREGROUND.
+ *
+ * If we fail to locate the style, @rgba is set to transparent black.
+ * such as #rgba(0,0,0,0).
+ *
+ * Returns: %TRUE if the value placed into @rgba can be trusted.
+ */
 static gboolean
 get_style_rgba (GtkSourceStyleScheme *scheme,
                 const gchar          *style_name,
@@ -168,9 +270,7 @@ get_style_rgba (GtkSourceStyleScheme *scheme,
   if (scheme == NULL)
     return FALSE;
 
-  style = gtk_source_style_scheme_get_style (scheme, style_name);
-
-  if (style != NULL)
+  if (NULL != (style = gtk_source_style_scheme_get_style (scheme, style_name)))
     {
       g_autofree gchar *str = NULL;
       gboolean set = FALSE;
@@ -180,11 +280,10 @@ get_style_rgba (GtkSourceStyleScheme *scheme,
                     type ? "background-set" : "foreground-set", &set,
                     NULL);
 
-      if (str != NULL && set)
-        {
-          gdk_rgba_parse (rgba, str);
-          return TRUE;
-        }
+      if (str != NULL)
+        gdk_rgba_parse (rgba, str);
+
+      return set;
     }
 
   return FALSE;
@@ -214,13 +313,14 @@ reload_style_colors (IdeOmniGutterRenderer *self,
   gtk_style_context_get_background_color (context, state, &bg);
   G_GNUC_END_IGNORE_DEPRECATIONS;
 
+  /* Extract common values from style schemes. */
   if (!get_style_rgba (scheme, "line-numbers", FOREGROUND, &self->text.fg))
     self->text.fg = fg;
 
   if (!get_style_rgba (scheme, "line-numbers", BACKGROUND, &self->text.bg))
     self->text.bg = bg;
 
-  if (!get_style_boolean (scheme, "line-numbers", "bold", &self->text.bold))
+  if (!style_get_is_bold (scheme, "line-numbers", &self->text.bold))
     self->text.bold = FALSE;
 
   if (!get_style_rgba (scheme, "current-line-number", FOREGROUND, &self->current.fg))
@@ -229,9 +329,14 @@ reload_style_colors (IdeOmniGutterRenderer *self,
   if (!get_style_rgba (scheme, "current-line-number", BACKGROUND, &self->current.bg))
     self->current.bg = bg;
 
-  if (!get_style_boolean (scheme, "current-line-number", "bold", &self->current.bold))
+  if (!style_get_is_bold (scheme, "current-line-number", &self->current.bold))
     self->current.bold = TRUE;
 
+  /*
+   * These debugger:: prefix values come from Builder's style-scheme xml
+   * as well as in the IdeBuffer class. Other style schemes may also
+   * support them, though.
+   */
   if (!get_style_rgba (scheme, "debugger::current-breakpoint", BACKGROUND, &self->stopped_bg))
     gdk_rgba_parse (&self->stopped_bg, "#fcaf3e");
 
@@ -241,9 +346,12 @@ reload_style_colors (IdeOmniGutterRenderer *self,
   if (!get_style_rgba (scheme, "debugger::breakpoint", BACKGROUND, &self->bkpt.bg))
     get_style_rgba (scheme, "selection", BACKGROUND, &self->bkpt.bg);
 
-  if (!get_style_boolean (scheme, "debugger::breakpoint", "bold", &self->bkpt.bold))
+  if (!style_get_is_bold (scheme, "debugger::breakpoint", &self->bkpt.bold))
     self->bkpt.bold = FALSE;
 
+  /* These gutter:: prefix values come from Builder's style-scheme xml
+   * files, but other style schemes may also support them now too.
+   */
   if (!get_style_rgba (scheme, "gutter::added-line", FOREGROUND, &self->changes.add))
     gdk_rgba_parse (&self->changes.add, "#8ae234");
 
@@ -265,6 +373,10 @@ collect_breakpoint_info (IdeDebuggerBreakpoint *breakpoint,
   } *bkpt_info = user_data;
   guint line;
 
+  g_assert (IDE_IS_DEBUGGER_BREAKPOINT (breakpoint));
+  g_assert (bkpt_info != NULL);
+
+  /* Debugger breakpoints are 1-based line numbers */
   if (!(line = ide_debugger_breakpoint_get_line (breakpoint)))
     return;
 
@@ -287,25 +399,27 @@ ide_omni_gutter_renderer_load_breakpoints (IdeOmniGutterRenderer *self,
                                            GtkTextIter           *end,
                                            GArray                *lines)
 {
-  struct {
-    GArray *lines;
-    guint begin;
-    guint end;
-  } bkpt_info;
-
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
   g_assert (begin != NULL);
   g_assert (lines != NULL);
   g_assert (lines->len > 0);
 
-  bkpt_info.lines = lines;
-  bkpt_info.begin = gtk_text_iter_get_line (begin);
-  bkpt_info.end = gtk_text_iter_get_line (end);
-
   if (self->breakpoints != NULL)
-    ide_debugger_breakpoints_foreach (self->breakpoints,
-                                      (GFunc)collect_breakpoint_info,
-                                      &bkpt_info);
+    {
+      struct {
+        GArray *lines;
+        guint begin;
+        guint end;
+      } info;
+
+      info.lines = lines;
+      info.begin = gtk_text_iter_get_line (begin);
+      info.end = gtk_text_iter_get_line (end);
+
+      ide_debugger_breakpoints_foreach (self->breakpoints,
+                                        (GFunc)collect_breakpoint_info,
+                                        &info);
+    }
 }
 
 static void
@@ -313,7 +427,7 @@ ide_omni_gutter_renderer_load_basic (IdeOmniGutterRenderer *self,
                                      GtkTextIter           *begin,
                                      GArray                *lines)
 {
-  IdeBuffer *buffer;
+  GtkTextBuffer *buffer;
   guint line;
 
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
@@ -321,27 +435,23 @@ ide_omni_gutter_renderer_load_basic (IdeOmniGutterRenderer *self,
   g_assert (lines != NULL);
   g_assert (lines->len > 0);
 
-  buffer = (IdeBuffer *)gtk_text_iter_get_buffer (begin);
-
+  buffer = gtk_text_iter_get_buffer (begin);
   if (!IDE_IS_BUFFER (buffer))
     return;
 
   line = gtk_text_iter_get_line (begin);
 
-  if (buffer != NULL)
+  for (guint i = 0; i < lines->len; i++)
     {
-      for (guint i = 0; i < lines->len; i++)
-        {
-          LineInfo *info = &g_array_index (lines, LineInfo, i);
-          IdeBufferLineFlags flags = ide_buffer_get_line_flags (buffer, line + i);
-
-          info->is_add = !!(flags & IDE_BUFFER_LINE_FLAGS_ADDED);
-          info->is_change = !!(flags & IDE_BUFFER_LINE_FLAGS_CHANGED);
-          info->is_delete = !!(flags & IDE_BUFFER_LINE_FLAGS_DELETED);
-          info->is_warning = !!(flags & IDE_BUFFER_LINE_FLAGS_WARNING);
-          info->is_note = !!(flags & IDE_BUFFER_LINE_FLAGS_NOTE);
-          info->is_error = !!(flags & IDE_BUFFER_LINE_FLAGS_ERROR);
-        }
+      LineInfo *info = &g_array_index (lines, LineInfo, i);
+      IdeBufferLineFlags flags = ide_buffer_get_line_flags (IDE_BUFFER (buffer), line + i);
+
+      info->is_add = !!(flags & IDE_BUFFER_LINE_FLAGS_ADDED);
+      info->is_change = !!(flags & IDE_BUFFER_LINE_FLAGS_CHANGED);
+      info->is_delete = !!(flags & IDE_BUFFER_LINE_FLAGS_DELETED);
+      info->is_warning = !!(flags & IDE_BUFFER_LINE_FLAGS_WARNING);
+      info->is_note = !!(flags & IDE_BUFFER_LINE_FLAGS_NOTE);
+      info->is_error = !!(flags & IDE_BUFFER_LINE_FLAGS_ERROR);
     }
 }
 
@@ -368,7 +478,7 @@ ide_omni_gutter_renderer_recalculate_size (IdeOmniGutterRenderer *self)
   const PangoFontDescription *font_desc;
   g_autofree gchar *numbers = NULL;
   GtkTextBuffer *buffer;
-  IdeSourceView *view;
+  GtkTextView *view;
   PangoLayout *layout;
   GtkTextIter end;
   guint line;
@@ -377,35 +487,44 @@ ide_omni_gutter_renderer_recalculate_size (IdeOmniGutterRenderer *self)
 
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
 
-  view = (IdeSourceView *)gtk_source_gutter_renderer_get_view (GTK_SOURCE_GUTTER_RENDERER (self));
+  /* There is nothing we can do until a view has been attached. */
+  view = gtk_source_gutter_renderer_get_view (GTK_SOURCE_GUTTER_RENDERER (self));
   if (!IDE_IS_SOURCE_VIEW (view))
     return;
 
   /*
    * First, we need to get the size of the text for the last line of the
-   * buffer, which will be the longest. We size the font with '9' since it
-   * will generally be the widest of the numbers. Although, we only support
-   * monospace anyway, so it shouldn't matter.
+   * buffer (which will be the longest). We size the font with '9' since it
+   * will generally be one of the widest of the numbers. Although, we only
+   * "support" * monospace anyway, so it shouldn't be drastic if we're off.
    */
 
-  buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
+  buffer = gtk_text_view_get_buffer (view);
   gtk_text_buffer_get_end_iter (buffer, &end);
   line = gtk_text_iter_get_line (&end) + 1;
 
   self->n_chars = count_num_digits (line);
   numbers = g_strnfill (self->n_chars, '9');
 
-  font_desc = ide_source_view_get_font_desc (view);
+  /*
+   * Get the font description used by the IdeSourceView so we can
+   * match the font styling as much as possible.
+   */
+  font_desc = ide_source_view_get_font_desc (IDE_SOURCE_VIEW (view));
   layout = gtk_widget_create_pango_layout (GTK_WIDGET (view), numbers);
   pango_layout_set_font_description (layout, font_desc);
+
+  /*
+   * Now cache the width of the text layout so we can simplify our
+   * positioning later. We simply size everything the same and then
+   * align to the right to reduce the draw overhead.
+   */
   pango_layout_get_pixel_size (layout, &self->number_width, &height);
-  g_clear_object (&layout);
 
+  /* Now calculate the size based on enabled features */
   size = 2;
-
   if (self->show_line_diagnostics)
     size += DIAGNOSTICS_SIZE + 2;
-
   if (self->show_line_numbers)
     size += self->number_width + 2;
 
@@ -417,9 +536,11 @@ ide_omni_gutter_renderer_recalculate_size (IdeOmniGutterRenderer *self)
   else if (self->show_line_changes)
     size += CHANGE_WIDTH + 2;
 
+  /* Update the size and ensure we are re-drawn */
   gtk_source_gutter_renderer_set_size (GTK_SOURCE_GUTTER_RENDERER (self), size );
-
   gtk_source_gutter_renderer_queue_draw (GTK_SOURCE_GUTTER_RENDERER (self));
+
+  g_clear_object (&layout);
 }
 
 static void
@@ -466,16 +587,20 @@ ide_omni_gutter_renderer_begin (GtkSourceGutterRenderer *renderer,
   g_assert (begin != NULL);
   g_assert (end != NULL);
 
+  /*
+   * This is the start of our draw process. The first thing we want to
+   * do is collect as much information as we'll need when doing the
+   * actual draw. That helps us coalesce similar work together, which is
+   * good for the CPU usage. We are *very* sensitive to CPU usage here
+   * as the GtkTextView does not pixel cache the gutter.
+   */
+
   self->stopped_line = -1;
 
+  /* Locate the current stopped breakpoint if any. */
   buffer = gtk_text_iter_get_buffer (begin);
   table = gtk_text_buffer_get_tag_table (buffer);
   tag = gtk_text_tag_table_lookup (table, "debugger::current-breakpoint");
-
-  /*
-   * Locate the current stopped breakpoint if any.
-   */
-
   if (tag != NULL)
     {
       bkpt = *begin;
@@ -496,17 +621,22 @@ ide_omni_gutter_renderer_begin (GtkSourceGutterRenderer *renderer,
   self->begin_line = gtk_text_iter_get_line (begin);
   end_line = gtk_text_iter_get_line (end);
 
+  /* Give ourselves a fresh array to stash our line info */
   g_array_set_size (self->lines, end_line - self->begin_line + 1);
   memset (self->lines->data, 0, self->lines->len * sizeof (LineInfo));
 
+  /* Now load breakpoints, diagnostics, and line changes */
   ide_omni_gutter_renderer_load_basic (self, begin, self->lines);
   ide_omni_gutter_renderer_load_breakpoints (self, begin, end, self->lines);
 
+  /* Create a new layout for rendering lines to */
   self->layout = gtk_widget_create_pango_layout (GTK_WIDGET (view), "");
   pango_layout_set_alignment (self->layout, PANGO_ALIGN_RIGHT);
-  pango_layout_set_font_description (self->layout,
-                                     ide_source_view_get_font_desc (view));
+  pango_layout_set_font_description (self->layout, ide_source_view_get_font_desc (view));
 
+  /* Tweak the sizing (for proper alignment) based on the if we are
+   * going to be rendering the breakpoints arrow or not.
+   */
   if (self->breakpoints != NULL)
     pango_layout_set_width (self->layout, (cell_area->width - ARROW_WIDTH - 4) * PANGO_SCALE);
   else
@@ -519,14 +649,14 @@ ide_omni_gutter_renderer_query_activatable (GtkSourceGutterRenderer *renderer,
                                             GdkRectangle            *area,
                                             GdkEvent                *event)
 {
-  IdeOmniGutterRenderer *self = (IdeOmniGutterRenderer *)renderer;
-
-  g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
+  g_assert (IDE_IS_OMNI_GUTTER_RENDERER (renderer));
   g_assert (begin != NULL);
   g_assert (area != NULL);
   g_assert (event != NULL);
 
-  return (self->breakpoints != NULL);
+  /* Clicking will move the cursor, so always TRUE */
+
+  return TRUE;
 }
 
 static void
@@ -541,6 +671,8 @@ animate_at_iter (IdeOmniGutterRenderer *self,
   g_assert (area != NULL);
   g_assert (iter != NULL);
 
+  /* Show a little bullet animation shooting right */
+
   view = gtk_source_gutter_renderer_get_view (GTK_SOURCE_GUTTER_RENDERER (self));
 
   theatric = g_object_new (DZL_TYPE_BOX_THEATRIC,
@@ -590,6 +722,11 @@ ide_omni_gutter_renderer_activate (GtkSourceGutterRenderer *renderer,
   g_assert (event != NULL);
   g_assert (self->breakpoints != NULL);
 
+  /* TODO: We could check for event->button.button to see if we
+   *       can display a popover with information such as
+   *       diagnostics, or breakpoints, or git blame.
+   */
+
   buffer = gtk_text_iter_get_buffer (iter);
   context = ide_buffer_get_context (IDE_BUFFER (buffer));
   debug_manager = ide_context_get_debug_manager (context);
@@ -665,6 +802,11 @@ draw_breakpoint_bg (IdeOmniGutterRenderer        *self,
   g_assert (cr != NULL);
   g_assert (bg_area != NULL);
 
+  /*
+   * This draws a little arrow starting from the left and pointing
+   * over the line changes portion of the gutter.
+   */
+
   area.x = bg_area->x;
   area.y = bg_area->y;
   area.height = bg_area->height;
@@ -685,6 +827,11 @@ draw_breakpoint_bg (IdeOmniGutterRenderer        *self,
 
   rgba = self->bkpt.bg;
 
+  /*
+   * Tweak the brightness based on if we are in pre-light and
+   * if we are also still an active breakpoint.
+   */
+
   if ((state & GTK_SOURCE_GUTTER_RENDERER_STATE_PRELIT) != 0)
     {
       if (IS_BREAKPOINT (info))
@@ -693,6 +840,8 @@ draw_breakpoint_bg (IdeOmniGutterRenderer        *self,
         rgba.alpha *= 0.4;
     }
 
+  /* And draw... */
+
   gdk_cairo_set_source_rgba (cr, &rgba);
   cairo_fill (cr);
 }
@@ -708,18 +857,30 @@ draw_add_change (IdeOmniGutterRenderer        *self,
   g_assert (cr != NULL);
   g_assert (area != NULL);
 
-  cairo_rectangle (cr,
-                   area->x + area->width - 2 - CHANGE_WIDTH,
-                   area->y,
-                   CHANGE_WIDTH,
-                   area->y + area->height);
+  /*
+   * Draw a simple line with the appropriate color from the style scheme
+   * based on the type of change we have.
+   */
 
-  if (info->is_add)
-    gdk_cairo_set_source_rgba (cr, &self->changes.add);
-  else
-    gdk_cairo_set_source_rgba (cr, &self->changes.change);
+  if (info->is_add || info->is_change)
+    {
+      cairo_rectangle (cr,
+                       area->x + area->width - 2 - CHANGE_WIDTH,
+                       area->y,
+                       CHANGE_WIDTH,
+                       area->y + area->height);
 
-  cairo_fill (cr);
+      if (info->is_add)
+        gdk_cairo_set_source_rgba (cr, &self->changes.add);
+      else
+        gdk_cairo_set_source_rgba (cr, &self->changes.change);
+
+      cairo_fill (cr);
+    }
+
+  /*
+   * TODO: Draw a red deletion mark if necessary.
+   */
 }
 
 static void
@@ -790,6 +951,13 @@ ide_omni_gutter_renderer_draw (GtkSourceGutterRenderer      *renderer,
   g_assert (begin != NULL);
   g_assert (end != NULL);
 
+  /*
+   * This is our primary draw routine. It is called for every line that
+   * is visible. We are incredibly sensitive to performance churn here
+   * so it is important that we be as minimal as possible while
+   * retaining the features we need.
+   */
+
   view = gtk_source_gutter_renderer_get_view (GTK_SOURCE_GUTTER_RENDERER (self));
   highlight_line = gtk_source_view_get_highlight_current_line (GTK_SOURCE_VIEW (view));
   has_focus = gtk_widget_has_focus (GTK_WIDGET (view));
@@ -805,6 +973,12 @@ ide_omni_gutter_renderer_draw (GtkSourceGutterRenderer      *renderer,
       gchar linestr[16];
       gint len;
 
+      /*
+       * Draw some background for the line so that it looks like the
+       * breakpoint arrow draws over it. Debugger break line takes
+       * precidence over the current highlight line. Also, ensure that
+       * the view is drawing the highlight line first.
+       */
       if (line == self->stopped_line)
         {
           gdk_cairo_rectangle (cr, bg_area);
@@ -818,9 +992,15 @@ ide_omni_gutter_renderer_draw (GtkSourceGutterRenderer      *renderer,
           cairo_fill (cr);
         }
 
+      /* Draw line changes next so it will show up underneath the
+       * breakpoint arrows.
+       */
       if (self->show_line_changes && (info->is_add || info->is_change))
         draw_add_change (self, cr, cell_area, info, state);
 
+      /* Draw breakpoint arrows if we have any breakpoints that could
+       * potentially match.
+       */
       if (self->breakpoints != NULL)
         {
           has_breakpoint = IS_BREAKPOINT (info);
@@ -828,11 +1008,24 @@ ide_omni_gutter_renderer_draw (GtkSourceGutterRenderer      *renderer,
             draw_breakpoint_bg (self, cr, cell_area, info, state);
         }
 
+      /* Now that we might have an altered background for the line,
+       * we can draw the diagnostic icon (with possibly altered
+       * color for symbolic icon).
+       */
       if (self->show_line_diagnostics && IS_DIAGNOSTIC (info))
         draw_diagnostic (self, cr, cell_area, info, state);
 
+      /*
+       * Now draw the line numbers if we are showing them. Ensure
+       * we tweak the style to match closely to how the default
+       * gtksourceview lines gutter renderer does it.
+       */
       if (self->show_line_numbers)
         {
+          /* TODO: Easy performance win here is to use an array of
+           *       strings containing the first 1000 or so line numbers
+           *       with direct index offsets.
+           */
           len = g_snprintf (linestr, sizeof linestr, "%u", line + 1);
           pango_layout_set_text (self->layout, linestr, len);
 
@@ -854,6 +1047,7 @@ ide_omni_gutter_renderer_draw (GtkSourceGutterRenderer      *renderer,
               bold = self->text.bold;
             }
 
+          /* Current line is always bold */
           if (state & GTK_SOURCE_GUTTER_RENDERER_STATE_CURSOR)
             bold |= self->current.bold;
 
@@ -880,6 +1074,16 @@ get_icon_surface (IdeOmniGutterRenderer *self,
   g_assert (GTK_IS_WIDGET (widget));
   g_assert (icon_name != NULL);
 
+  /*
+   * This deals with loading a given icon by icon name and trying to
+   * apply our current style as the symbolic colors. We do not support
+   * error/warning/etc for symbolic icons so they are all replaced with
+   * the proper foreground color.
+   *
+   * If selected is set, we alter the color to make sure it will look
+   * good on top of a breakpoint arrow.
+   */
+
   screen = gtk_widget_get_screen (widget);
   icon_theme = gtk_icon_theme_get_for_screen (screen);
 
@@ -920,6 +1124,15 @@ ide_omni_gutter_renderer_reload_icons (IdeOmniGutterRenderer *self)
 
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
 
+  /*
+   * This isn't ideal (we should find a better way to cache icons that
+   * is safe with scale and foreground color changes we need).
+   *
+   * TODO: Create something similar to pixbuf helpers that allow for
+   *       more control over the cache key so it can be shared between
+   *       multiple instances.
+   */
+
   g_clear_pointer (&self->note_surface, cairo_surface_destroy);
   g_clear_pointer (&self->warning_surface, cairo_surface_destroy);
   g_clear_pointer (&self->error_surface, cairo_surface_destroy);
@@ -954,6 +1167,12 @@ ide_omni_gutter_renderer_reload (IdeOmniGutterRenderer *self)
   view = gtk_source_gutter_renderer_get_view (GTK_SOURCE_GUTTER_RENDERER (self));
   buffer = gtk_text_view_get_buffer (view);
 
+  /*
+   * Use the Language ID to determine if it makes sense to show
+   * breakpoints. We don't want to show them for things like
+   * markdown files and such.
+   */
+
   if (NULL != (language = gtk_source_buffer_get_language (GTK_SOURCE_BUFFER (buffer))))
     id = gtk_source_language_get_id (language);
 
@@ -971,49 +1190,44 @@ ide_omni_gutter_renderer_reload (IdeOmniGutterRenderer *self)
         }
     }
 
+  /* Replace our previous breakpoints */
   g_set_object (&self->breakpoints, breakpoints);
 
+  /* Reload icons and then recalcuate our physical size */
   ide_omni_gutter_renderer_reload_icons (self);
   ide_omni_gutter_renderer_recalculate_size (self);
 }
 
 static void
-ide_omni_gutter_renderer_bind_view (IdeOmniGutterRenderer *self,
-                                    IdeSourceView         *view,
-                                    DzlSignalGroup        *view_signals)
+ide_omni_gutter_renderer_notify_buffer (IdeOmniGutterRenderer *self,
+                                        GParamSpec            *pspec,
+                                        IdeSourceView         *view)
 {
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
   g_assert (IDE_IS_SOURCE_VIEW (view));
-  g_assert (DZL_IS_SIGNAL_GROUP (view_signals));
 
   if (self->buffer_signals != NULL)
     {
-      GtkTextBuffer *buffer;
+      GtkTextBuffer *buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
 
-      buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
-      if (IDE_IS_BUFFER (buffer))
-        dzl_signal_group_set_target (self->buffer_signals, buffer);
+      if (!IDE_IS_BUFFER (buffer))
+        buffer = NULL;
 
+      dzl_signal_group_set_target (self->buffer_signals, buffer);
       ide_omni_gutter_renderer_reload (self);
     }
 }
 
 static void
-ide_omni_gutter_renderer_notify_buffer (IdeOmniGutterRenderer *self,
-                                        GParamSpec            *pspec,
-                                        IdeSourceView         *view)
+ide_omni_gutter_renderer_bind_view (IdeOmniGutterRenderer *self,
+                                    IdeSourceView         *view,
+                                    DzlSignalGroup        *view_signals)
 {
-  GtkTextBuffer *buffer;
-
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
-  g_assert (pspec != NULL);
   g_assert (IDE_IS_SOURCE_VIEW (view));
+  g_assert (DZL_IS_SIGNAL_GROUP (view_signals));
 
-  buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
-  if (IDE_IS_BUFFER (buffer))
-    dzl_signal_group_set_target (self->buffer_signals, buffer);
-
-  ide_omni_gutter_renderer_reload (self);
+  ide_omni_gutter_renderer_notify_buffer (self, NULL, view);
 }
 
 static void
@@ -1036,6 +1250,9 @@ ide_omni_gutter_renderer_notify_view (IdeOmniGutterRenderer *self)
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
 
   view = gtk_source_gutter_renderer_get_view (GTK_SOURCE_GUTTER_RENDERER (self));
+  if (!IDE_IS_SOURCE_VIEW (view))
+    view = NULL;
+
   dzl_signal_group_set_target (self->view_signals, view);
 }
 
@@ -1047,9 +1264,7 @@ ide_omni_gutter_renderer_do_recalc (gpointer data)
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
 
   self->resize_source = 0;
-
   ide_omni_gutter_renderer_recalculate_size (self);
-
   return G_SOURCE_REMOVE;
 }
 
@@ -1060,6 +1275,7 @@ ide_omni_gutter_renderer_buffer_changed (IdeOmniGutterRenderer *self,
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
   g_assert (IDE_IS_BUFFER (buffer));
 
+  /* Run immediately at the end of this main loop iteration */
   if (self->resize_source == 0)
     self->resize_source = gdk_threads_add_idle_full (G_PRIORITY_HIGH,
                                                      ide_omni_gutter_renderer_do_recalc,
@@ -1077,8 +1293,11 @@ ide_omni_gutter_renderer_notify_style_scheme (IdeOmniGutterRenderer *self,
   g_assert (IDE_IS_OMNI_GUTTER_RENDERER (self));
   g_assert (IDE_IS_BUFFER (buffer));
 
+  /* Update our cached rgba colors */
   scheme = gtk_source_buffer_get_style_scheme (GTK_SOURCE_BUFFER (buffer));
   reload_style_colors (self, scheme);
+
+  /* Regenerate icons matching the scheme colors */
   ide_omni_gutter_renderer_reload_icons (self);
 }
 
@@ -1115,6 +1334,12 @@ ide_omni_gutter_renderer_dispose (GObject *object)
 
   ide_clear_source (&self->resize_source);
 
+  g_clear_object (&self->breakpoints);
+  g_clear_pointer (&self->lines, g_array_unref);
+
+  g_clear_object (&self->view_signals);
+  g_clear_object (&self->buffer_signals);
+
   g_clear_pointer (&self->note_surface, cairo_surface_destroy);
   g_clear_pointer (&self->warning_surface, cairo_surface_destroy);
   g_clear_pointer (&self->error_surface, cairo_surface_destroy);
@@ -1122,16 +1347,9 @@ ide_omni_gutter_renderer_dispose (GObject *object)
   g_clear_pointer (&self->warning_selected_surface, cairo_surface_destroy);
   g_clear_pointer (&self->error_selected_surface, cairo_surface_destroy);
 
-  g_clear_pointer (&self->lines, g_array_unref);
-
   g_clear_object (&self->layout);
   g_clear_pointer (&self->bold_attrs, pango_attr_list_unref);
 
-  g_clear_object (&self->breakpoints);
-
-  g_clear_object (&self->buffer_signals);
-  g_clear_object (&self->view_signals);
-
   G_OBJECT_CLASS (ide_omni_gutter_renderer_parent_class)->dispose (object);
 }