Merge branch 'refactoring' of gitlab.com:shlomif/inkscape

23 files changed, 284 insertions, 295 deletions

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7437f12ae..4b3357d30 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -14,6 +14,7 @@ set(sp_SRC
   sp-cursor.cpp
   sp-defs.cpp
   sp-desc.cpp
+  sp-dimensions.cpp
   sp-ellipse.cpp
   sp-factory.cpp
   sp-filter-primitive.cpp
@@ -97,6 +98,7 @@ set(sp_SRC
   sp-cursor.h
   sp-defs.h
   sp-desc.h
+  sp-dimensions.h
   sp-ellipse.h
   sp-factory.h
   sp-filter-primitive.h
diff --git a/src/desktop-style.h b/src/desktop-style.h
index a404c94a8..bc2c56fac 100644
--- a/src/desktop-style.h
+++ b/src/desktop-style.h
@@ -46,7 +46,7 @@ enum { // which property was queried (add when you need more)
     QUERY_STYLE_PROPERTY_PAINTORDER, // paint-order
     QUERY_STYLE_PROPERTY_FONT_SPECIFICATION, //-inkscape-font-specification
     QUERY_STYLE_PROPERTY_FONTFAMILY, // font-family
-    QUERY_STYLE_PROPERTY_FONTSTYLE, // font style 
+    QUERY_STYLE_PROPERTY_FONTSTYLE, // font style
     QUERY_STYLE_PROPERTY_FONTVARIANTS, // font variants (OpenType features)
     QUERY_STYLE_PROPERTY_FONTFEATURESETTINGS, // font feature settings (OpenType features)
     QUERY_STYLE_PROPERTY_FONTNUMBERS, // size, spacings
diff --git a/src/display/canvas-rotate.cpp b/src/display/canvas-rotate.cpp
index aaf6b962c..1d917a677 100644
--- a/src/display/canvas-rotate.cpp
+++ b/src/display/canvas-rotate.cpp
@@ -72,13 +72,13 @@ static void sp_canvas_rotate_update( SPCanvasItem *item, Geom::Affine const &/*a
         // std::cout << "sp_canvas_rotate_update: surface_copy is NULL" << std::endl;
         return;
     }
-    
+
     // Destroy surface_rotated if it already exists.
     if (cr->surface_rotated != NULL) {
         cairo_surface_destroy (cr->surface_rotated);
         cr->surface_rotated = NULL;
     }
-    
+
     // Create rotated surface
     cr->surface_rotated = ink_cairo_surface_create_identical(cr->surface_copy);
     double width  = cairo_image_surface_get_width  (cr->surface_rotated);
@@ -91,7 +91,7 @@ static void sp_canvas_rotate_update( SPCanvasItem *item, Geom::Affine const &/*a
     cairo_set_source_surface( context, cr->surface_copy, 0, 0 );
     cairo_paint(        context );
     cairo_destroy(      context);
-    
+
     // We cover the entire canvas
     item->x1 = -G_MAXINT;
     item->y1 = -G_MAXINT;
@@ -108,8 +108,9 @@ static void sp_canvas_rotate_render( SPCanvasItem *item, SPCanvasBuf *buf)
     // std::cout << "  buf->rect:         " << buf->rect << std::endl;
     // std::cout << "  buf->canvas_rect: " << buf->canvas_rect << std::endl;
     SPCanvasRotate *cr = SP_CANVAS_ROTATE(item);
+    auto ct = buf->ct;
 
-    if (!buf->ct) {
+    if (!ct) {
         return;
     }
 
@@ -119,26 +120,26 @@ static void sp_canvas_rotate_render( SPCanvasItem *item, SPCanvasBuf *buf)
     }
 
     // Draw rotated canvas
-    cairo_save (buf->ct);
-    cairo_translate (buf->ct,
+    cairo_save (ct);
+    cairo_translate (ct,
                      buf->canvas_rect.left() - buf->rect.left(),
                      buf->canvas_rect.top()  - buf->rect.top() );
-    cairo_set_operator (buf->ct, CAIRO_OPERATOR_SOURCE );
-    cairo_set_source_surface (buf->ct, cr->surface_rotated, 0, 0 );
-    cairo_paint   (buf->ct);
-    cairo_restore (buf->ct);
+    cairo_set_operator (ct, CAIRO_OPERATOR_SOURCE );
+    cairo_set_source_surface (ct, cr->surface_rotated, 0, 0 );
+    cairo_paint   (ct);
+    cairo_restore (ct);
 
 
     // Draw line from center to cursor
-    cairo_save (buf->ct);
-    cairo_translate   (buf->ct, -buf->rect.left(), -buf->rect.top());
-    cairo_new_path    (buf->ct);
-    cairo_move_to     (buf->ct, cr->center[Geom::X], cr->center[Geom::Y]);
-    cairo_rel_line_to (buf->ct, cr->cursor[Geom::X], cr->cursor[Geom::Y]);
-    cairo_set_line_width (buf->ct, 2);
-    ink_cairo_set_source_rgba32 (buf->ct, 0xff00007f);
-    cairo_stroke (buf->ct);
-    cairo_restore (buf->ct);
+    cairo_save (ct);
+    cairo_translate   (ct, -buf->rect.left(), -buf->rect.top());
+    cairo_new_path    (ct);
+    cairo_move_to     (ct, cr->center[Geom::X], cr->center[Geom::Y]);
+    cairo_rel_line_to (ct, cr->cursor[Geom::X], cr->cursor[Geom::Y]);
+    cairo_set_line_width (ct, 2);
+    ink_cairo_set_source_rgba32 (ct, 0xff00007f);
+    cairo_stroke (ct);
+    cairo_restore (ct);
 
 }
 
@@ -187,7 +188,7 @@ static int sp_canvas_rotate_event  (SPCanvasItem *item, GdkEvent *event)
 
             // Correct line for snapping of angle
             double distance = rcursor.length();
-            cr->cursor = Geom::Point::polar( Geom::rad_from_deg(angle), distance );        
+            cr->cursor = Geom::Point::polar( Geom::rad_from_deg(angle), distance );
 
             // Update screen
             // sp_canvas_item_request_update( item );
@@ -214,7 +215,7 @@ static int sp_canvas_rotate_event  (SPCanvasItem *item, GdkEvent *event)
                 cr->surface_rotated = NULL;
             }
             // sp_canvas_item_show (desktop->drawing);
-            
+
             break;
         case GDK_KEY_PRESS:
             // std::cout << "  Key press: " << std::endl;
@@ -260,10 +261,10 @@ void sp_canvas_rotate_paint (SPCanvasRotate *canvas_rotate, cairo_surface_t *bac
 
     double width  = cairo_image_surface_get_width  (background);
     double height = cairo_image_surface_get_height (background);
-    
+
     // Draw rotated canvas
     cairo_t *context = cairo_create( background );
-    
+
     cairo_save (context);
     cairo_set_operator( context, CAIRO_OPERATOR_SOURCE );
     cairo_translate(    context, width/2.0, height/2.0 );
diff --git a/src/display/drawing-text.cpp b/src/display/drawing-text.cpp
index 21af7b200..3f6b5b34a 100644
--- a/src/display/drawing-text.cpp
+++ b/src/display/drawing-text.cpp
@@ -91,12 +91,11 @@ unsigned DrawingGlyphs::_updateItem(Geom::IntRect const &/*area*/, UpdateContext
     if (_transform) {
         scale_bigbox /= _transform->descrim();
     }
-    
 
     /* Because there can be text decorations the bounding box must correspond in Y to a little above the glyph's ascend
     and a little below its descend.  This leaves room for overline and underline.  The left and right sides
-    come from the glyph's bounding box.  Note that the initial direction of ascender is positive down in Y, and 
-    this flips after the transform is applied.  So change the sign on descender. 1.1 provides a little extra space 
+    come from the glyph's bounding box.  Note that the initial direction of ascender is positive down in Y, and
+    this flips after the transform is applied.  So change the sign on descender. 1.1 provides a little extra space
     above and below the max/min y positions of the letters to place the text decorations.*/
 
     Geom::Rect b;
@@ -129,10 +128,10 @@ unsigned DrawingGlyphs::_updateItem(Geom::IntRect const &/*area*/, UpdateContext
         Geom::Rect pbigbox(Geom::Point(0.0, _asc*scale_bigbox*0.66),Geom::Point(_width*scale_bigbox, 0.0));
         pb = pbigbox * ctx.ctm;
     }
-    
+
 #if 0
     /* FIXME  if this is commented out then not even an approximation of pick on decorations */
-    /* adjust the pick box up or down to include the decorations.  
+    /* adjust the pick box up or down to include the decorations.
        This is only approximate since at this point we don't know how wide that line is, if it has
        an unusual offset, and so forth.  The selection point is set at what is roughly the center of
        the decoration (vertically) for the wide ones, like wavy and double line.
@@ -146,7 +145,7 @@ unsigned DrawingGlyphs::_updateItem(Geom::IntRect const &/*area*/, UpdateContext
         Geom::Rect padjbox(Geom::Point(0.0, top),Geom::Point(_width*scale_bigbox, bot));
         pb.unionWith(padjbox * ctx.ctm);
     }
-#endif   
+#endif
 
     if (ggroup->_nrstyle.stroke.type != NRStyle::PAINT_NONE) {
         // this expands the selection box for cases where the stroke is "thick"
@@ -263,8 +262,8 @@ DrawingText::_updateItem(Geom::IntRect const &area, UpdateContext const &ctx, un
 void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphase, Geom::Point const &p1, Geom::Point const &p2, double thickness)
 {
     double wave[16]={
-        0.000000,  0.382499,  0.706825,  0.923651,   1.000000,  0.923651,  0.706825,  0.382499, 
-        0.000000, -0.382499, -0.706825, -0.923651,  -1.000000, -0.923651, -0.706825, -0.382499, 
+        0.000000,  0.382499,  0.706825,  0.923651,   1.000000,  0.923651,  0.706825,  0.382499,
+        0.000000, -0.382499, -0.706825, -0.923651,  -1.000000, -0.923651, -0.706825, -0.382499,
     };
     int dashes[16]={
         8,   7,   6,   5,
@@ -283,7 +282,7 @@ void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphas
 
     /* For most spans draw the last little bit right to p2 or even a little beyond.
        This allows decoration continuity within the line, and does not step outside the clip box off the end
-       For the first/last section on the line though, stay well clear of the edge, or when the 
+       For the first/last section on the line though, stay well clear of the edge, or when the
        text is dragged it may "spray" pixels.
     */
     /* snap to nearest step in X */
@@ -329,7 +328,7 @@ void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphas
             }
             i = 0;  // once in phase, it stays in phase
         }
-    } 
+    }
     else if(_nrstyle.text_decoration_style & TEXT_DECORATION_STYLE_DASHED){
         Geom::Point pv = ps;
         while(1){
@@ -354,7 +353,7 @@ void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphas
             }
             i = 0;  // once in phase, it stays in phase
         }
-    } 
+    }
     else if(_nrstyle.text_decoration_style & TEXT_DECORATION_STYLE_WAVY){
         double   amp  = vextent/10.0;
         double   x    = ps[Geom::X];
@@ -375,10 +374,10 @@ void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphas
            if(x <= ps[Geom::X])break;
         }
         dc.closePath();
