Use util_clip_triangle() in pan_item_tri_new()
[geeqie.git] / src / pixbuf-util.cc
index c5f16ae..0735a04 100644 (file)
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#include "main.h"
 #include "pixbuf-util.h"
 
+#include <algorithm>
+#include <cmath>
+#include <cstdlib>
+#include <cstring>
+#include <utility>
+
+#include <cairo.h>
+#include <gio/gio.h>
+#include <glib-object.h>
+#include <pango/pangocairo.h>
+
+#include <config.h>
+
+#include "debug.h"
 #include "exif.h"
 #include "filedata.h"
+#include "main-defines.h"
+#include "typedefs.h"
 #include "ui-fileops.h"
 
-#include <cmath>
-
 
 /*
  *-----------------------------------------------------------------------------
@@ -64,7 +77,7 @@ gboolean pixbuf_to_file_as_png(GdkPixbuf *pixbuf, const gchar *filename)
 #pragma GCC diagnostic ignored "-Wunused-function"
 gboolean pixbuf_to_file_as_jpg_unused(GdkPixbuf *pixbuf, const gchar *filename, gint quality)
 {
-       GError *error = NULL;
+       GError *error = nullptr;
        gchar *qbuf;
        gboolean ret;
 
@@ -357,7 +370,8 @@ GdkPixbuf *pixbuf_fallback(FileData *fd, gint requested_width, gint requested_he
 
                if (w > requested_width || h > requested_height)
                        {
-                       gint nw, nh;
+                       gint nw;
+                       gint nh;
 
                        if (pixbuf_scale_aspect(requested_width, requested_height,
                                                          w, h, &nw, &nh))
@@ -381,24 +395,23 @@ GdkPixbuf *pixbuf_fallback(FileData *fd, gint requested_width, gint requested_he
  */
 
 gboolean util_clip_region(gint x, gint y, gint w, gint h,
-                         gint clip_x, gint clip_y, gint clip_w, gint clip_h,
-                         gint *rx, gint *ry, gint *rw, gint *rh)
+                          gint clip_x, gint clip_y, gint clip_w, gint clip_h,
+                          gint &rx, gint &ry, gint &rw, gint &rh)
 {
-       if (clip_x + clip_w <= x ||
-           clip_x >= x + w ||
-           clip_y + clip_h <= y ||
-           clip_y >= y + h)
+       GdkRectangle main{x, y, w, h};
+       GdkRectangle clip{clip_x, clip_y, clip_w, clip_h};
+       GdkRectangle r;
+
+       const gboolean rectangles_intersect = gdk_rectangle_intersect(&main, &clip, &r);
+       if (rectangles_intersect)
                {
-               return FALSE;
+               rx = r.x;
+               ry = r.y;
+               rw = r.width;
+               rh = r.height;
                }
 
-       *rx = MAX(x, clip_x);
-       *rw = MIN((x + w), (clip_x + clip_w)) - *rx;
-
-       *ry = MAX(y, clip_y);
-       *rh = MIN((y + h), (clip_y + clip_h)) - *ry;
-
-       return TRUE;
+       return rectangles_intersect;
 }
 
 /*
@@ -411,7 +424,8 @@ static void pixbuf_copy_block_rotate(guchar *src, gint src_row_stride, gint x, g
                                     guchar *dest, gint dest_row_stride, gint w, gint h,
                                     gint bytes_per_pixel, gboolean counter_clockwise)
 {
-       gint i, j;
+       gint i;
+       gint j;
        guchar *sp;
        guchar *dp;
 
@@ -452,8 +466,10 @@ static void pixbuf_copy_block(guchar *src, gint src_row_stride, gint w, gint h,
                }
 }
 
-#define ROTATE_BUFFER_WIDTH 48
-#define ROTATE_BUFFER_HEIGHT 48
+enum {
+       ROTATE_BUFFER_WIDTH = 48,
+       ROTATE_BUFFER_HEIGHT = 48
+};
 
 /*
  * Returns a copy of pixbuf src rotated 90 degrees clockwise or 90 counterclockwise
@@ -463,16 +479,22 @@ GdkPixbuf *pixbuf_copy_rotate_90(GdkPixbuf *src, gboolean counter_clockwise)
 {
        GdkPixbuf *dest;
        gboolean has_alpha;
-       gint sw, sh, srs;
-       gint dw, dh, drs;
+       gint sw;
+       gint sh;
+       gint srs;
+       gint dw;
+       gint dh;
+       gint drs;
        guchar *s_pix;
        guchar *d_pix;
-       gint i, j;
+       gint i;
+       gint j;
        gint a;
        GdkPixbuf *buffer;
        guchar *b_pix;
        gint brs;
-       gint w, h;
+       gint w;
+       gint h;
 
        if (!src) return nullptr;
 
@@ -500,7 +522,8 @@ GdkPixbuf *pixbuf_copy_rotate_90(GdkPixbuf *src, gboolean counter_clockwise)
                w = MIN(ROTATE_BUFFER_WIDTH, (sh - i));
                for (j = 0; j < sw; j += ROTATE_BUFFER_HEIGHT)
                        {
-                       gint x, y;
+                       gint x;
+                       gint y;
 
                        h = MIN(ROTATE_BUFFER_HEIGHT, (sw - j));
                        pixbuf_copy_block_rotate(s_pix, srs, j, i,
@@ -561,13 +584,16 @@ GdkPixbuf *pixbuf_copy_mirror(GdkPixbuf *src, gboolean mirror, gboolean flip)
 {
        GdkPixbuf *dest;
        gboolean has_alpha;
-       gint w, h, srs;
+       gint w;
+       gint h;
+       gint srs;
        gint drs;
        guchar *s_pix;
        guchar *d_pix;
        guchar *sp;
        guchar *dp;
-       gint i, j;
+       gint i;
+       gint j;
        gint a;
 
        if (!src) return nullptr;
@@ -686,10 +712,13 @@ void pixbuf_draw_rect_fill(GdkPixbuf *pb,
                           gint r, gint g, gint b, gint a)
 {
        gboolean has_alpha;
-       gint pw, ph, prs;
+       gint pw;
+       gint ph;
+       gint prs;
        guchar *p_pix;
        guchar *pp;
-       gint i, j;
+       gint i;
+       gint j;
 
        if (!pb) return;
 
@@ -703,18 +732,19 @@ void pixbuf_draw_rect_fill(GdkPixbuf *pb,
        prs = gdk_pixbuf_get_rowstride(pb);
        p_pix = gdk_pixbuf_get_pixels(pb);
 
+       const gint p_step = has_alpha ? 4 : 3;
+
        for (i = 0; i < h; i++)
                {
-               pp = p_pix + (y + i) * prs + (x * (has_alpha ? 4 : 3));
+               pp = p_pix + (y + i) * prs + (x * p_step);
                for (j = 0; j < w; j++)
                        {
-                       *pp = (r * a + *pp * (256-a)) >> 8;
-                       pp++;
-                       *pp = (g * a + *pp * (256-a)) >> 8;
-                       pp++;
-                       *pp = (b * a + *pp * (256-a)) >> 8;
-                       pp++;
-                       if (has_alpha) pp++;
+                       pp[0] = (r * a + pp[0] * (256-a)) >> 8;
+                       pp[1] = (g * a + pp[1] * (256-a)) >> 8;
+                       pp[2] = (b * a + pp[2] * (256-a)) >> 8;
+                        // TODO(xsdg): Should we do anything about a potential
+                        // existing alpha value here?
+                       pp += p_step;
                        }
                }
 }
@@ -742,10 +772,13 @@ void pixbuf_set_rect_fill(GdkPixbuf *pb,
                          gint r, gint g, gint b, gint a)
 {
        gboolean has_alpha;
-       gint pw, ph, prs;
+       gint pw;
+       gint ph;
+       gint prs;
        guchar *p_pix;
        guchar *pp;
-       gint i, j;
+       gint i;
+       gint j;
 
        if (!pb) return;
 
@@ -759,9 +792,11 @@ void pixbuf_set_rect_fill(GdkPixbuf *pb,
        prs = gdk_pixbuf_get_rowstride(pb);
        p_pix = gdk_pixbuf_get_pixels(pb);
 
+       const gint p_step = has_alpha ? 4 : 3;
+
        for (i = 0; i < h; i++)
                {
-               pp = p_pix + (y + i) * prs + (x * (has_alpha ? 4 : 3));
+               pp = p_pix + (y + i) * prs + (x * p_step);
                for (j = 0; j < w; j++)
                        {
                        *pp = r; pp++;
@@ -775,15 +810,22 @@ void pixbuf_set_rect_fill(GdkPixbuf *pb,
 void pixbuf_set_rect(GdkPixbuf *pb,
                     gint x, gint y, gint w, gint h,
                     gint r, gint g, gint b, gint a,
-                    gint left, gint right, gint top, gint bottom)
+                    gint left_width, gint right_width, gint top_width, gint bottom_width)
 {
-       pixbuf_set_rect_fill(pb, x + left, y, w - left - right, top,
+       // TODO(xsdg): This function has multiple off-by-one errors.  Would be
+       // much easier to read (and implement correctly) with temporaries to
+       // translate from (x, y, w, h) coordinates to (x1, y1, x2, y2).
+       pixbuf_set_rect_fill(pb,
+                            x + left_width, y, w - left_width - right_width, top_width,
                             r, g, b ,a);
-       pixbuf_set_rect_fill(pb, x + w - right, y, right, h,
+       pixbuf_set_rect_fill(pb,
+                            x + w - right_width, y, right_width, h,
                             r, g, b ,a);
-       pixbuf_set_rect_fill(pb, x + left, y + h - bottom, w - left - right, bottom,
+       pixbuf_set_rect_fill(pb,
+                            x + left_width, y + h - bottom_width, w - left_width - right_width, bottom_width,
                             r, g, b ,a);
-       pixbuf_set_rect_fill(pb, x, y, left, h,
+       pixbuf_set_rect_fill(pb,
+                            x, y, left_width, h,
                             r, g, b ,a);
 }
 
@@ -820,18 +862,23 @@ static void pixbuf_copy_font(GdkPixbuf *src, gint sx, gint sy,
                             gint w, gint h,
                             guint8 r, guint8 g, guint8 b, guint8 a)
 {
-       gint sw, sh, srs;
+       gint sw;
+       gint sh;
+       gint srs;
        gboolean s_alpha;
        gint s_step;
        guchar *s_pix;
-       gint dw, dh, drs;
+       gint dw;
+       gint dh;
+       gint drs;
        gboolean d_alpha;
        gint d_step;
        guchar *d_pix;
 
        guchar *sp;
        guchar *dp;
-       gint i, j;
+       gint i;
+       gint j;
 
        if (!src || !dest) return;
 
@@ -898,9 +945,12 @@ void pixbuf_draw_layout(GdkPixbuf *pixbuf, PangoLayout *layout, GtkWidget *,
                        guint8 r, guint8 g, guint8 b, guint8 a)
 {
        GdkPixbuf *buffer;
-       gint w, h;
-       gint sx, sy;
-       gint dw, dh;
+       gint w;
+       gint h;
+       gint sx;
+       gint sy;
+       gint dw;
+       gint dh;
        cairo_surface_t *source;
        cairo_t *cr;
 
@@ -964,23 +1014,12 @@ void pixbuf_draw_layout(GdkPixbuf *pixbuf, PangoLayout *layout, GtkWidget *,
  */
 
 void util_clip_triangle(gint x1, gint y1, gint x2, gint y2, gint x3, gint y3,
-                       gint *rx, gint *ry, gint *rw, gint *rh)
+                        gint &rx, gint &ry, gint &rw, gint &rh)
 {
-       gint tx, ty, tw, th;
-
-       tx = MIN(x1, x2);
-       tx = MIN(tx, x3);
-       ty = MIN(y1, y2);
-       ty = MIN(ty, y3);
-       tw = MAX(abs(x1 - x2), abs(x2 - x3));
-       tw = MAX(tw, abs(x3 - x1));
-       th = MAX(abs(y1 - y2), abs(y2 - y3));
-       th = MAX(th, abs(y3 - y1));
-
-       *rx = tx;
-       *ry = ty;
-       *rw = tw;
-       *rh = th;
+       rx = std::min({x1, x2, x3});
+       ry = std::min({y1, y2, y3});
+       rw = std::max({abs(x1 - x2), abs(x2 - x3), abs(x3 - x1)});
+       rh = std::max({abs(y1 - y2), abs(y2 - y3), abs(y3 - y1)});
 }
 
 void pixbuf_draw_triangle(GdkPixbuf *pb,
@@ -989,19 +1028,31 @@ void pixbuf_draw_triangle(GdkPixbuf *pb,
                          guint8 r, guint8 g, guint8 b, guint8 a)
 {
        gboolean has_alpha;
-       gint pw, ph, prs;
-       gint rx, ry, rw, rh;
-       gint tx, ty, tw, th;
-       gint fx1, fy1;
-       gint fx2, fy2;
-       gint fw, fh;
+       gint pw;
+       gint ph;
+       gint prs;
+       gint rx;
+       gint ry;
+       gint rw;
+       gint rh;
+       gint tx;
+       gint ty;
+       gint tw;
+       gint th;
+       gint fx1;
+       gint fy1;
+       gint fx2;
+       gint fy2;
+       gint fw;
+       gint fh;
        guchar *p_pix;
        guchar *pp;
        gint p_step;
-       gdouble slope1, slope2;
-       gint slope1_x, slope1_y;
+       gdouble slope1;
+       gdouble slope2;
+       gint slope1_x;
+       gint slope1_y;
        gint y;
-       gint t;
        gboolean middle = FALSE;
 
        if (!pb) return;
@@ -1009,16 +1060,20 @@ void pixbuf_draw_triangle(GdkPixbuf *pb,
        pw = gdk_pixbuf_get_width(pb);
        ph = gdk_pixbuf_get_height(pb);
 
+       // Intersects the clip region with the pixbuf. r{x,y,w,h} is that
+       // intersecting region.
        if (!util_clip_region(0, 0, pw, ph,
-                             clip_x, clip_y, clip_w, clip_h,
-                             &rx, &ry, &rw, &rh)) return;
+                             clip_x, clip_y, clip_w, clip_h,
+                             rx, ry, rw, rh)) return;
 
+       // Determine the bounding box for the triangle.
        util_clip_triangle(x1, y1, x2, y2, x3, y3,
-                          &tx, &ty, &tw, &th);
+                          tx, ty, tw, th);
 
