Partial fix for bug 1430873. Rectangles should behave properly with % values.

(bzr r14004)

6 files changed, 140 insertions, 91 deletions

diff --git a/src/sp-rect.cpp b/src/sp-rect.cpp
index 30571a8dd..0128c248f 100644
--- a/src/sp-rect.cpp
+++ b/src/sp-rect.cpp
@@ -129,19 +129,19 @@ Inkscape::XML::Node * SPRect::write(Inkscape::XML::Document *xml_doc, Inkscape::
         repr = xml_doc->createElement("svg:rect");
     }
 
-    sp_repr_set_svg_double(repr, "width", this->width.computed);
-    sp_repr_set_svg_double(repr, "height", this->height.computed);
+    sp_repr_set_svg_length(repr, "width",  this->width);
+    sp_repr_set_svg_length(repr, "height", this->height);
 
     if (this->rx._set) {
-    	sp_repr_set_svg_double(repr, "rx", this->rx.computed);
+    	sp_repr_set_svg_length(repr, "rx", this->rx);
     }
 
     if (this->ry._set) {
-    	sp_repr_set_svg_double(repr, "ry", this->ry.computed);
+    	sp_repr_set_svg_length(repr, "ry", this->ry);
     }
 
-    sp_repr_set_svg_double(repr, "x", this->x.computed);
-    sp_repr_set_svg_double(repr, "y", this->y.computed);
+    sp_repr_set_svg_length(repr, "x", this->x);
+    sp_repr_set_svg_length(repr, "y", this->y);
 
     this->set_shape(); // evaluate SPCurve
     SPShape::write(xml_doc, repr, flags);