-    }  
+    }
     else { // TEXT_DECORATION_STYLE_SOLID, also default in case it was not set for some reason
         dc.rectangle( Geom::Rect(ps + poff, pf - poff));
-    } 
+    }
 }
 
 /* returns scaled line thickness */
@@ -629,7 +628,7 @@ unsigned DrawingText::_renderItem(DrawingContext &dc, Geom::IntRect const &/*are
             {
                 Inkscape::DrawingContext::Save save(dc);
                 dc.transform(_ctm);  // Needed so that fill pattern rotates with text
-                
+
                 if (has_td_fill && fill_first) {
                     _nrstyle.applyTextDecorationFill(dc);
                     dc.fillPreserve();
@@ -685,11 +684,7 @@ void DrawingText::_clipItem(DrawingContext &dc, Geom::IntRect const &/*area*/)
 DrawingItem *
 DrawingText::_pickItem(Geom::Point const &p, double delta, unsigned flags)
 {
-    DrawingItem *picked = DrawingGroup::_pickItem(p, delta, flags);
-    if (picked) {
-        return this;
-    }
-    return NULL;
+    return DrawingGroup::_pickItem(p, delta, flags) ? this : NULL;
 }
 
 bool
diff --git a/src/display/nr-filter-blend.cpp b/src/display/nr-filter-blend.cpp
index d0db6b42e..49dfc7ec4 100644
--- a/src/display/nr-filter-blend.cpp
+++ b/src/display/nr-filter-blend.cpp
@@ -3,7 +3,7 @@
  *//*
  * "This filter composites two objects together using commonly used
  * imaging software blending modes. It performs a pixel-wise combination
