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

(bzr r14009)

5 files changed, 103 insertions, 86 deletions

diff --git a/src/sp-ellipse.cpp b/src/sp-ellipse.cpp
index 885fedafa..932a3a1b7 100644
--- a/src/sp-ellipse.cpp
+++ b/src/sp-ellipse.cpp
@@ -95,6 +95,10 @@ void SPGenericEllipse::build(SPDocument *document, Inkscape::XML::Node *repr)
             std::cerr << "SPGenericEllipse::build() unknown defined type." << std::endl;
     }
 
+    // std::cout << "    cx: " << cx.write() << std::endl;
+    // std::cout << "    cy: " << cy.write() << std::endl;
+    // std::cout << "    rx: " << rx.write() << std::endl;
+    // std::cout << "    ry: " << ry.write() << std::endl;
     SPShape::build(document, repr);
 }
 
@@ -103,29 +107,41 @@ void SPGenericEllipse::set(unsigned int key, gchar const *value)
     // There are multiple ways to set internal cx, cy, rx, and ry (via SVG attributes or Sodipodi
     // attributes) thus we don't want to unset them if a read fails (e.g., when we explicitly clear
     // an attribute by setting it to NULL).
+
+    // We must update the SVGLengths immediately or nodes may be misplaced after they are moved.
+    double const w = viewport.width();
+    double const h = viewport.height();
+    double const d = hypot(w, h) / sqrt(2); // diagonal
+    double const em = style->font_size.computed;
+    double const ex = em * 0.5;
+
     SVGLength t;
     switch (key) {
     case SP_ATTR_CX:
     case SP_ATTR_SODIPODI_CX:
         if( t.read(value) ) cx = t;
+        cx.update( em, ex, w );
         this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_CY:
     case SP_ATTR_SODIPODI_CY:
         if( t.read(value) ) cy = t;
+        cy.update( em, ex, h );
         this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_RX:
     case SP_ATTR_SODIPODI_RX:
-        if( t.read(value) && t.value > 0.0 ) this->rx = t;
+        if( t.read(value) && t.value > 0.0 ) rx = t;
+        rx.update( em, ex, w );
         this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_RY:
     case SP_ATTR_SODIPODI_RY:
-        if( t.read(value) && t.value > 0.0 ) this->ry = t;
+        if( t.read(value) && t.value > 0.0 ) ry = t;
+        ry.update( em, ex, h );
         this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
@@ -133,6 +149,8 @@ void SPGenericEllipse::set(unsigned int key, gchar const *value)
         if( t.read(value) && t.value > 0.0 ) {
             this->ry = this->rx = t;
         }
+        rx.update( em, ex, d );
+        ry.update( em, ex, d );
         this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
@@ -196,7 +214,7 @@ Inkscape::XML::Node *SPGenericEllipse::write(Inkscape::XML::Document *xml_doc, I
     //           << ")" << std::endl;
 
     GenericEllipseType new_type = SP_GENERIC_ELLIPSE_UNDEFINED;
-    if (this->_isSlice() || hasPathEffect() ) {
+    if (_isSlice() || hasPathEffect() ) {
         new_type = SP_GENERIC_ELLIPSE_ARC;
     } else if ( rx.computed == ry.computed ) {
         new_type = SP_GENERIC_ELLIPSE_CIRCLE;
@@ -246,10 +264,10 @@ Inkscape::XML::Node *SPGenericEllipse::write(Inkscape::XML::Document *xml_doc, I
     }
 
     // std::cout << "  type: " << g_quark_to_string( repr->code() ) << std::endl;
-    // std::cout << "  cx: " << cx.computed
-    //           << "  cy: " << cy.computed
-    //           << "  rx: " << rx.computed
-    //           << "  ry: " << ry.computed << std::endl;
+    // std::cout << "  cx: " << cx.write() << " " << cx.computed
+    //           << "  cy: " << cy.write() << " " << cy.computed
+    //           << "  rx: " << rx.write() << " " << rx.computed
+    //           << "  ry: " << ry.write() << " " << ry.computed << std::endl;
 
     switch ( type ) {
         case SP_GENERIC_ELLIPSE_UNDEFINED:
@@ -264,17 +282,17 @@ Inkscape::XML::Node *SPGenericEllipse::write(Inkscape::XML::Document *xml_doc, I
             if (flags & SP_OBJECT_WRITE_EXT) {
 
                 repr->setAttribute("sodipodi:type", "arc");
-                sp_repr_set_svg_double(repr, "sodipodi:cx", this->cx.computed);
-                sp_repr_set_svg_double(repr, "sodipodi:cy", this->cy.computed);
-                sp_repr_set_svg_double(repr, "sodipodi:rx", this->rx.computed);
-                sp_repr_set_svg_double(repr, "sodipodi:ry", this->ry.computed);
+                sp_repr_set_svg_length(repr, "sodipodi:cx", cx);
+                sp_repr_set_svg_length(repr, "sodipodi:cy", cy);
+                sp_repr_set_svg_length(repr, "sodipodi:rx", rx);
+                sp_repr_set_svg_length(repr, "sodipodi:ry", ry);
 
                 // write start and end only if they are non-trivial; otherwise remove
-                if (this->_isSlice()) {
-                    sp_repr_set_svg_double(repr, "sodipodi:start", this->start);
-                    sp_repr_set_svg_double(repr, "sodipodi:end", this->end);
+                if (_isSlice()) {
+                    sp_repr_set_svg_double(repr, "sodipodi:start", start);
+                    sp_repr_set_svg_double(repr, "sodipodi:end", end);
 
-                    repr->setAttribute("sodipodi:open", (!this->_closed) ? "true" : NULL);
+                    repr->setAttribute("sodipodi:open", (!_closed) ? "true" : NULL);
                 } else {
                     repr->setAttribute("sodipodi:end", NULL);
                     repr->setAttribute("sodipodi:start", NULL);
@@ -283,13 +301,13 @@ Inkscape::XML::Node *SPGenericEllipse::write(Inkscape::XML::Document *xml_doc, I
             }
 
             // write d=
-            this->set_elliptical_path_attribute(repr);
+            set_elliptical_path_attribute(repr);
             break;
 
         case SP_GENERIC_ELLIPSE_CIRCLE:
-            sp_repr_set_svg_double(repr, "cx", this->cx.computed);
-            sp_repr_set_svg_double(repr, "cy", this->cy.computed);
-            sp_repr_set_svg_double(repr, "r",  this->rx.computed);
+            sp_repr_set_svg_length(repr, "cx", cx);
+            sp_repr_set_svg_length(repr, "cy", cy);
+            sp_repr_set_svg_length(repr, "r",  rx);
             repr->setAttribute("rx", NULL );
             repr->setAttribute("ry", NULL );
             repr->setAttribute("sodipodi:cx", NULL );
@@ -304,10 +322,10 @@ Inkscape::XML::Node *SPGenericEllipse::write(Inkscape::XML::Document *xml_doc, I
             break;
 
         case SP_GENERIC_ELLIPSE_ELLIPSE:
-            sp_repr_set_svg_double(repr, "cx", this->cx.computed);
-            sp_repr_set_svg_double(repr, "cy", this->cy.computed);
-            sp_repr_set_svg_double(repr, "rx", this->rx.computed);
-            sp_repr_set_svg_double(repr, "ry", this->ry.computed);
+            sp_repr_set_svg_length(repr, "cx", cx);
+            sp_repr_set_svg_length(repr, "cy", cy);
+            sp_repr_set_svg_length(repr, "rx", rx);
+            sp_repr_set_svg_length(repr, "ry", ry);
             repr->setAttribute("r", NULL );
             repr->setAttribute("sodipodi:cx", NULL );
             repr->setAttribute("sodipodi:cy", NULL );
@@ -324,11 +342,10 @@ Inkscape::XML::Node *SPGenericEllipse::write(Inkscape::XML::Document *xml_doc, I
             std::cerr << "SPGenericEllipse::write: unknown type." << std::endl;
     }
 
-    this->set_shape(); // evaluate SPCurve
+    set_shape(); // evaluate SPCurve
 
     SPShape::write(xml_doc, repr, flags);
 
-    // std::cout << "SPGenericEllipse::write: Exit: " << g_quark_to_string(repr->code()) << "\n" << std::endl;
     return repr;
 }
 
@@ -339,8 +356,8 @@ const char *SPGenericEllipse::displayName() const
         case SP_GENERIC_ELLIPSE_UNDEFINED:
         case SP_GENERIC_ELLIPSE_ARC:
 
-            if (this->_isSlice()) {
-                if (this->_closed) {
+            if (_isSlice()) {
+                if (_closed) {
                     return _("Segment");
                 } else {
                     return _("Arc");
@@ -480,11 +497,11 @@ Geom::Affine SPGenericEllipse::set_transform(Geom::Affine const &xform)
     }
 
     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 */
@@ -628,10 +645,10 @@ bool SPGenericEllipse::set_elliptical_path_attribute(Inkscape::XML::Node *repr)
 
 void SPGenericEllipse::position_set(gdouble x, gdouble y, gdouble rx, gdouble ry)
 {
-    this->cx.computed = x;
-    this->cy.computed = y;
-    this->rx.computed = rx;
-    this->ry.computed = ry;
+    this->cx = x;
+    this->cy = y;
+    this->rx = rx;
+    this->ry = ry;
 
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
 
diff --git a/src/sp-item.cpp b/src/sp-item.cpp
index 5d181b80a..8c99e9bcf 100644
--- a/src/sp-item.cpp
+++ b/src/sp-item.cpp
@@ -75,18 +75,6 @@ static SPItemView*          sp_item_view_list_remove(SPItemView     *list,
 
 
 SPItem::SPItem() : SPObject() {
-	this->sensitive = 0;
-	this->clip_ref = NULL;
-	this->avoidRef = NULL;
-	this->_is_evaluated = false;
-	this->stop_paint = 0;
-	this->_evaluated_status = StatusUnknown;
-	this->bbox_valid = 0;
-	this->freeze_stroke_width = false;
-	this->transform_center_x = 0;
-	this->transform_center_y = 0;
-	this->display = NULL;
-	this->mask_ref = NULL;
 
     sensitive = TRUE;
     bbox_valid = FALSE;
@@ -96,12 +84,13 @@ SPItem::SPItem() : SPObject() {
     transform_center_x = 0;
     transform_center_y = 0;
 
+    freeze_stroke_width = false;
+
     _is_evaluated = true;
     _evaluated_status = StatusUnknown;
 
     transform = Geom::identity();
-    doc_bbox = Geom::OptRect();
-    freeze_stroke_width = false;
+    // doc_bbox = Geom::OptRect();
 
     display = NULL;
 
@@ -670,54 +659,56 @@ void SPItem::stroke_ps_ref_changed(SPObject *old_ps, SPObject *ps, SPItem *item)
     }
 }
 
-void SPItem::update(SPCtx* /*ctx*/, guint flags) {
-    SPItem *item = this;
-    SPItem* object = item;
+void SPItem::update(SPCtx* ctx, guint flags) {
 
-//    SPObject::onUpdate(ctx, flags);
+    SPItemCtx const *ictx = reinterpret_cast<SPItemCtx const *>(ctx);
 
-    // any of the modifications defined in sp-object.h might change bbox,
+    // Any of the modifications defined in sp-object.h might change bbox,
     // so we invalidate it unconditionally
-    item->bbox_valid = FALSE;
+    bbox_valid = FALSE;
+
+    viewport = ictx->viewport; // Cache viewport
 
-    if (flags & (SP_OBJECT_CHILD_MODIFIED_FLAG | SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_STYLE_MODIFIED_FLAG)) {
+    if (flags & (SP_OBJECT_CHILD_MODIFIED_FLAG |
+                 SP_OBJECT_MODIFIED_FLAG |
+                 SP_OBJECT_STYLE_MODIFIED_FLAG) ) {
         if (flags & SP_OBJECT_MODIFIED_FLAG) {
-            for (SPItemView *v = item->display; v != NULL; v = v->next) {
-                v->arenaitem->setTransform(item->transform);
+            for (SPItemView *v = display; v != NULL; v = v->next) {
+                v->arenaitem->setTransform(transform);
             }
         }
 
-        SPClipPath *clip_path = item->clip_ref ? item->clip_ref->getObject() : NULL;
-        SPMask *mask = item->mask_ref ? item->mask_ref->getObject() : NULL;
+        SPClipPath *clip_path = clip_ref ? clip_ref->getObject() : NULL;
+        SPMask *mask = mask_ref ? mask_ref->getObject() : NULL;
 
         if ( clip_path || mask ) {
-            Geom::OptRect bbox = item->geometricBounds();
+            Geom::OptRect bbox = geometricBounds();
             if (clip_path) {
-                for (SPItemView *v = item->display; v != NULL; v = v->next) {
+                for (SPItemView *v = display; v != NULL; v = v->next) {
                     clip_path->setBBox(v->arenaitem->key(), bbox);
                 }
             }
             if (mask) {
-                for (SPItemView *v = item->display; v != NULL; v = v->next) {
+                for (SPItemView *v = display; v != NULL; v = v->next) {
                     mask->sp_mask_set_bbox(v->arenaitem->key(), bbox);
                 }
             }
         }
 
         if (flags & SP_OBJECT_STYLE_MODIFIED_FLAG) {
-            for (SPItemView *v = item->display; v != NULL; v = v->next) {
-                v->arenaitem->setOpacity(SP_SCALE24_TO_FLOAT(object->style->opacity.value));
-                v->arenaitem->setAntialiasing(object->style->shape_rendering.computed != SP_CSS_SHAPE_RENDERING_CRISPEDGES);
-                v->arenaitem->setIsolation( object->style->isolation.value );
-                v->arenaitem->setBlendMode( object->style->mix_blend_mode.value );
-                v->arenaitem->setVisible(!item->isHidden());
+            for (SPItemView *v = display; v != NULL; v = v->next) {
+                v->arenaitem->setOpacity(SP_SCALE24_TO_FLOAT(style->opacity.value));
+                v->arenaitem->setAntialiasing(style->shape_rendering.computed != SP_CSS_SHAPE_RENDERING_CRISPEDGES);
+                v->arenaitem->setIsolation( style->isolation.value );
+                v->arenaitem->setBlendMode( style->mix_blend_mode.value );
+                v->arenaitem->setVisible(!isHidden());
             }
         }
     }
     /* Update bounding box in user space, used for filter and objectBoundingBox units */
-    if (item->style->filter.set && item->display) {
-        Geom::OptRect item_bbox = item->geometricBounds();
-        SPItemView *itemview = item->display;
+    if (style->filter.set && display) {
+        Geom::OptRect item_bbox = geometricBounds();
+        SPItemView *itemview = display;
         do {
             if (itemview->arenaitem)
                 itemview->arenaitem->setItemBounds(item_bbox);
@@ -725,8 +716,8 @@ void SPItem::update(SPCtx* /*ctx*/, guint flags) {
     }
 
     // Update libavoid with item geometry (for connector routing).
-    if (item->avoidRef)
-        item->avoidRef->handleSettingChange();
+    if (avoidRef)
+        avoidRef->handleSettingChange();
 }
 
 void SPItem::modified(unsigned int /*flags*/)
diff --git a/src/sp-item.h b/src/sp-item.h
index b9d71c551..bdc6a0ad9 100644
--- a/src/sp-item.h
+++ b/src/sp-item.h
@@ -145,6 +145,7 @@ public:
 
     Geom::Affine transform;
     mutable Geom::OptRect doc_bbox;
+    Geom::Rect viewport;  // Cache viewport information
 
     SPClipPathReference *clip_ref;
     SPMaskReference *mask_ref;
diff --git a/src/sp-rect.cpp b/src/sp-rect.cpp
index 0128c248f..e17d7373c 100644
--- a/src/sp-rect.cpp
+++ b/src/sp-rect.cpp
@@ -52,22 +52,30 @@ void SPRect::build(SPDocument* doc, Inkscape::XML::Node* repr) {
 void SPRect::set(unsigned key, gchar const *value) {
     /* fixme: We need real error processing some time */
 
+    // We must update the SVGLengths immediately or nodes may be misplaced after they are moved.
+    double const w = viewport.width();
+    double const h = viewport.height();
+    double const em = style->font_size.computed;
+    double const ex = em * 0.5;
+
     switch (key) {
         case SP_ATTR_X:
-        	this->x.readOrUnset(value);
-        	this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+            this->x.readOrUnset(value);
+            this->x.update( ex, em, w );
+            this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
             break;
 
         case SP_ATTR_Y:
-        	this->y.readOrUnset(value);
-        	this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+            this->y.readOrUnset(value);
+            this->y.update( ex, em, h );
+            this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
             break;
 
         case SP_ATTR_WIDTH:
             if (!this->width.read(value) || this->width.value < 0.0) {
             	this->width.unset();
             }
-
+            this->width.update( ex, em, w );
             this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
             break;
 
@@ -75,7 +83,7 @@ void SPRect::set(unsigned key, gchar const *value) {
             if (!this->height.read(value) || this->height.value < 0.0) {
             	this->height.unset();
             }
-
+            this->height.update( ex, em, h );
             this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
             break;
 
@@ -83,7 +91,7 @@ void SPRect::set(unsigned key, gchar const *value) {
             if (!this->rx.read(value) || this->rx.value <= 0.0) {
             	this->rx.unset();
             }
-
+            this->rx.update( ex, em, w );
             this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
             break;
 
@@ -91,7 +99,7 @@ void SPRect::set(unsigned key, gchar const *value) {
             if (!this->ry.read(value) || this->ry.value <= 0.0) {
             	this->ry.unset();
             }
-
+            this->ry.update( ex, em, h );
             this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
             break;
 
diff --git a/src/ui/object-edit.cpp b/src/ui/object-edit.cpp
index 04ee13cef..0a6c792dc 100644
--- a/src/ui/object-edit.cpp
+++ b/src/ui/object-edit.cpp
@@ -903,10 +903,10 @@ ArcKnotHolderEntityRX::knot_set(Geom::Point const &p, Geom::Point const &/*origi
 
     Geom::Point const s = snap_knot_position(p, state);
 
-    ge->rx.computed = fabs( ge->cx.computed - s[Geom::X] );
+    ge->rx = fabs( ge->cx.computed - s[Geom::X] );
 
     if ( state & GDK_CONTROL_MASK ) {
-        ge->ry.computed = ge->rx.computed;
+        ge->ry = ge->rx.computed;
     }
 
     item->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
@@ -928,7 +928,7 @@ ArcKnotHolderEntityRX::knot_click(unsigned int state)
     g_assert(ge != NULL);
 
     if (state & GDK_CONTROL_MASK) {
-        ge->ry.computed = ge->rx.computed;
+        ge->ry = ge->rx.computed;
         ge->updateRepr();
     }
 }
@@ -941,10 +941,10 @@ ArcKnotHolderEntityRY::knot_set(Geom::Point const &p, Geom::Point const &/*origi
 
     Geom::Point const s = snap_knot_position(p, state);
 
-    ge->ry.computed = fabs( ge->cy.computed - s[Geom::Y] );
+    ge->ry = fabs( ge->cy.computed - s[Geom::Y] );
 
     if ( state & GDK_CONTROL_MASK ) {
-        ge->rx.computed = ge->ry.computed;
+        ge->rx = ge->ry.computed;
     }
 
     item->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
@@ -966,7 +966,7 @@ ArcKnotHolderEntityRY::knot_click(unsigned int state)
     g_assert(ge != NULL);
 
     if (state & GDK_CONTROL_MASK) {
-        ge->rx.computed = ge->ry.computed;
+        ge->rx = ge->ry.computed;
         ge->updateRepr();
     }
 }