Iron out some rounding errors in the displaying of the rulers and of guides

(bzr r4662)

5 files changed, 43 insertions, 20 deletions

diff --git a/src/display/guideline.cpp b/src/display/guideline.cpp
index 51b2ec38d..99dfcc316 100644
--- a/src/display/guideline.cpp
+++ b/src/display/guideline.cpp
@@ -94,7 +94,7 @@ static void sp_guideline_render(SPCanvasItem *item, SPCanvasBuf *buf)
     unsigned int const a = NR_RGBA32_A (gl->rgba);
 
     if (gl->is_vertical()) {
-        int position = gl->point_on_line[Geom::X];
+        int position = (int) Inkscape::round(gl->point_on_line[Geom::X]);
         if (position < buf->rect.x0 || position >= buf->rect.x1) {
             return;
         }
@@ -111,7 +111,7 @@ static void sp_guideline_render(SPCanvasItem *item, SPCanvasBuf *buf)
             d += step;
         }
     } else if (gl->is_horizontal()) {
-        int position = gl->point_on_line[Geom::Y];
+        int position = (int) Inkscape::round(gl->point_on_line[Geom::Y]);
         if (position < buf->rect.y0 || position >= buf->rect.y1) {
             return;
         }
@@ -176,13 +176,13 @@ static void sp_guideline_update(SPCanvasItem *item, NR::Matrix const &affine, un
         ((SPCanvasItemClass *) parent_class)->update(item, affine, flags);
     }
 
-    gl->point_on_line[Geom::X] = affine[4] +0.5;
-    gl->point_on_line[Geom::Y] = affine[5] -0.5;
+    gl->point_on_line[Geom::X] = affine[4];
+    gl->point_on_line[Geom::Y] = affine[5];
 
     if (gl->is_horizontal()) {
-        sp_canvas_update_bbox (item, -1000000, gl->point_on_line[Geom::Y], 1000000, gl->point_on_line[Geom::Y] + 1);
+        sp_canvas_update_bbox (item, -1000000, (int) Inkscape::round(gl->point_on_line[Geom::Y]), 1000000, (int) Inkscape::round(gl->point_on_line[Geom::Y] + 1));
     } else if (gl->is_vertical()) {
-        sp_canvas_update_bbox (item, gl->point_on_line[Geom::X], -1000000, gl->point_on_line[Geom::X]+1, 1000000);
+        sp_canvas_update_bbox (item, (int) Inkscape::round(gl->point_on_line[Geom::X]), -1000000, (int) Inkscape::round(gl->point_on_line[Geom::X] + 1), 1000000);
     } else {
         sp_canvas_update_bbox (item, -1000000, -1000000, 1000000, 1000000);
     }
diff --git a/src/display/sp-canvas.cpp b/src/display/sp-canvas.cpp
index cce43825a..88a457490 100644
--- a/src/display/sp-canvas.cpp
+++ b/src/display/sp-canvas.cpp
@@ -2253,11 +2253,20 @@ bool sp_canvas_world_pt_inside_window(SPCanvas const *canvas, NR::Point const &w
 NR::Rect SPCanvas::getViewbox() const
 {
     GtkWidget const *w = GTK_WIDGET(this);
-
     return NR::Rect(NR::Point(dx0, dy0),
                     NR::Point(dx0 + w->allocation.width, dy0 + w->allocation.height));
 }
 
+/**
+ * Return canvas window coordinates as IRect (a rectangle defined by integers).
+ */
+NR::IRect SPCanvas::getViewboxIntegers() const
+{
+    GtkWidget const *w = GTK_WIDGET(this);
+    return NR::IRect(NR::IPoint(x0, y0),
+                    NR::IPoint(x0 + w->allocation.width, y0 + w->allocation.height));
+}
+
 inline int sp_canvas_tile_floor(int x)
 {
     return (x & (~(TILE_SIZE - 1))) / TILE_SIZE;
diff --git a/src/display/sp-canvas.h b/src/display/sp-canvas.h
index d8cc3157d..8abb0a359 100644
--- a/src/display/sp-canvas.h
+++ b/src/display/sp-canvas.h
@@ -190,6 +190,7 @@ struct SPCanvas {
     bool is_scrolling;
 
     NR::Rect getViewbox() const;
+    NR::IRect getViewboxIntegers() const;
 };
 
 GtkWidget *sp_canvas_new_aa();
diff --git a/src/widgets/desktop-widget.cpp b/src/widgets/desktop-widget.cpp
index f4d87f084..bd329e336 100644
--- a/src/widgets/desktop-widget.cpp
+++ b/src/widgets/desktop-widget.cpp
@@ -1366,7 +1366,12 @@ sp_desktop_widget_update_rulers (SPDesktopWidget *dtw)
 void
 sp_desktop_widget_update_hruler (SPDesktopWidget *dtw)
 {
-    NR::Rect viewbox = dtw->canvas->getViewbox();
+    /* The viewbox (in integers) must exactly match the size of SPCanvasbuf's pixel buffer.
+     * This is important because the former is being used for drawing the ruler, whereas
+     * the latter is used for drawing e.g. the grids and guides. Only when the viewbox 
+     * coincides with the pixel buffer, everything will line up nicely.
+     */  
+    NR::IRect viewbox = dtw->canvas->getViewboxIntegers();
     
     double const scale = dtw->desktop->current_zoom();
     double s = viewbox.min()[NR::X] / scale - dtw->ruler_origin[NR::X];
@@ -1377,7 +1382,12 @@ sp_desktop_widget_update_hruler (SPDesktopWidget *dtw)
 void
 sp_desktop_widget_update_vruler (SPDesktopWidget *dtw)
 {
-    NR::Rect viewbox = dtw->canvas->getViewbox();
+    /* The viewbox (in integers) must exactly match the size of SPCanvasbuf's pixel buffer.
+     * This is important because the former is being used for drawing the ruler, whereas
+     * the latter is used for drawing e.g. the grids and guides. Only when the viewbox 
+     * coincides with the pixel buffer, everything will line up nicely.
+     */
+    NR::IRect viewbox = dtw->canvas->getViewboxIntegers();
     
     double const scale = dtw->desktop->current_zoom();
     double s = viewbox.min()[NR::Y] / -scale - dtw->ruler_origin[NR::Y];
diff --git a/src/widgets/ruler.cpp b/src/widgets/ruler.cpp
index b0037578d..255bdd853 100644
--- a/src/widgets/ruler.cpp
+++ b/src/widgets/ruler.cpp
@@ -178,12 +178,14 @@ sp_hruler_draw_ticks (GtkRuler *ruler)
 		 0, 0, 
 		 widget->allocation.width, widget->allocation.height);
 
-  upper = ruler->upper / ruler->metric->pixels_per_unit;
+  upper = ruler->upper / ruler->metric->pixels_per_unit; // upper and lower are expressed in ruler units
   lower = ruler->lower / ruler->metric->pixels_per_unit;
-
+  /* "pixels_per_unit" should be "points_per_unit". This is the size of the unit
+   * in 1/72nd's of an inch and has nothing to do with screen pixels */
+  
   if ((upper - lower) == 0) 
     return;
-  increment = (double) (width + 2*UNUSED_PIXELS) / (upper - lower);
+  increment = (double) (width + 2*UNUSED_PIXELS) / (upper - lower); // screen pixels per ruler unit
   
    /* determine the scale
    *  We calculate the text size as for the vruler instead of using
@@ -229,17 +231,16 @@ sp_hruler_draw_ticks (GtkRuler *ruler)
 	}
 
     tick_index = 0;
-    cur = start;
+    cur = start; // location (in ruler units) of the first invisible tick at the left side of the canvas 
 
 	while (cur <= end)
 	{
 	  // due to the typical values for cur, lower and increment, pos will often end up to
       // be e.g. 641.50000000000; rounding behaviour is not defined in such a case (see round.h)
       // and jitter will be apparent (upon redrawing some of the lines on the ruler might jump a
-      // by a pixel, and jump back on the next redraw). This is suppressed by adding 1e-12
+      // by a pixel, and jump back on the next redraw). This is suppressed by adding 1e-9 (that's only one nanopixel ;-))
       pos = int(Inkscape::round((cur - lower) * increment + 1e-12)) - UNUSED_PIXELS; 
-      
-	  gdk_draw_line (ruler->backing_store, gc,
+      gdk_draw_line (ruler->backing_store, gc,
 			 pos, height + ythickness, 
 			 pos, height - length + ythickness);
 
@@ -515,12 +516,14 @@ sp_vruler_draw_ticks (GtkRuler *ruler)
 		 0, 0, 
 		 widget->allocation.width, widget->allocation.height);
   
-  upper = ruler->upper / ruler->metric->pixels_per_unit;
+  upper = ruler->upper / ruler->metric->pixels_per_unit; // upper and lower are expressed in ruler units
   lower = ruler->lower / ruler->metric->pixels_per_unit;
+  /* "pixels_per_unit" should be "points_per_unit". This is the size of the unit
+   * in 1/72nd's of an inch and has nothing to do with screen pixels */
   
   if ((upper - lower) == 0)
     return;
-  increment = (double) (width + 2*UNUSED_PIXELS) / (upper - lower);
+  increment = (double) (width + 2*UNUSED_PIXELS) / (upper - lower); // screen pixels per ruler unit
 
   /* determine the scale
    *   use the maximum extents of the ruler to determine the largest
@@ -566,13 +569,13 @@ sp_vruler_draw_ticks (GtkRuler *ruler)
 			}
 
     tick_index = 0;
-    cur = start;        
+    cur = start; // location (in ruler units) of the first invisible tick at the top side of the canvas       
 
     while (cur < end) {
           // due to the typical values for cur, lower and increment, pos will often end up to
           // be e.g. 641.50000000000; rounding behaviour is not defined in such a case (see round.h)
           // and jitter will be apparent (upon redrawing some of the lines on the ruler might jump a
-          // by a pixel, and jump back on the next redraw). This is suppressed by adding 1e-12
+          // by a pixel, and jump back on the next redraw). This is suppressed by adding 1e-9 (that's only one nanopixel ;-))
             pos = int(Inkscape::round((cur - lower) * increment + 1e-12)) - UNUSED_PIXELS;
 
 			gdk_draw_line (ruler->backing_store, gc,