- * of two input images." 
+ * of two input images."
  * http://www.w3.org/TR/SVG11/filters.html#feBlend
  *
  * Authors:
@@ -32,7 +32,18 @@
 namespace Inkscape {
 namespace Filters {
 
-FilterBlend::FilterBlend() 
+const std::set<FilterBlendMode> FilterBlend::_valid_modes {
+        BLEND_NORMAL,      BLEND_MULTIPLY,
+        BLEND_SCREEN,      BLEND_DARKEN,
+        BLEND_LIGHTEN,     BLEND_OVERLAY,
+        BLEND_COLORDODGE,  BLEND_COLORBURN,
+        BLEND_HARDLIGHT,   BLEND_SOFTLIGHT,
+        BLEND_DIFFERENCE,  BLEND_EXCLUSION,
+        BLEND_HUE,         BLEND_SATURATION,
+        BLEND_COLOR,       BLEND_LUMINOSITY
+        };
+
+FilterBlend::FilterBlend()
     : _blend_mode(BLEND_NORMAL),
       _input2(NR_FILTER_SLOT_NOT_SET)
 {}
@@ -44,6 +55,47 @@ FilterPrimitive * FilterBlend::create() {
 FilterBlend::~FilterBlend()
 {}
 
+static inline cairo_operator_t get_cairo_op(FilterBlendMode _blend_mode)
+{
+    switch (_blend_mode) {
+    case BLEND_MULTIPLY:
+        return CAIRO_OPERATOR_MULTIPLY;
+    case BLEND_SCREEN:
+        return CAIRO_OPERATOR_SCREEN;
+    case BLEND_DARKEN:
+        return CAIRO_OPERATOR_DARKEN;
+    case BLEND_LIGHTEN:
+        return CAIRO_OPERATOR_LIGHTEN;
+    // New in CSS Compositing and Blending Level 1
+    case BLEND_OVERLAY:
+        return CAIRO_OPERATOR_OVERLAY;
+    case BLEND_COLORDODGE:
+        return CAIRO_OPERATOR_COLOR_DODGE;
+    case BLEND_COLORBURN:
+        return CAIRO_OPERATOR_COLOR_BURN;
+    case BLEND_HARDLIGHT:
+        return CAIRO_OPERATOR_HARD_LIGHT;
+    case BLEND_SOFTLIGHT:
+        return CAIRO_OPERATOR_SOFT_LIGHT;
+    case BLEND_DIFFERENCE:
+        return CAIRO_OPERATOR_DIFFERENCE;
+    case BLEND_EXCLUSION:
+        return CAIRO_OPERATOR_EXCLUSION;
+    case BLEND_HUE:
+        return CAIRO_OPERATOR_HSL_HUE;
+    case BLEND_SATURATION:
+        return CAIRO_OPERATOR_HSL_SATURATION;
+    case BLEND_COLOR:
+        return CAIRO_OPERATOR_HSL_COLOR;
+    case BLEND_LUMINOSITY:
+        return CAIRO_OPERATOR_HSL_LUMINOSITY;
+
+    case BLEND_NORMAL:
+    default:
+        return CAIRO_OPERATOR_OVER;
+    }
+}
+
 void FilterBlend::render_cairo(FilterSlot &slot)
 {
     cairo_surface_t *input1 = slot.getcairo(_input);
@@ -71,59 +123,7 @@ void FilterBlend::render_cairo(FilterSlot &slot)
     // All of the blend modes are implemented in Cairo as of 1.10.
     // For a detailed description, see:
     // http://cairographics.org/operators/
-    switch (_blend_mode) {
-    case BLEND_MULTIPLY:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_MULTIPLY);
-        break;
-    case BLEND_SCREEN:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_SCREEN);
-        break;
-    case BLEND_DARKEN:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_DARKEN);
-        break;
-    case BLEND_LIGHTEN:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_LIGHTEN);
-        break;
-    // New in CSS Compositing and Blending Level 1
-    case BLEND_OVERLAY:   
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_OVERLAY);
-        break;
-    case BLEND_COLORDODGE:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_COLOR_DODGE);
-        break;
-    case BLEND_COLORBURN:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_COLOR_BURN);
-        break;
-    case BLEND_HARDLIGHT:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_HARD_LIGHT);
-        break;
-    case BLEND_SOFTLIGHT:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_SOFT_LIGHT);
-        break;
-    case BLEND_DIFFERENCE:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_DIFFERENCE);
-        break;
-    case BLEND_EXCLUSION:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_EXCLUSION);
-        break;
-    case BLEND_HUE:       
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_HSL_HUE);
-        break;
-    case BLEND_SATURATION:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_HSL_SATURATION);
-        break;
-    case BLEND_COLOR:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_HSL_COLOR);
-        break;
-    case BLEND_LUMINOSITY:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_HSL_LUMINOSITY);
-        break;
-
-    case BLEND_NORMAL:
-    default:
-        cairo_set_operator(out_ct, CAIRO_OPERATOR_OVER);
-        break;
-    }
+    cairo_set_operator(out_ct, get_cairo_op(_blend_mode));
 
     cairo_paint(out_ct);
     cairo_destroy(out_ct);