@@ -235,29 +235,29 @@ void SPRect::set_shape() {
 /* fixme: Think (Lauris) */
 
 void SPRect::setPosition(gdouble x, gdouble y, gdouble width, gdouble height) {
-    this->x.computed = x;
-    this->y.computed = y;
-    this->width.computed = width;
-    this->height.computed = height;
+    this->x = x;
+    this->y = y;
+    this->width = width;
+    this->height = height;
 
     this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
 }
 
 void SPRect::setRx(bool set, gdouble value) {
-	this->rx._set = set;
+    this->rx._set = set;
 
     if (set) {
-    	this->rx.computed = value;
+    	this->rx = value;
     }
 
     this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
 }
 
 void SPRect::setRy(bool set, gdouble value) {
-	this->ry._set = set;
+    this->ry._set = set;
 
     if (set) {
-    	this->ry.computed = value;
+    	this->ry = value;
     }
 
     this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
@@ -289,16 +289,16 @@ Geom::Affine SPRect::set_transform(Geom::Affine const& xform) {
         ret[3] = 1.0;
     }
 
-    /* fixme: Would be nice to preserve units here */
-    this->width = this->width.computed * sw;
-    this->height = this->height.computed * sh;
+    /* Preserve units */
+    this->width.scale( sw );
+    this->height.scale( sh );
 
     if (this->rx._set) {
-    	this->rx = this->rx.computed * sw;
+    	this->rx.scale( sw );
     }
 
     if (this->ry._set) {
-    	this->ry = this->ry.computed * sh;
+    	this->ry.scale( sh );
     }
 
     /* Find start in item coords */
@@ -336,15 +336,12 @@ gdouble SPRect::vectorStretch(Geom::Point p0, Geom::Point p1, Geom::Affine xform
 
 void SPRect::setVisibleRx(gdouble rx) {
     if (rx == 0) {
-        this->rx.computed = 0;
-        this->rx._set = false;
+        this->rx.unset();
     } else {
-    	this->rx.computed = rx / SPRect::vectorStretch(
+    	this->rx = rx / SPRect::vectorStretch(
             Geom::Point(this->x.computed + 1, this->y.computed),
             Geom::Point(this->x.computed, this->y.computed),
             this->i2doc_affine());
-
-    	this->rx._set = true;
     }
 
     this->updateRepr();
@@ -352,15 +349,12 @@ void SPRect::setVisibleRx(gdouble rx) {
 
 void SPRect::setVisibleRy(gdouble ry) {
     if (ry == 0) {
-    	this->ry.computed = 0;
-    	this->ry._set = false;
+    	this->ry.unset();
     } else {
-    	this->ry.computed = ry / SPRect::vectorStretch(
+    	this->ry = ry / SPRect::vectorStretch(
             Geom::Point(this->x.computed, this->y.computed + 1),
             Geom::Point(this->x.computed, this->y.computed),
             this->i2doc_affine());
-
-    	this->ry._set = true;
     }
 
     this->updateRepr();
@@ -418,37 +412,33 @@ void SPRect::compensateRxRy(Geom::Affine xform) {
     // This is needed because if we just set them the same length in SVG, they might end up unequal because of transform
     if ((this->rx._set && !this->ry._set) || (this->ry._set && !this->rx._set)) {
         gdouble r = MAX(this->rx.computed, this->ry.computed);
-        this->rx.computed = r / eX;
-        this->ry.computed = r / eY;
+        this->rx = r / eX;
+        this->ry = r / eY;
     } else {
-    	this->rx.computed = this->rx.computed / eX;
-    	this->ry.computed = this->ry.computed / eY;
+    	this->rx = this->rx.computed / eX;
+    	this->ry = this->ry.computed / eY;
     }
 
     // Note that a radius may end up larger than half-side if the rect is scaled down;
     // that's ok because this preserves the intended radii in case the rect is enlarged again,
     // and set_shape will take care of trimming too large radii when generating d=
-
-    this->rx._set = this->ry._set = true;
 }
 
 void SPRect::setVisibleWidth(gdouble width) {
-	this->width.computed = width / SPRect::vectorStretch(
+    this->width = width / SPRect::vectorStretch(
         Geom::Point(this->x.computed + 1, this->y.computed),
         Geom::Point(this->x.computed, this->y.computed),
         this->i2doc_affine());
 
-	this->width._set = true;
-	this->updateRepr();
+    this->updateRepr();
 }
 
 void SPRect::setVisibleHeight(gdouble height) {
-	this->height.computed = height / SPRect::vectorStretch(
+    this->height = height / SPRect::vectorStretch(
         Geom::Point(this->x.computed, this->y.computed + 1),
         Geom::Point(this->x.computed, this->y.computed),
         this->i2doc_affine());
 
-	this->height._set = true;
 	this->updateRepr();
 }
 
diff --git a/src/svg/svg-length.cpp b/src/svg/svg-length.cpp
index ea235b2e4..edbc59c36 100644
--- a/src/svg/svg-length.cpp
+++ b/src/svg/svg-length.cpp
@@ -14,6 +14,7 @@
 #include <cstring>
 #include <string>
 #include <glib.h>
+#include <iostream>
 
 #include "svg.h"
 #include "stringstream.h"
@@ -462,6 +463,51 @@ unsigned int sp_svg_length_read_ldd(gchar const *str, SVGLength::Unit *unit, dou
     return r;
 }
 
+std::string const SVGLength::write()
+{
+    return sp_svg_length_write_with_units(*this);
+}
+
+void SVGLength::set(SVGLength::Unit u, float v)
+{
+    _set = true;
+    unit = u;
+    Glib::ustring hack("px");
+    switch( unit ) {
+        case NONE:
+        case PX:
+        case EM:
+        case EX:
+        case PERCENT:
+            break;
+        case PT:
+            hack = "pt";
+            break;
+        case PC:
+            hack = "pc";
+            break;
+        case MM:
+            hack = "pt";
+            break;
+        case CM:
+            hack = "pt";
+            break;
+        case INCH:
+            hack = "pt";
+            break;
+        case FOOT:
+            hack = "pt";
+            break;
+        case MITRE:
+            hack = "m";
+            break;
+        default:
+            break;
+    }
+    value = v;
+    computed =  Inkscape::Util::Quantity::convert(v, hack, "px");
+}
+
 void SVGLength::set(SVGLength::Unit u, float v, float c)
 {
     _set = true;
@@ -478,6 +524,12 @@ void SVGLength::unset(SVGLength::Unit u, float v, float c)
     computed = c;
 }
 
+void SVGLength::scale(double scale)
+{
+    value *= scale;
+    computed *= scale;
+}
+
 void SVGLength::update(double em, double ex, double scale)
 {
     if (unit == EM) {
diff --git a/src/svg/svg-length.h b/src/svg/svg-length.h
index 1e6b4c96c..84056dd5f 100644
--- a/src/svg/svg-length.h
+++ b/src/svg/svg-length.h
@@ -57,9 +57,13 @@ public:
     bool read(char const *str);
     void readOrUnset(char const *str, Unit u = NONE, float v = 0, float c = 0);
     bool readAbsolute(char const *str);
-    void set(Unit u, float v, float c);
+    std::string const write();
+    // To set 'v' use '='
+    void set(Unit u, float v); // Sets computed value based on u and v.
+    void set(Unit u, float v, float c); // Sets all three values.
     void unset(Unit u = NONE, float v = 0, float c = 0);
-    void update(double em, double ex, double scale);
+    void scale(double scale); // Scales length (value, computed), leaving unit alone.
+    void update(double em, double ex, double scale); // Updates computed value
 };
 
 #endif // SEEN_SP_SVG_LENGTH_H
diff --git a/src/ui/object-edit.cpp b/src/ui/object-edit.cpp
index c3bc2d52c..04ee13cef 100644
--- a/src/ui/object-edit.cpp
+++ b/src/ui/object-edit.cpp
@@ -154,12 +154,9 @@ RectKnotHolderEntityRX::knot_set(Geom::Point const &p, Geom::Point const &/*orig
 
     if (state & GDK_CONTROL_MASK) {
         gdouble temp = MIN(rect->height.computed, rect->width.computed) / 2.0;
-        rect->rx.computed = rect->ry.computed = CLAMP(rect->x.computed + rect->width.computed - s[Geom::X], 0.0, temp);
-        rect->rx._set = rect->ry._set = true;
-
+        rect->rx = rect->ry = CLAMP(rect->x.computed + rect->width.computed - s[Geom::X], 0.0, temp);
     } else {
-        rect->rx.computed = CLAMP(rect->x.computed + rect->width.computed - s[Geom::X], 0.0, rect->width.computed / 2.0);
-        rect->rx._set = true;
+        rect->rx = CLAMP(rect->x.computed + rect->width.computed - s[Geom::X], 0.0, rect->width.computed / 2.0);
     }
 
     update_knot();
@@ -207,20 +204,17 @@ RectKnotHolderEntityRY::knot_set(Geom::Point const &p, Geom::Point const &/*orig
     if (state & GDK_CONTROL_MASK) { // When holding control then rx will be kept equal to ry,
                                     // resulting in a perfect circle (and not an ellipse)
         gdouble temp = MIN(rect->height.computed, rect->width.computed) / 2.0;
-        rect->rx.computed = rect->ry.computed = CLAMP(s[Geom::Y] - rect->y.computed, 0.0, temp);
-        rect->ry._set = rect->rx._set = true;
+        rect->rx = rect->ry = CLAMP(s[Geom::Y] - rect->y.computed, 0.0, temp);
     } else {
         if (!rect->rx._set || rect->rx.computed == 0) {
-            rect->ry.computed = CLAMP(s[Geom::Y] - rect->y.computed,
-                                      0.0,
-                                      MIN(rect->height.computed / 2.0, rect->width.computed / 2.0));
+            rect->ry = CLAMP(s[Geom::Y] - rect->y.computed,
+                             0.0,
+                             MIN(rect->height.computed / 2.0, rect->width.computed / 2.0));
         } else {
-            rect->ry.computed = CLAMP(s[Geom::Y] - rect->y.computed,
-                                      0.0,
-                                      rect->height.computed / 2.0);
+            rect->ry = CLAMP(s[Geom::Y] - rect->y.computed,
+                             0.0,
+                             rect->height.computed / 2.0);
         }
-
-        rect->ry._set = true;
     }
 
     update_knot();
@@ -250,12 +244,10 @@ static void sp_rect_clamp_radii(SPRect *rect)
 {
     // clamp rounding radii so that they do not exceed width/height
     if (2 * rect->rx.computed > rect->width.computed) {
-        rect->rx.computed = 0.5 * rect->width.computed;
-        rect->rx._set = true;
+        rect->rx = 0.5 * rect->width.computed;
     }
     if (2 * rect->ry.computed > rect->height.computed) {
-        rect->ry.computed = 0.5 * rect->height.computed;
-        rect->ry._set = true;
+        rect->ry = 0.5 * rect->height.computed;
     }
 }
 
@@ -298,16 +290,16 @@ RectKnotHolderEntityWH::set_internal(Geom::Point const &p, Geom::Point const &or
                 minx = s[Geom::X] - origin[Geom::X];
                 // Dead assignment: Value stored to 'miny' is never read
                 //miny = s[Geom::Y] - origin[Geom::Y];
-                rect->height.computed = MAX(h_orig + minx / ratio, 0);
+                rect->height = MAX(h_orig + minx / ratio, 0);
             } else {
                 // closer to the horizontal, change only width, height is h_orig
                 s = snap_knot_position_constrained(p, Inkscape::Snapper::SnapConstraint(p_handle, Geom::Point(-1, 0)), state);
                 minx = s[Geom::X] - origin[Geom::X];
                 // Dead assignment: Value stored to 'miny' is never read
                 //miny = s[Geom::Y] - origin[Geom::Y];
-                rect->height.computed = MAX(h_orig, 0);
+                rect->height = MAX(h_orig, 0);
             }
-            rect->width.computed = MAX(w_orig + minx, 0);
+            rect->width = MAX(w_orig + minx, 0);
 
         } else {
             // snap to vertical or diagonal
@@ -317,27 +309,24 @@ RectKnotHolderEntityWH::set_internal(Geom::Point const &p, Geom::Point const &or
                 // Dead assignment: Value stored to 'minx' is never read
                 //minx = s[Geom::X] - origin[Geom::X];
                 miny = s[Geom::Y] - origin[Geom::Y];
-                rect->width.computed = MAX(w_orig + miny * ratio, 0);
+                rect->width = MAX(w_orig + miny * ratio, 0);
             } else {
                 // closer to the vertical, change only height, width is w_orig
                 s = snap_knot_position_constrained(p, Inkscape::Snapper::SnapConstraint(p_handle, Geom::Point(0, -1)), state);
                 // Dead assignment: Value stored to 'minx' is never read
                 //minx = s[Geom::X] - origin[Geom::X];
                 miny = s[Geom::Y] - origin[Geom::Y];
-                rect->width.computed = MAX(w_orig, 0);
+                rect->width = MAX(w_orig, 0);
             }
-            rect->height.computed = MAX(h_orig + miny, 0);
+            rect->height = MAX(h_orig + miny, 0);
 
         }
 
-        rect->width._set = rect->height._set = true;
-
     } else {
         // move freely
         s = snap_knot_position(p, state);
-        rect->width.computed = MAX(s[Geom::X] - rect->x.computed, 0);
-        rect->height.computed = MAX(s[Geom::Y] - rect->y.computed, 0);
-        rect->width._set = rect->height._set = true;
+        rect->width = MAX(s[Geom::X] - rect->x.computed, 0);
+        rect->height = MAX(s[Geom::Y] - rect->y.computed, 0);
     }
 
     sp_rect_clamp_radii(rect);
@@ -394,19 +383,19 @@ RectKnotHolderEntityXY::knot_set(Geom::Point const &p, Geom::Point const &origin
                 minx = s[Geom::X] - origin[Geom::X];
                 // Dead assignment: Value stored to 'miny' is never read
                 //miny = s[Geom::Y] - origin[Geom::Y];
-                rect->y.computed = MIN(origin[Geom::Y] + minx / ratio, opposite_y);
-                rect->height.computed = MAX(h_orig - minx / ratio, 0);
+                rect->y = MIN(origin[Geom::Y] + minx / ratio, opposite_y);
+                rect->height = MAX(h_orig - minx / ratio, 0);
             } else {
                 // closer to the horizontal, change only width, height is h_orig
                 s = snap_knot_position_constrained(p, Inkscape::Snapper::SnapConstraint(p_handle, Geom::Point(-1, 0)), state);
                 minx = s[Geom::X] - origin[Geom::X];
                 // Dead assignment: Value stored to 'miny' is never read
                 //miny = s[Geom::Y] - origin[Geom::Y];
-                rect->y.computed = MIN(origin[Geom::Y], opposite_y);
-                rect->height.computed = MAX(h_orig, 0);
+                rect->y = MIN(origin[Geom::Y], opposite_y);
+                rect->height = MAX(h_orig, 0);
             }
-            rect->x.computed = MIN(s[Geom::X], opposite_x);
-            rect->width.computed = MAX(w_orig - minx, 0);
+            rect->x = MIN(s[Geom::X], opposite_x);
+            rect->width = MAX(w_orig - minx, 0);
         } else {
             // snap to vertical or diagonal
             if (miny != 0 && fabs(minx/miny) > 0.5 *ratio && (SGN(minx) == SGN(miny))) {
@@ -415,34 +404,31 @@ RectKnotHolderEntityXY::knot_set(Geom::Point const &p, Geom::Point const &origin
                 // Dead assignment: Value stored to 'minx' is never read
                 //minx = s[Geom::X] - origin[Geom::X];
                 miny = s[Geom::Y] - origin[Geom::Y];
-                rect->x.computed = MIN(origin[Geom::X] + miny * ratio, opposite_x);
-                rect->width.computed = MAX(w_orig - miny * ratio, 0);
+                rect->x = MIN(origin[Geom::X] + miny * ratio, opposite_x);
+                rect->width = MAX(w_orig - miny * ratio, 0);
             } else {
                 // closer to the vertical, change only height, width is w_orig
                 s = snap_knot_position_constrained(p, Inkscape::Snapper::SnapConstraint(p_handle, Geom::Point(0, -1)), state);
                 // Dead assignment: Value stored to 'minx' is never read
                 //minx = s[Geom::X] - origin[Geom::X];
                 miny = s[Geom::Y] - origin[Geom::Y];
-                rect->x.computed = MIN(origin[Geom::X], opposite_x);
-                rect->width.computed = MAX(w_orig, 0);
+                rect->x = MIN(origin[Geom::X], opposite_x);
+                rect->width = MAX(w_orig, 0);
             }
-            rect->y.computed = MIN(s[Geom::Y], opposite_y);
-            rect->height.computed = MAX(h_orig - miny, 0);
+            rect->y = MIN(s[Geom::Y], opposite_y);
+            rect->height = MAX(h_orig - miny, 0);
         }
 
-        rect->width._set = rect->height._set = rect->x._set = rect->y._set = true;
-
     } else {
         // move freely
         s = snap_knot_position(p, state);
         minx = s[Geom::X] - origin[Geom::X];
         miny = s[Geom::Y] - origin[Geom::Y];
 
-        rect->x.computed = MIN(s[Geom::X], opposite_x);
-        rect->width.computed = MAX(w_orig - minx, 0);
-        rect->y.computed = MIN(s[Geom::Y], opposite_y);
-        rect->height.computed = MAX(h_orig - miny, 0);
-        rect->width._set = rect->height._set = rect->x._set = rect->y._set = true;
+        rect->x = MIN(s[Geom::X], opposite_x);
+        rect->y = MIN(s[Geom::Y], opposite_y);
+        rect->width = MAX(w_orig - minx, 0);
+        rect->height = MAX(h_orig - miny, 0);
     }
 
     sp_rect_clamp_radii(rect);
diff --git a/src/xml/repr-util.cpp b/src/xml/repr-util.cpp
index 12280ea5a..7c5d2d6fc 100644
--- a/src/xml/repr-util.cpp
+++ b/src/xml/repr-util.cpp
@@ -32,6 +32,7 @@
 #include <2geom/point.h>
 #include "svg/stringstream.h"
 #include "svg/css-ostringstream.h"
+#include "svg/svg-length.h"
 
 #include "xml/repr.h"
 #include "xml/repr-sorting.h"
@@ -502,6 +503,20 @@ unsigned int sp_repr_set_svg_double(Inkscape::XML::Node *repr, gchar const *key,
     return true;
 }
 
+/**
+ * For attributes where an exponent is allowed.
+ *
+ * Not suitable for property attributes.
+ */
+unsigned int sp_repr_set_svg_length(Inkscape::XML::Node *repr, gchar const *key, SVGLength &val)
+{
+    g_return_val_if_fail(repr != NULL, FALSE);
+    g_return_val_if_fail(key != NULL, FALSE);
+
+    repr->setAttribute(key, val.write());
+    return true;
+}
+
 unsigned sp_repr_set_point(Inkscape::XML::Node *repr, gchar const *key, Geom::Point const & val)
 {
     g_return_val_if_fail(repr != NULL, FALSE);
diff --git a/src/xml/repr.h b/src/xml/repr.h
index e1d7fdfd6..c3ba40e45 100644
--- a/src/xml/repr.h
+++ b/src/xml/repr.h
@@ -30,6 +30,7 @@
 #define SP_DC_NS_URI "http://purl.org/dc/elements/1.1/"
 
 class SPCSSAttr;
+class SVGLength;
 
 namespace Inkscape {
 namespace IO {
@@ -115,6 +116,7 @@ unsigned sp_repr_set_boolean(Inkscape::XML::Node *repr, char const *key, unsigne
 unsigned sp_repr_set_int(Inkscape::XML::Node *repr, char const *key, int val);
 unsigned sp_repr_set_css_double(Inkscape::XML::Node *repr, char const *key, double val);
 unsigned sp_repr_set_svg_double(Inkscape::XML::Node *repr, char const *key, double val);
+unsigned sp_repr_set_svg_length(Inkscape::XML::Node *repr, char const *key, SVGLength &val);
 unsigned sp_repr_set_point(Inkscape::XML::Node *repr, char const *key, Geom::Point const & val);
 unsigned sp_repr_get_point(Inkscape::XML::Node *repr, char const *key, Geom::Point *val);