+       // And now clip the triangle bounding box to the pixbuf clipping region.
        if (!util_clip_region(rx, ry, rw, rh,
-                             tx, ty, tw, th,
-                             &fx1, &fy1, &fw, &fh)) return;
+                             tx, ty, tw, th,
+                             fx1, fy1, fw, fh)) return;
        fx2 = fx1 + fw;
        fy2 = fy1 + fh;
 
@@ -1028,22 +1083,25 @@ void pixbuf_draw_triangle(GdkPixbuf *pb,
 
        p_step = (has_alpha) ? 4 : 3;
 
+       // Ensure that points are ordered by increasing y coordinate.
        if (y1 > y2)
                {
-               t = x1; x1 = x2; x2 = t;
-               t = y1; y1 = y2; y2 = t;
+               std::swap(x1, x2);
+               std::swap(y1, y2);
                }
        if (y2 > y3)
                {
-               t = x2; x2 = x3; x3 = t;
-               t = y2; y2 = y3; y3 = t;
+               std::swap(x2, x3);
+               std::swap(y2, y3);
                }
        if (y1 > y2)
                {
-               t = x1; x1 = x2; x2 = t;
-               t = y1; y1 = y2; y2 = t;
+               std::swap(x1, x2);
+               std::swap(y1, y2);
                }
 
+       // TODO(xsdg): Drop these explicit casts.  Int will always promote to
+       // double without issue.
        slope1 = static_cast<gdouble>(y2 - y1);
        if (slope1) slope1 = static_cast<gdouble>(x2 - x1) / slope1;
        slope1_x = x1;
@@ -1053,7 +1111,8 @@ void pixbuf_draw_triangle(GdkPixbuf *pb,
 
        for (y = fy1; y < fy2; y++)
                {
-               gint xa, xb;
+               gint xa;
+               gint xb;
 
                if (!middle && y > y2)
                        {
@@ -1070,7 +1129,7 @@ void pixbuf_draw_triangle(GdkPixbuf *pb,
 
                if (xa > xb)
                        {
-                       t = xa; xa = xb; xb = t;
+                       std::swap(xa, xb);
                        }
 
                xa = CLAMP(xa, fx1, fx2);
@@ -1080,13 +1139,10 @@ void pixbuf_draw_triangle(GdkPixbuf *pb,
 
                while (xa < xb)
                        {
-                       *pp = (r * a + *pp * (256-a)) >> 8;
-                       pp++;
-                       *pp = (g * a + *pp * (256-a)) >> 8;
-                       pp++;
-                       *pp = (b * a + *pp * (256-a)) >> 8;
-                       pp++;
-                       if (has_alpha) pp++;
+                       pp[0] = (r * a + pp[0] * (256-a)) >> 8;
+                       pp[1] = (g * a + pp[1] * (256-a)) >> 8;
+                       pp[2] = (b * a + pp[2] * (256-a)) >> 8;
+                       pp += p_step;
 
                        xa++;
                        }
@@ -1099,6 +1155,17 @@ void pixbuf_draw_triangle(GdkPixbuf *pb,
  *-----------------------------------------------------------------------------
  */
 
+/**
+ * @brief Clips the specified line segment to the specified clipping region.
+ * @param[in] clip_x,clip_y Coordinates of the top-left corner of the clipping region.
+ * @param[in] clip_w,clip_h Extent of the clipping region.
+ * @param[in] x1,y1 Coordinates of the first point of the line segment.
+ * @param[in] x2,y2 Coordinates of the second point of the line segment.
+ * @param[out] rx1,ry1 Computed coordinates of the first point of the clipped line segment.
+ * @param[out] rx2,ry2 Computed coordinates of the second point of the clipped line segment.
+ * @retval FALSE The line segment lies outside of the clipping region.
+ * @retval TRUE The clip operation was performed, and the output params were set.
+ */
 static gboolean util_clip_line(gdouble clip_x, gdouble clip_y, gdouble clip_w, gdouble clip_h,
                               gdouble x1, gdouble y1, gdouble x2, gdouble y2,
                               gdouble *rx1, gdouble *ry1, gdouble *rx2, gdouble *ry2)
@@ -1106,17 +1173,21 @@ static gboolean util_clip_line(gdouble clip_x, gdouble clip_y, gdouble clip_w, g
        gboolean flip = FALSE;
        gdouble d;
 
+       // Normalize: Line endpoint 1 must be farther left.
        if (x1 > x2)
                {
-               gdouble t;
-
-               t = x1; x1 = x2; x2 = t;
-               t = y1; y1 = y2; y2 = t;
+               std::swap(x1, x2);
+               std::swap(y1, y2);
                flip = TRUE;
                }
 
+       // Ensure the line horizontally overlaps with the clip region.
        if (x2 < clip_x || x1 > clip_x + clip_w) return FALSE;
 
+       // Ensure the line vertically overlaps with the clip region.
+       // Note that a line can both horizontally and vertically overlap with
+       // clipping region, while still being outside of the clipping region.  That
+       // case is detected further below.
        if (y1 < y2)
                {
                if (y2 < clip_y || y1 > clip_y + clip_h) return FALSE;
@@ -1127,16 +1198,21 @@ static gboolean util_clip_line(gdouble clip_x, gdouble clip_y, gdouble clip_w, g
                }
 
        d = x2 - x1;
+       // TODO(xsdg): Either use ints here, or define a reasonable epsilon to do the
+       // right thing if -epsilon < d < 0.  We already guaranteed above that x2 >= x1.
        if (d > 0.0)
                {
                gdouble slope;
 
                slope = (y2 - y1) / d;
+               // If needed, project (x1, y1) to be horizontally within the clip
+               // region, while maintaining the line's slope and y-offset.
                if (x1 < clip_x)
                        {
                        y1 = y1 + slope * (clip_x - x1);
                        x1 = clip_x;
                        }
+               // Likewise with (x2, y2).
                if (x2 > clip_x + clip_w)
                        {
                        y2 = y2 + slope * (clip_x + clip_w - x2);
@@ -1144,18 +1220,19 @@ static gboolean util_clip_line(gdouble clip_x, gdouble clip_y, gdouble clip_w, g
                        }
                }
 
+       // Check that any horizontal projections didn't cause the line segment to
+       // no longer vertically overlap with the clip region.
        if (y1 < y2)
                {
                if (y2 < clip_y || y1 > clip_y + clip_h) return FALSE;
                }
        else
                {
-               gdouble t;
-
                if (y1 < clip_y || y2 > clip_y + clip_h) return FALSE;
 
-               t = x1; x1 = x2; x2 = t;
-               t = y1; y1 = y2; y2 = t;
+               // Re-normalize: line endpoint 1 must be farther up.
+               std::swap(x1, x2);
+               std::swap(y1, y2);
                flip = !flip;
                }
 
@@ -1165,11 +1242,14 @@ static gboolean util_clip_line(gdouble clip_x, gdouble clip_y, gdouble clip_w, g
                gdouble slope;
 
                slope = (x2 - x1) / d;
+               // If needed, project (x1, y1) to be vertically within the clip
+               // region, while maintaining the line's slope and x-offset.
                if (y1 < clip_y)
                        {
                        x1 = x1 + slope * (clip_y - y1);
                        y1 = clip_y;
                        }
+               // Likewise with (x2, y2).
                if (y2 > clip_y + clip_h)
                        {
                        x2 = x2 + slope * (clip_y + clip_h - y2);
@@ -1177,6 +1257,8 @@ static gboolean util_clip_line(gdouble clip_x, gdouble clip_y, gdouble clip_w, g
                        }
                }
 
+       // Set the output params, accounting for any flips that might have
+       // happened during normalization.
        if (flip)
                {
                *rx1 = x2;
@@ -1201,25 +1283,42 @@ void pixbuf_draw_line(GdkPixbuf *pb,
                      guint8 r, guint8 g, guint8 b, guint8 a)
 {
        gboolean has_alpha;
-       gint pw, ph, prs;
-       gint rx, ry, rw, rh;
-       gdouble rx1, ry1, rx2, ry2;
+       gint pw;
+       gint ph;
+       gint prs;
+       gint rx;
+       gint ry;
+       gint rw;
+       gint rh;
+       gdouble rx1;
+       gdouble ry1;
+       gdouble rx2;
+       gdouble ry2;
        guchar *p_pix;
        guchar *pp;
        gint p_step;
        gdouble slope;
-       gdouble x, y;
-       gint px, py;
-       gint cx1, cy1, cx2, cy2;
+       gdouble x;
+       gdouble y;
+       gint px;
+       gint py;
+       gint cx1;
+       gint cy1;
+       gint cx2;
+       gint cy2;
 
        if (!pb) return;
 
        pw = gdk_pixbuf_get_width(pb);
        ph = gdk_pixbuf_get_height(pb);
 
+       // Intersects the clip region with the pixbuf. r{x,y,w,h} is that
+       // intersecting region.
        if (!util_clip_region(0, 0, pw, ph,
-                             clip_x, clip_y, clip_w, clip_h,
-                             &rx, &ry, &rw, &rh)) return;
+                             clip_x, clip_y, clip_w, clip_h,
+                             rx, ry, rw, rh)) return;
+       // TODO(xsdg): These explicit casts are unnecessary and harm readability.
+       // Clips the specified line segment to the intersecting region from above.
        if (!util_clip_line(static_cast<gdouble>(rx), static_cast<gdouble>(ry), static_cast<gdouble>(rw), static_cast<gdouble>(rh),
                            static_cast<gdouble>(x1), static_cast<gdouble>(y1), static_cast<gdouble>(x2), static_cast<gdouble>(y2),
                            &rx1, &ry1, &rx2, &ry2)) return;
@@ -1235,13 +1334,16 @@ void pixbuf_draw_line(GdkPixbuf *pb,
 
        p_step = (has_alpha) ? 4 : 3;
 
+       // We draw the clipped line segment along the longer axis first, and
+       // allow the shorter axis to follow.  This is because our raster line segment
+       // will contain max(rx2-rx1, ry2-ry1) pixels, and the pixels along the
+       // shorter axis may not advance for each cycle (the line is not anti-aliased).
        if (fabs(rx2 - rx1) > fabs(ry2 - ry1))
                {
                if (rx1 > rx2)
                        {
-                       gdouble t;
-                       t = rx1; rx1 = rx2; rx2 = t;
-                       t = ry1; ry1 = ry2; ry2 = t;
+                       std::swap(rx1, rx2);
+                       std::swap(ry1, ry2);
                        }
 
                slope = rx2 - rx1;
@@ -1266,9 +1368,8 @@ void pixbuf_draw_line(GdkPixbuf *pb,
                {
                if (ry1 > ry2)
                        {
-                       gdouble t;
-                       t = rx1; rx1 = rx2; rx2 = t;
-                       t = ry1; ry1 = ry2; ry2 = t;
+                       std::swap(rx1, rx2);
+                       std::swap(ry1, ry2);
                        }
 
                slope = ry2 - ry1;
@@ -1297,6 +1398,27 @@ void pixbuf_draw_line(GdkPixbuf *pb,
  *-----------------------------------------------------------------------------
  */
 
+/**
+ * @brief Composites a horizontal or vertical linear gradient into the rectangular
+ *        region defined by corners `(x1, y1)` and `(x2, y2)`.  Note that the
+ *        current implementation breaks if the max distance between `s` and
+ *        `x1/x2/y1/y2` is greater than `border`.
+ * @param p_pix The pixel buffer to paint into.
+ * @param prs The pixel row stride (how many pixels per row of the buffer).
+ * @param has_alpha TRUE if the p_pix representation is rgba.  FALSE if just rgb.
+ * @param s The "center" of the gradient, along the axis defined by `vertical`.
+ *          Note that if the center is not along an edge, the gradient will be
+ *          symmetric about the center.
+ * @param vertical When `TRUE`, the gradient color will vary vertically.  When `FALSE`,
+ *                 horizontally.
+ * @param border The maximum extent of the gradient, in pixels.
+ * @param x1,y1 Coordinates of the first corner of the region.
+ * @param x2,y2 Coordinates of the second corner of the region.
+ * @param r,g,b Base color of the gradient.
+ * @param a The peak alpha value when compositing the gradient.  The alpha varies
+ *          from this value down to 0 (fully transparent).  Note that any alpha
+ *          value associated with the original pixel is unmodified.
+ */
 static void pixbuf_draw_fade_linear(guchar *p_pix, gint prs, gboolean has_alpha,
                                    gint s, gboolean vertical, gint border,
                                    gint x1, gint y1, gint x2, gint y2,
@@ -1305,7 +1427,8 @@ static void pixbuf_draw_fade_linear(guchar *p_pix, gint prs, gboolean has_alpha,
        guchar *pp;
        gint p_step;
        guint8 n = a;
-       gint i, j;
+       gint i;
+       gint j;
 
        p_step = (has_alpha) ? 4 : 3;
        for (j = y1; j < y2; j++)
@@ -1315,25 +1438,42 @@ static void pixbuf_draw_fade_linear(guchar *p_pix, gint prs, gboolean has_alpha,
                for (i = x1; i < x2; i++)
                        {
                        if (vertical) n = a - a * abs(i - s) / border;
-                       *pp = (r * n + *pp * (256-n)) >> 8;
-                       pp++;
-                       *pp = (g * n + *pp * (256-n)) >> 8;
-                       pp++;
-                       *pp = (b * n + *pp * (256-n)) >> 8;
-                       pp++;
-                       if (has_alpha) pp++;
+                       pp[0] = (r * n + pp[0] * (256-n)) >> 8;
+                       pp[1] = (g * n + pp[1] * (256-n)) >> 8;
+                       pp[2] = (b * n + pp[2] * (256-n)) >> 8;
+                       pp += p_step;
                        }
                }
 }
 
+/**
+ * @brief Composites a radial gradient into the rectangular region defined by
+ *        corners `(x1, y1)` and `(x2, y2)`.
+ * @param p_pix The pixel buffer to paint into.
+ * @param prs The pixel row stride (how many pixels per row of the buffer).
+ * @param has_alpha TRUE if the p_pix representation is rgba.  FALSE if just rgb.
+ * @param sx,sy The coordinates of the center of the gradient.
+ * @param border The max radius, in pixels, of the gradient.  Pixels farther away
+ *               from the center than this will be unaffected.
+ * @param x1,y1 Coordinates of the first corner of the region.
+ * @param x2,y2 Coordinates of the second corner of the region.
+ * @param r,g,b Base color of the gradient.
+ * @param a The peak alpha value when compositing the gradient.  The alpha varies
+ *          from this value down to 0 (fully transparent).  Note that any alpha
+ *          value associated with the original pixel is unmodified.
+ */
 static void pixbuf_draw_fade_radius(guchar *p_pix, gint prs, gboolean has_alpha,
                                    gint sx, gint sy, gint border,
                                    gint x1, gint y1, gint x2, gint y2,
                                    guint8, guint8 g, guint8 b, guint8 a)
 {
+       // TODO(xsdg): r (red) was shadowed by r (radius), and was removed from
+       // the params list by an automated cleanup.  Fix this and distinguish the
+       // red param from the radius temporary variable.
        guchar *pp;
        gint p_step;
-       gint i, j;
+       gint i;
+       gint j;
 
        p_step = (has_alpha) ? 4 : 3;
        for (j = y1; j < y2; j++)
@@ -1346,13 +1486,10 @@ static void pixbuf_draw_fade_radius(guchar *p_pix, gint prs, gboolean has_alpha,
 
                        r = MIN(border, (gint)hypot(i - sx, j - sy));
                        n = a - a * r / border;
-                       *pp = (r * n + *pp * (256-n)) >> 8;
-                       pp++;
-                       *pp = (g * n + *pp * (256-n)) >> 8;
-                       pp++;
-                       *pp = (b * n + *pp * (256-n)) >> 8;
-                       pp++;
-                       if (has_alpha) pp++;
+                       pp[0] = (r * n + pp[0] * (256-n)) >> 8;
+                       pp[1] = (g * n + pp[1] * (256-n)) >> 8;
+                       pp[2] = (b * n + pp[2] * (256-n)) >> 8;
+                       pp += p_step;
                        }
                }
 }
@@ -1363,9 +1500,17 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
                        guint8 r, guint8 g, guint8 b, guint8 a)
 {
        gint has_alpha;
-       gint pw, ph, prs;
-       gint rx, ry, rw, rh;
-       gint fx, fy, fw, fh;
+       gint pw;
+       gint ph;
+       gint prs;
+       gint rx;
+       gint ry;
+       gint rw;
+       gint rh;
+       gint fx;
+       gint fy;
+       gint fw;
+       gint fh;
        guchar *p_pix;
 
        if (!pb) return;
@@ -1373,26 +1518,32 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
        pw = gdk_pixbuf_get_width(pb);
        ph = gdk_pixbuf_get_height(pb);
 
+       // Intersects the clip region with the pixbuf. r{x,y,w,h} is that
+       // intersecting region.
        if (!util_clip_region(0, 0, pw, ph,
-                             clip_x, clip_y, clip_w, clip_h,
-                             &rx, &ry, &rw, &rh)) return;
+                             clip_x, clip_y, clip_w, clip_h,
+                             rx, ry, rw, rh)) return;
 
        has_alpha = gdk_pixbuf_get_has_alpha(pb);
        prs = gdk_pixbuf_get_rowstride(pb);
        p_pix = gdk_pixbuf_get_pixels(pb);
 
+       // Composites the specified color into the rectangle specified by x, y, w, h,
+       // as contracted by `border` pixels, with a composition fraction that's defined
+       // by the supplied `a` parameter.
        if (util_clip_region(x + border, y + border, w - border * 2, h - border * 2,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_rect_fill(pb, fx, fy, fw, fh, r, g, b, a);
                }
 
        if (border < 1) return;
 
+       // Draws linear gradients along each of the 4 edges.
        if (util_clip_region(x, y + border, border, h - border * 2,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_linear(p_pix, prs, has_alpha,
                                        x + border, TRUE, border,
@@ -1400,8 +1551,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
                                        r, g, b, a);
                }
        if (util_clip_region(x + w - border, y + border, border, h - border * 2,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_linear(p_pix, prs, has_alpha,
                                        x + w - border, TRUE, border,
@@ -1409,8 +1560,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
                                        r, g, b, a);
                }
        if (util_clip_region(x + border, y, w - border * 2, border,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_linear(p_pix, prs, has_alpha,
                                        y + border, FALSE, border,
@@ -1418,17 +1569,18 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
                                        r, g, b, a);
                }
        if (util_clip_region(x + border, y + h - border, w - border * 2, border,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_linear(p_pix, prs, has_alpha,
                                        y + h - border, FALSE, border,
                                        fx, fy, fx + fw, fy + fh,
                                        r, g, b, a);
                }
+       // Draws radial gradients at each of the 4 corners.
        if (util_clip_region(x, y, border, border,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_radius(p_pix, prs, has_alpha,
                                        x + border, y + border, border,
@@ -1436,8 +1588,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
                                        r, g, b, a);
                }
        if (util_clip_region(x + w - border, y, border, border,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_radius(p_pix, prs, has_alpha,
                                        x + w - border, y + border, border,
@@ -1445,8 +1597,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
                                        r, g, b, a);
                }
        if (util_clip_region(x, y + h - border, border, border,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_radius(p_pix, prs, has_alpha,
                                        x + border, y + h - border, border,
@@ -1454,8 +1606,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb,
                                        r, g, b, a);
                }
        if (util_clip_region(x + w - border, y + h - border, border, border,
-                            rx, ry, rw, rh,
-                            &fx, &fy, &fw, &fh))
+                            rx, ry, rw, rh,
+                            fx, fy, fw, fh))
                {
                pixbuf_draw_fade_radius(p_pix, prs, has_alpha,
                                        x + w - border, y + h - border, border,
@@ -1475,10 +1627,13 @@ void pixbuf_desaturate_rect(GdkPixbuf *pb,
                            gint x, gint y, gint w, gint h)
 {
        gboolean has_alpha;
-       gint pw, ph, prs;
+       gint pw;
+       gint ph;
+       gint prs;
        guchar *p_pix;
        guchar *pp;
-       gint i, j;
+       gint i;
+       gint j;
 
        if (!pb) return;
 
@@ -1492,36 +1647,39 @@ void pixbuf_desaturate_rect(GdkPixbuf *pb,
        prs = gdk_pixbuf_get_rowstride(pb);
        p_pix = gdk_pixbuf_get_pixels(pb);
 
+       const gint p_step = has_alpha ? 4 : 3;
+
        for (i = 0; i < h; i++)
                {
-               pp = p_pix + (y + i) * prs + (x * (has_alpha ? 4 : 3));
+               pp = p_pix + (y + i) * prs + (x * p_step);
                for (j = 0; j < w; j++)
                        {
                        guint8 grey;
 
                        grey = (pp[0] + pp[1] + pp[2]) / 3;
-                       *pp = grey;
-                       pp++;
-                       *pp = grey;
-                       pp++;
-                       *pp = grey;
-                       pp++;
-                       if (has_alpha) pp++;
+                       pp[0] = grey;
+                       pp[1] = grey;
+                       pp[2] = grey;
+                       pp += p_step;
                        }
                }
 }
 
 /*
  *-----------------------------------------------------------------------------
- * pixbuf highlight under/over exposure *-----------------------------------------------------------------------------
+ * pixbuf highlight under/over exposure
+ *-----------------------------------------------------------------------------
  */
 void pixbuf_highlight_overunderexposed(GdkPixbuf *pb, gint x, gint y, gint w, gint h)
 {
        gboolean has_alpha;
-       gint pw, ph, prs;
+       gint pw;
+       gint ph;
+       gint prs;
        guchar *p_pix;
        guchar *pp;
-       gint i, j;
+       gint i;
+       gint j;
 
        if (!pb) return;
 
@@ -1535,26 +1693,20 @@ void pixbuf_highlight_overunderexposed(GdkPixbuf *pb, gint x, gint y, gint w, gi
        prs = gdk_pixbuf_get_rowstride(pb);
        p_pix = gdk_pixbuf_get_pixels(pb);
 
+       const gint p_step = has_alpha ? 4 : 3;
+
        for (i = 0; i < h; i++)
                {
-               pp = p_pix + (y + i) * prs + (x * (has_alpha ? 4 : 3));
+               pp = p_pix + (y + i) * prs + (x * p_step);
                for (j = 0; j < w; j++)
                        {
                        if (pp[0] == 255 || pp[1] == 255 || pp[2] == 255 || pp[0] == 0 || pp[1] == 0 || pp[2] == 0)
                                {
-                               *pp = 255;
-                               pp++;
-                               *pp = 0;
-                               pp++;
-                               *pp = 0;
-                               pp++;
-                               if (has_alpha) pp++;
-                               }
-                       else
-                               {
-                               pp = pp + 3;
-                               if (has_alpha) pp++;
+                               pp[0] = 255;
+                               pp[1] = 0;
+                               pp[2] = 0;
                                }
+                       pp += p_step;
                        }
                }
 }
@@ -1565,13 +1717,16 @@ void pixbuf_highlight_overunderexposed(GdkPixbuf *pb, gint x, gint y, gint w, gi
  *-----------------------------------------------------------------------------
 */
 void pixbuf_ignore_alpha_rect(GdkPixbuf *pb,
-                 gint x, gint y, gint w, gint h)
+                             gint x, gint y, gint w, gint h)
 {
    gboolean has_alpha;
-   gint pw, ph, prs;
+   gint pw;
+   gint ph;
+   gint prs;
    guchar *p_pix;
    guchar *pp;
-   gint i, j;
+   gint i;
+   gint j;
 
    if (!pb) return;