@@ -145,13 +145,8 @@ double FilterBlend::complexity(Geom::Affine const &)
 
 bool FilterBlend::uses_background()
 {
-    if (_input == NR_FILTER_BACKGROUNDIMAGE || _input == NR_FILTER_BACKGROUNDALPHA ||
-        _input2 == NR_FILTER_BACKGROUNDIMAGE || _input2 == NR_FILTER_BACKGROUNDALPHA)
-    {
-        return true;
-    } else {
-        return false;
-    }
+    return (_input == NR_FILTER_BACKGROUNDIMAGE || _input == NR_FILTER_BACKGROUNDALPHA ||
+            _input2 == NR_FILTER_BACKGROUNDIMAGE || _input2 == NR_FILTER_BACKGROUNDALPHA);
 }
 
 void FilterBlend::set_input(int slot) {
@@ -164,15 +159,7 @@ void FilterBlend::set_input(int input, int slot) {
 }
 
 void FilterBlend::set_mode(FilterBlendMode mode) {
-    if (mode == BLEND_NORMAL     || mode == BLEND_MULTIPLY   ||
-        mode == BLEND_SCREEN     || mode == BLEND_DARKEN     ||
-        mode == BLEND_LIGHTEN    || mode == BLEND_OVERLAY    ||
-        mode == BLEND_COLORDODGE || mode == BLEND_COLORBURN  ||
-        mode == BLEND_HARDLIGHT  || mode == BLEND_SOFTLIGHT  ||
-        mode == BLEND_DIFFERENCE || mode == BLEND_EXCLUSION  ||
-        mode == BLEND_HUE        || mode == BLEND_SATURATION ||
-        mode == BLEND_COLOR      || mode == BLEND_LUMINOSITY
-        )
+    if (_valid_modes.count(mode))
     {
         _blend_mode = mode;
     }
diff --git a/src/display/nr-filter-blend.h b/src/display/nr-filter-blend.h
index 30c9d6725..5b1295c88 100644
--- a/src/display/nr-filter-blend.h
+++ b/src/display/nr-filter-blend.h
@@ -6,7 +6,7 @@
  *
  * "This filter composites two objects together using commonly used
  * imaging software blending modes. It performs a pixel-wise combination
- * of two input images." 
+ * of two input images."
  * http://www.w3.org/TR/SVG11/filters.html#feBlend
  *
  * Authors:
@@ -16,6 +16,7 @@
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
  */
+#include <set>
 
 #include "display/nr-filter-primitive.h"
 
@@ -29,14 +30,14 @@ enum FilterBlendMode {
     BLEND_DARKEN,
     BLEND_LIGHTEN,
     // New in CSS Compositing and Blending Level 1
-    BLEND_OVERLAY,   
+    BLEND_OVERLAY,
     BLEND_COLORDODGE,
     BLEND_COLORBURN,
     BLEND_HARDLIGHT,
     BLEND_SOFTLIGHT,
     BLEND_DIFFERENCE,
     BLEND_EXCLUSION,
-    BLEND_HUE,       
+    BLEND_HUE,
     BLEND_SATURATION,
     BLEND_COLOR,
     BLEND_LUMINOSITY,
@@ -59,6 +60,7 @@ public:
     void set_mode(FilterBlendMode mode);
 
 private:
+    static const std::set<FilterBlendMode> _valid_modes;
     FilterBlendMode _blend_mode;
     int _input2;
 };
diff --git a/src/display/nr-filter-diffuselighting.h b/src/display/nr-filter-diffuselighting.h
index 7739b3ea6..5ce505979 100644
--- a/src/display/nr-filter-diffuselighting.h
+++ b/src/display/nr-filter-diffuselighting.h
@@ -7,7 +7,7 @@
  * Authors:
  *   Niko Kiirala <niko@kiirala.com>
  *   Jean-Rene Reinhard <jr@komite.net>
- * 
+ *
  * Copyright (C) 2007 authors
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
@@ -26,7 +26,7 @@ typedef unsigned int guint32;
 
 namespace Inkscape {
 namespace Filters {
-    
+
 class FilterDiffuseLighting : public FilterPrimitive {
 public:
     FilterDiffuseLighting();
@@ -54,7 +54,7 @@ private:
 } /* namespace Filters */
 } /* namespace Inkscape */
 
-#endif /* __NR_FILTER_DIFFUSELIGHTING_H__ */
+#endif /* SEEN_NR_FILTER_DIFFUSELIGHTING_H */
 /*
   Local Variables:
   mode:c++
diff --git a/src/display/nr-filter-primitive.h b/src/display/nr-filter-primitive.h
index 4b7577159..a1339cc6c 100644
--- a/src/display/nr-filter-primitive.h
+++ b/src/display/nr-filter-primitive.h
@@ -69,7 +69,7 @@ public:
     // returns cache score factor, reflecting the cost of rendering this filter
     // this should return how many times slower this primitive is that normal rendering
     virtual double complexity(Geom::Affine const &/*ctm*/) { return 1.0; }
-    
+
     virtual bool uses_background() {
         if (_input == NR_FILTER_BACKGROUNDIMAGE || _input == NR_FILTER_BACKGROUNDALPHA) {
             return true;
@@ -138,10 +138,7 @@ protected:
 } /* namespace Filters */
 } /* namespace Inkscape */
 
-
-
-
-#endif /* __NR_FILTER_PRIMITIVE_H__ */
+#endif /* SEEN_NR_FILTER_PRIMITIVE_H */
 /*
   Local Variables:
   mode:c++
diff --git a/src/sp-dimensions.cpp b/src/sp-dimensions.cpp
new file mode 100644
index 000000000..f39b98945
--- /dev/null
+++ b/src/sp-dimensions.cpp
@@ -0,0 +1,55 @@
+/*
+ * SVG dimensions implementation
+ *
+ * Authors:
+ *   Lauris Kaplinski <lauris@kaplinski.com>
+ *   Edward Flick (EAF)
+ *   Abhishek Sharma
+ *   Jon A. Cruz <jon@joncruz.org>
+ *
+ * Copyright (C) 1999-2005 Authors
+ * Copyright (C) 2000-2001 Ximian, Inc.
+ *
+ * Released under GNU GPL, read the file 'COPYING' for more information
+ */
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+#include "sp-dimensions.h"
+#include "sp-item.h"
+
+void SPDimensions::calcDimsFromParentViewport(const SPItemCtx *ictx, bool assign_to_set)
+{
+#define ASSIGN(field) { if (assign_to_set) { field._set = true; } }
+    if (this->x.unit == SVGLength::PERCENT) {
+        ASSIGN(x);
+        this->x.computed = this->x.value * ictx->viewport.width();
+    }
+
+    if (this->y.unit == SVGLength::PERCENT) {
+        ASSIGN(y);
+        this->y.computed = this->y.value * ictx->viewport.height();
+    }
+
+    if (this->width.unit == SVGLength::PERCENT) {
+        ASSIGN(width);
+        this->width.computed = this->width.value * ictx->viewport.width();
+    }
+
+    if (this->height.unit == SVGLength::PERCENT) {
+        ASSIGN(height);
+        this->height.computed = this->height.value * ictx->viewport.height();
+    }
+}
+
+/*
+  Local Variables:
+  mode:c++
+  c-file-style:"stroustrup"
+  c-file-offsets:((innamespace . 0)(inline-open . 0)(case-label . +))
+  indent-tabs-mode:nil
+  fill-column:99
+  End:
+*/
+// vim: filetype=cpp:expandtab:shiftwidth=4:tabstop=8:softtabstop=4 :
diff --git a/src/sp-dimensions.h b/src/sp-dimensions.h
new file mode 100644
index 000000000..eb76df739
--- /dev/null
+++ b/src/sp-dimensions.h
@@ -0,0 +1,41 @@
+#ifndef SP_DIMENSIONS_H__
+#define SP_DIMENSIONS_H__
+
+/*
+ * dimensions helper class, common code used by root, image and others
+ *
+ * Authors:
+ *  Shlomi Fish
+ * Copyright (C) 2017 Shlomi Fish, authors
+ *
+ * Released under dual Expat and GNU GPL, read the file 'COPYING' for more information
+ *
+ */
+
+#include "svg/svg-length.h"
+
+class SPItemCtx;
+
+class SPDimensions {
+
+public:
+    SVGLength x;
+    SVGLength y;
+    SVGLength width;
+    SVGLength height;
+    void calcDimsFromParentViewport(const SPItemCtx *ictx, bool assign_to_set = false);
+};
+
+#endif
+
+/*
+  Local Variables:
+  mode:c++
+  c-file-style:"stroustrup"
+  c-basic-offset:2
+  c-file-offsets:((innamespace . 0)(inline-open . 0)(case-label . +))
+  indent-tabs-mode:nil
+  fill-column:99
+  End:
+*/
+// vim: filetype=cpp:expandtab:shiftwidth=2:tabstop=8:softtabstop=2:fileencoding=utf-8:textwidth=99 :
diff --git a/src/sp-filter-primitive.cpp b/src/sp-filter-primitive.cpp
index 0fbeed15b..7ced52e11 100644
--- a/src/sp-filter-primitive.cpp
+++ b/src/sp-filter-primitive.cpp
@@ -139,25 +139,7 @@ void SPFilterPrimitive::update(SPCtx *ctx, guint flags) {
     SPFilter *parent = SP_FILTER(this->parent);
 
     if( parent->primitiveUnits == SP_FILTER_UNITS_USERSPACEONUSE ) {
-        if (this->x.unit == SVGLength::PERCENT) {
-            this->x._set = true;
-            this->x.computed = this->x.value * ictx->viewport.width();
-        }
-
-        if (this->y.unit == SVGLength::PERCENT) {
-            this->y._set = true;
-            this->y.computed = this->y.value * ictx->viewport.height();
-        }
-
-        if (this->width.unit == SVGLength::PERCENT) {
-            this->width._set = true;
-            this->width.computed = this->width.value * ictx->viewport.width();
-        }
-
-        if (this->height.unit == SVGLength::PERCENT) {
-            this->height._set = true;
-            this->height.computed = this->height.value * ictx->viewport.height();
-        }
+        this->calcDimsFromParentViewport(ictx, true);
     }
 
     SPObject::update(ctx, flags);
diff --git a/src/sp-filter-primitive.h b/src/sp-filter-primitive.h
index d81adbd10..0ecb8af62 100644
--- a/src/sp-filter-primitive.h
+++ b/src/sp-filter-primitive.h
@@ -15,6 +15,7 @@
  */
 
 #include "sp-object.h"
+#include "sp-dimensions.h"
 #include "svg/svg-length.h"
 
 #define SP_FILTER_PRIMITIVE(obj) (dynamic_cast<SPFilterPrimitive*>((SPObject*)obj))
@@ -26,16 +27,13 @@ class Filter;
 class FilterPrimitive;
 } }
 
-class SPFilterPrimitive : public SPObject {
+class SPFilterPrimitive : public SPObject, public SPDimensions {
 public:
 	SPFilterPrimitive();
 	virtual ~SPFilterPrimitive();
 
     int image_in, image_out;
 
-    /* filter primitive subregion */
-    SVGLength x, y, height, width;
-
 protected:
 	virtual void build(SPDocument* doc, Inkscape::XML::Node* repr);
 	virtual void release();
diff --git a/src/sp-filter.cpp b/src/sp-filter.cpp
index 2a5ec0755..aa5fe2942 100644
--- a/src/sp-filter.cpp
+++ b/src/sp-filter.cpp
@@ -213,25 +213,7 @@ void SPFilter::update(SPCtx *ctx, guint flags) {
         // Note: This only works for root viewport since this routine is not called after
         // setting a new viewport. A true fix requires a strategy like SPItemView or SPMarkerView.
         if(this->filterUnits == SP_FILTER_UNITS_USERSPACEONUSE) {
-            if (this->x.unit == SVGLength::PERCENT) {
-                this->x._set = true;
-                this->x.computed = this->x.value * ictx->viewport.width();
-            }
-
-            if (this->y.unit == SVGLength::PERCENT) {
-                this->y._set = true;
-                this->y.computed = this->y.value * ictx->viewport.height();
-            }
-
-            if (this->width.unit == SVGLength::PERCENT) {
-                this->width._set = true;
-                this->width.computed = this->width.value * ictx->viewport.width();
-            }
-
-            if (this->height.unit == SVGLength::PERCENT) {
-                this->height._set = true;
-                this->height.computed = this->height.value * ictx->viewport.height();
-            }
+            this->calcDimsFromParentViewport(ictx, true);
         }
         /* do something to trigger redisplay, updates? */
 
diff --git a/src/sp-filter.h b/src/sp-filter.h
index 1c214c6b7..054562d39 100644
--- a/src/sp-filter.h
+++ b/src/sp-filter.h
@@ -16,6 +16,7 @@
 #include <map>
 
 #include "number-opt-number.h"
+#include "sp-dimensions.h"
 #include "sp-object.h"
 #include "sp-filter-units.h"
 #include "svg/svg-length.h"
@@ -38,7 +39,7 @@ struct ltstr {
     bool operator()(const char* s1, const char* s2) const;
 };
 
-class SPFilter : public SPObject {
+class SPFilter : public SPObject, public SPDimensions {
 public:
 	SPFilter();
 	virtual ~SPFilter();
@@ -47,14 +48,10 @@ public:
     unsigned int filterUnits_set : 1;
     SPFilterUnits primitiveUnits;
     unsigned int primitiveUnits_set : 1;
-    SVGLength x;
-    SVGLength y;
-    SVGLength width;
-    SVGLength height;
     NumberOptNumber filterRes;
     SPFilterReference *href;
     sigc::connection modified_connection;
-    
+
     guint getRefCount();
     guint _refcount;
 
diff --git a/src/sp-image.cpp b/src/sp-image.cpp
index 1961971cb..6736efdec 100644
--- a/src/sp-image.cpp
+++ b/src/sp-image.cpp
@@ -374,23 +374,8 @@ void SPImage::update(SPCtx *ctx, unsigned int flags) {
         }
     }
 
-
     // Calculate x, y, width, height from parent/initial viewport, see sp-root.cpp
-    if (this->x.unit == SVGLength::PERCENT) {
-        this->x.computed = this->x.value * ictx->viewport.width();
-    }
-
-    if (this->y.unit == SVGLength::PERCENT) {
-        this->y.computed = this->y.value * ictx->viewport.height();
-    }
-
-    if (this->width.unit == SVGLength::PERCENT) {
-        this->width.computed = this->width.value * ictx->viewport.width();
-    }
-
-    if (this->height.unit == SVGLength::PERCENT) {
-        this->height.computed = this->height.value * ictx->viewport.height();
-    }
+    this->calcDimsFromParentViewport(ictx);
 
     // Image creates a new viewport
     ictx->viewport= Geom::Rect::from_xywh( this->x.computed, this->y.computed,
diff --git a/src/sp-image.h b/src/sp-image.h
index 9fa33b5de..9cd5faa8b 100644
--- a/src/sp-image.h
+++ b/src/sp-image.h
@@ -19,6 +19,7 @@
 #include "display/curve.h"
 #include "sp-item.h"
 #include "viewbox.h"
+#include "sp-dimensions.h"
 
 #define SP_IMAGE(obj) (dynamic_cast<SPImage*>((SPObject*)obj))
 #define SP_IS_IMAGE(obj) (dynamic_cast<const SPImage*>((SPObject*)obj) != NULL)
@@ -26,16 +27,11 @@
 #define SP_IMAGE_HREF_MODIFIED_FLAG SP_OBJECT_USER_MODIFIED_FLAG_A
 
 namespace Inkscape { class Pixbuf; }
-class SPImage : public SPItem, public SPViewBox {
+class SPImage : public SPItem, public SPViewBox, public SPDimensions {
 public:
     SPImage();
     virtual ~SPImage();
 
-    SVGLength x;
-    SVGLength y;
-    SVGLength width;
-    SVGLength height;
-
     Geom::Rect clipbox;
     double sx, sy;
     double ox, oy;
diff --git a/src/sp-root.cpp b/src/sp-root.cpp
index 34047054a..9ea1aa976 100644
--- a/src/sp-root.cpp
+++ b/src/sp-root.cpp
@@ -40,8 +40,7 @@ SPRoot::SPRoot() : SPGroup(), SPViewBox()
     this->version.inkscape = zero_version;
     this->original.inkscape = zero_version;
 
-    this->x.unset(SVGLength::PERCENT, 0.0, 0.0); // Ignored for root SVG element
-    this->y.unset(SVGLength::PERCENT, 0.0, 0.0);
+    this->unset_x_and_y();
     this->width.unset(SVGLength::PERCENT, 1.0, 1.0);
     this->height.unset(SVGLength::PERCENT, 1.0, 1.0);
 
@@ -52,6 +51,12 @@ SPRoot::~SPRoot()
 {
 }
 
+void SPRoot::unset_x_and_y()
+{
+    this->x.unset(SVGLength::PERCENT, 0.0, 0.0); // Ignored for root SVG element
+    this->y.unset(SVGLength::PERCENT, 0.0, 0.0);
+}
+
 void SPRoot::build(SPDocument *document, Inkscape::XML::Node *repr)
 {
     //XML Tree being used directly here while it shouldn't be.
@@ -206,74 +211,63 @@ void SPRoot::remove_child(Inkscape::XML::Node *child)
     SPGroup::remove_child(child);
 }
 
-void SPRoot::update(SPCtx *ctx, guint flags)
+void SPRoot::setRootDimensions()
 {
-    SPItemCtx const *ictx = (SPItemCtx const *) ctx;
-
-    if( !this->parent ) {
+    /*
+     * This is the root SVG element:
+     *
+     * x, y, width, and height apply to positioning the SVG element inside a parent.
+     * For the root SVG in Inkscape there is no parent, thus special rules apply:
+     *   If width, height not set, width = 100%, height = 100% (as always).
+     *   If width and height are in percent, they are percent of viewBox width/height.
+     *   If width, height, and viewBox are not set... pick "random" width/height.
+     *   x, y are ignored.
+     *   initial viewport = (0 0 width height)
+     */
+    if( this->viewBox_set ) {
 
-        /*
-         * This is the root SVG element:
-         *
-         * x, y, width, and height apply to positioning the SVG element inside a parent.
-         * For the root SVG in Inkscape there is no parent, thus special rules apply:
-         *   If width, height not set, width = 100%, height = 100% (as always).
-         *   If width and height are in percent, they are percent of viewBox width/height.
-         *   If width, height, and viewBox are not set... pick "random" width/height.
-         *   x, y are ignored.
-         *   initial viewport = (0 0 width height)
-         */
-        if( this->viewBox_set ) {
-
-            if( this->width._set ) {
-                // Check if this is necessary
-                if (this->width.unit == SVGLength::PERCENT) {
-                    this->width.computed  = this->width.value  * this->viewBox.width();
-                }
-            } else {
-                this->width.set( SVGLength::PX, this->viewBox.width(),  this->viewBox.width()  );
+        if( this->width._set ) {
+            // Check if this is necessary
+            if (this->width.unit == SVGLength::PERCENT) {
+                this->width.computed  = this->width.value  * this->viewBox.width();
             }
+        } else {
+            this->width.set( SVGLength::PX, this->viewBox.width(),  this->viewBox.width()  );
+        }
 
-            if( this->height._set ) {
-                if (this->height.unit == SVGLength::PERCENT) {
-                    this->height.computed = this->height.value * this->viewBox.height();
-                }
-            } else {
-                this->height.set(SVGLength::PX, this->viewBox.height(), this->viewBox.height() );
+        if( this->height._set ) {
+            if (this->height.unit == SVGLength::PERCENT) {
+                this->height.computed = this->height.value * this->viewBox.height();
             }
-
         } else {
+            this->height.set(SVGLength::PX, this->viewBox.height(), this->viewBox.height() );
+        }
 
-            if( !this->width._set ) {
-                this->width.set(  SVGLength::PX, 100,  100 ); // Random default
-            }
+    } else {
 
-            if( !this->height._set ) {
-                this->height.set( SVGLength::PX, 100,  100 ); // Random default
-            }
+        if( !this->width._set ) {
+            this->width.set(  SVGLength::PX, 100,  100 ); // Random default
         }
 
-        // Ignore x, y values for root element
-        this->x.unset(SVGLength::PERCENT, 0.0, 0.0);
-        this->y.unset(SVGLength::PERCENT, 0.0, 0.0);
+        if( !this->height._set ) {
+            this->height.set( SVGLength::PX, 100,  100 ); // Random default
+        }
     }
 
-    // Calculate x, y, width, height from parent/initial viewport
-    if (this->x.unit == SVGLength::PERCENT) {
-        this->x.computed = this->x.value * ictx->viewport.width();
-    }
+    // Ignore x, y values for root element
+    this->unset_x_and_y();
+}
 
-    if (this->y.unit == SVGLength::PERCENT) {
-        this->y.computed = this->y.value * ictx->viewport.height();
-    }
+void SPRoot::update(SPCtx *ctx, guint flags)
+{
+    SPItemCtx const *ictx = (SPItemCtx const *) ctx;
 
-    if (this->width.unit == SVGLength::PERCENT) {
-        this->width.computed = this->width.value * ictx->viewport.width();
+    if( !this->parent ) {
+        this->setRootDimensions();
     }
 
-    if (this->height.unit == SVGLength::PERCENT) {
-        this->height.computed = this->height.value * ictx->viewport.height();
-    }
+    // Calculate x, y, width, height from parent/initial viewport
+    this->calcDimsFromParentViewport(ictx);
 
     // std::cout << "SPRoot::update: final:"
     //           << " x: " << x.computed
@@ -355,9 +349,7 @@ Inkscape::XML::Node *SPRoot::write(Inkscape::XML::Document *xml_doc, Inkscape::X
 
 Inkscape::DrawingItem *SPRoot::show(Inkscape::Drawing &drawing, unsigned int key, unsigned int flags)
 {
-    Inkscape::DrawingItem *ai = 0;
-
-    ai = SPGroup::show(drawing, key, flags);
+    Inkscape::DrawingItem *ai = SPGroup::show(drawing, key, flags);
 
     if (ai) {
         Inkscape::DrawingGroup *g = dynamic_cast<Inkscape::DrawingGroup *>(ai);
@@ -366,10 +358,10 @@ Inkscape::DrawingItem *SPRoot::show(Inkscape::Drawing &drawing, unsigned int key
 
     // Uncomment to print out XML tree
     // getRepr()->recursivePrintTree(0);
-    
+
     // Uncomment to print out SP Object tree
     // recursivePrintTree(0);
-    
+
     // Uncomment to print out Display Item tree
     // ai->recursivePrintTree(0);
 
diff --git a/src/sp-root.h b/src/sp-root.h
index 2776ae887..4a37840d9 100644
--- a/src/sp-root.h
+++ b/src/sp-root.h
@@ -18,6 +18,7 @@
 #include "svg/svg-length.h"
 #include "sp-item-group.h"
 #include "viewbox.h"
+#include "sp-dimensions.h"
 
 #define SP_ROOT(obj) (dynamic_cast<SPRoot*>((SPObject*)obj))
 #define SP_IS_ROOT(obj) (dynamic_cast<const SPRoot*>((SPObject*)obj) != NULL)
@@ -25,7 +26,7 @@
 class SPDefs;
 
 /** \<svg\> element */
-class SPRoot : public SPGroup, public SPViewBox {
+class SPRoot : public SPGroup, public SPViewBox, public SPDimensions {
 public:
 	SPRoot();
 	virtual ~SPRoot();
@@ -35,11 +36,6 @@ public:
         Inkscape::Version inkscape;
     } version, original;
 
-    SVGLength x;
-    SVGLength y;
-    SVGLength width;
-    SVGLength height;
-
     char *onload;
 
     /**
@@ -63,6 +59,9 @@ public:
 	virtual Inkscape::DrawingItem* show(Inkscape::Drawing &drawing, unsigned int key, unsigned int flags);
 	virtual void print(SPPrintContext *ctx);
     virtual const char* displayName() const;
+private:
+    void unset_x_and_y();
+    void setRootDimensions();
 };
 
 #endif /* !SP_ROOT_H_SEEN */
diff --git a/src/sp-tag.h b/src/sp-tag.h
index 927bb45d1..36459a04b 100644
--- a/src/sp-tag.h
+++ b/src/sp-tag.h
@@ -3,7 +3,7 @@
 
 /** \file
  * SVG <inkscape:tag> implementation
- * 
+ *
  * Authors:
  *   Theodore Janeczko
  *
@@ -25,19 +25,19 @@ class SPTag : public SPObject {
 public:
     SPTag() {}
     virtual ~SPTag() {}
-    
+
     virtual void build(SPDocument * doc, Inkscape::XML::Node *repr);
     //virtual void release();
     virtual void set(unsigned key, const gchar* value);
     virtual void update(SPCtx * ctx, unsigned flags);
-    
+
     virtual Inkscape::XML::Node* write(Inkscape::XML::Document* doc, Inkscape::XML::Node* repr, guint flags);
-    
+
     bool expanded() const { return _expanded; }
     void setExpanded(bool isexpanded);
 
     void moveTo(SPObject *target, gboolean intoafter);
-    
+
 private:
     bool _expanded;
 };
diff --git a/src/sp-use.cpp b/src/sp-use.cpp
index fae442cfa..61e8002bb 100644
--- a/src/sp-use.cpp
+++ b/src/sp-use.cpp
@@ -43,11 +43,8 @@
 
 SPUse::SPUse()
     : SPItem(),
+      SPDimensions(),
       child(NULL),
-      x(),
-      y(),
-      width(),
-      height(),
       href(NULL),
       ref(new SPUseReference(this)),
       _delete_connection(),
@@ -581,21 +578,7 @@ void SPUse::update(SPCtx *ctx, unsigned flags) {
     childflags &= SP_OBJECT_MODIFIED_CASCADE;
 
     /* Set up child viewport */
-    if (this->x.unit == SVGLength::PERCENT) {
-        this->x.computed = this->x.value * ictx->viewport.width();
-    }
-
-    if (this->y.unit == SVGLength::PERCENT) {
-        this->y.computed = this->y.value * ictx->viewport.height();
-    }
-
-    if (this->width.unit == SVGLength::PERCENT) {
-        this->width.computed = this->width.value * ictx->viewport.width();
-    }
-
-    if (this->height.unit == SVGLength::PERCENT) {
-        this->height.computed = this->height.value * ictx->viewport.height();
-    }
+    this->calcDimsFromParentViewport(ictx);
 
     childflags &= ~SP_OBJECT_USER_MODIFIED_FLAG_B;
 
diff --git a/src/sp-use.h b/src/sp-use.h
index 81e4391aa..bcf0a8513 100644
--- a/src/sp-use.h
+++ b/src/sp-use.h
@@ -19,12 +19,13 @@
 #include <sigc++/sigc++.h>
 
 #include "svg/svg-length.h"
+#include "sp-dimensions.h"
 #include "sp-item.h"
 #include "enums.h"
 
 class SPUseReference;
 
-class SPUse : public SPItem {
+class SPUse : public SPItem, public SPDimensions {
 public:
 	SPUse();
 	virtual ~SPUse();
@@ -34,10 +35,6 @@ public:
     SPItem *child;
 
     // SVG attrs
-    SVGLength x;
-    SVGLength y;
-    SVGLength width;
-    SVGLength height;
     char *href;
 
     // the reference to the original object
diff --git a/src/uri.cpp b/src/uri.cpp
index 9178c8ae8..219792b6e 100644
--- a/src/uri.cpp
+++ b/src/uri.cpp
@@ -159,7 +159,7 @@ const std::string URI::getFullPath(std::string const &base) const {
         path = Glib::build_filename(base, path);
     }
     // Check the existance of the file
-    if(! g_file_test(path.c_str(), G_FILE_TEST_EXISTS) 
+    if(! g_file_test(path.c_str(), G_FILE_TEST_EXISTS)
       || g_file_test(path.c_str(), G_FILE_TEST_IS_DIR) ) {
         path.clear();
     }
@@ -207,13 +207,11 @@ URI URI::fromUtf8( gchar const* path ) {
             tmp += (gunichar)one;
         } else {
             gchar scratch[4];
-            g_snprintf( scratch, 4, "%c%02X", '%', one );
+            g_snprintf( scratch, 4, "%%%02X", one );
             tmp.append( scratch );
         }
     }
-    const gchar *uri = tmp.data();
-    URI result(uri);
-    return result;
+    return URI( tmp.data() );
 }
 
 /* TODO !!! proper error handling */
diff --git a/src/vanishing-point.h b/src/vanishing-point.h
index ae2a88d6e..f396225df 100644
--- a/src/vanishing-point.h
+++ b/src/vanishing-point.h
@@ -176,7 +176,7 @@ public:
     std::vector<SPCtrlLine *> lines;
 
     void printDraggers(); // convenience for debugging
-    /* 
+    /*
      * FIXME: Should the following functions be merged?
      *        Also, they should make use of the info in a VanishingPoint structure (regarding boxes
      *        and perspectives) rather than each time iterating over the whole list of selected items?