1) Fix absolute scaling in transform dialog

2) Transform dialog now follows the user prefs for geometric vs. visual bounding box

(bzr r10615)

9 files changed, 90 insertions, 59 deletions

diff --git a/src/selection.cpp b/src/selection.cpp
index 92b35bce7..5376311b1 100644
--- a/src/selection.cpp
+++ b/src/selection.cpp
@@ -27,6 +27,7 @@
 #include "selection.h"
 #include "helper/recthull.h"
 #include "xml/repr.h"
+#include "preferences.h"
 
 #include "sp-shape.h"
 #include "sp-path.h"
@@ -390,6 +391,15 @@ Geom::OptRect Selection::visualBounds() const
     return bbox;
 }
 
+Geom::OptRect Selection::preferredBounds() const
+{
+    if (Inkscape::Preferences::get()->getInt("/tools/bounding_box") == 0) {
+        return bounds(SPItem::VISUAL_BBOX);
+    } else {
+        return bounds(SPItem::GEOMETRIC_BBOX);
+    }
+}
+
 Geom::OptRect Selection::documentBounds(SPItem::BBoxType type) const
 {
     Geom::OptRect bbox;
diff --git a/src/selection.h b/src/selection.h
index af0facc3d..39e75685e 100644
--- a/src/selection.h
+++ b/src/selection.h
@@ -247,6 +247,9 @@ public:
     Geom::OptRect bounds(SPItem::BBoxType type) const;
     Geom::OptRect visualBounds() const;
     Geom::OptRect geometricBounds() const;
+    /** @brief Returns either the visual or geometric bounding rectangle of the selection, based on the
+     * preferences specified for the selector tool */
+    Geom::OptRect preferredBounds() const;
 
     /// Returns the bounding rectangle of the selectionin document coordinates.
     Geom::OptRect documentBounds(SPItem::BBoxType type) const;
diff --git a/src/seltrans.cpp b/src/seltrans.cpp
index 19c09902b..20013ab0c 100644
--- a/src/seltrans.cpp
+++ b/src/seltrans.cpp
@@ -329,7 +329,7 @@ void Inkscape::SelTrans::grab(Geom::Point const &p, gdouble x, gdouble y, bool s
         bool emp = m.snapprefs.isTargetSnappable(SNAPTARGET_BBOX_EDGE_MIDPOINT);
         // Preferably we'd use the bbox of each selected item, instead of the bbox of the selection as a whole; for translations
         // this is easy to do, but when snapping the visual bbox while scaling we will have to compensate for the scaling of the
-        // stroke width. (see get_scale_transform_with_stroke()). This however is currently only implemented for a single bbox.
+        // stroke width. (see get_scale_transform_for_stroke()). This however is currently only implemented for a single bbox.
         // That's why we have both _bbox_points_for_translating and _bbox_points.
         getBBoxPoints(selection->bounds(_snap_bbox_type), &_bbox_points, false, c, emp, mp);
         if (((_items.size() > 0) && (_items.size() < 50)) || prefs->getBool("/options/snapclosestonly/value", false)) {
@@ -1560,7 +1560,7 @@ Geom::Point Inkscape::SelTrans::_getGeomHandlePos(Geom::Point const &visual_hand
     }
 
     // Using the Geom::Rect constructor below ensures that "min() < max()", which is important
-    // because this will also hold for _bbox, and which is required for get_scale_transform_with_stroke()
+    // because this will also hold for _bbox, and which is required for get_scale_transform_for_stroke()
     Geom::Rect new_bbox = Geom::Rect(_origin_for_bboxpoints, visual_handle_pos); // new visual bounding box
     // Please note that the new_bbox might in fact be just a single line, for example when stretching (in
     // which case the handle and origin will be aligned vertically or horizontally)
@@ -1569,7 +1569,7 @@ Geom::Point Inkscape::SelTrans::_getGeomHandlePos(Geom::Point const &visual_hand
     // Calculate the absolute affine while taking into account the scaling of the stroke width
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
     bool transform_stroke = prefs->getBool("/options/transform/stroke", true);
-    Geom::Affine abs_affine = get_scale_transform_with_uniform_stroke (*_bbox, _strokewidth, transform_stroke,
+    Geom::Affine abs_affine = get_scale_transform_for_uniform_stroke (*_bbox, _strokewidth, transform_stroke,
                     new_bbox.min()[Geom::X], new_bbox.min()[Geom::Y], new_bbox.max()[Geom::X], new_bbox.max()[Geom::Y]);
 
     // Calculate the scaled geometrical bbox
@@ -1616,7 +1616,7 @@ Geom::Point Inkscape::SelTrans::_calcAbsAffineDefault(Geom::Scale const default_
         strokewidth = _strokewidth;
     }
 
-    _absolute_affine = get_scale_transform_with_uniform_stroke (*_visual_bbox, strokewidth, transform_stroke,
+    _absolute_affine = get_scale_transform_for_uniform_stroke (*_visual_bbox, strokewidth, transform_stroke,
                     new_bbox_min[Geom::X], new_bbox_min[Geom::Y], new_bbox_max[Geom::X], new_bbox_max[Geom::Y]);
 
     // return the new handle position
diff --git a/src/sp-item-transform.cpp b/src/sp-item-transform.cpp
index 311604153..d1fe14f20 100644
--- a/src/sp-item-transform.cpp
+++ b/src/sp-item-transform.cpp
@@ -78,7 +78,7 @@ void sp_item_move_rel(SPItem *item, Geom::Translate const &tr)
  *
  * PS: This function will only return accurate results for the visual bounding box of a selection of one or more objects, all having
  * the same strokewidth. If the stroke width varies from object to object in this selection, then the function
- * get_scale_transform_with_unequal_stroke() should be called instead
+ * get_scale_transform_for_variable_stroke() should be called instead
  *
  * When scaling or stretching an object using the selector, e.g. by dragging the handles or by entering a value, we will
  * need to calculate the affine transformation for the old dimensions to the new dimensions. When using a geometric bounding
@@ -98,7 +98,7 @@ void sp_item_move_rel(SPItem *item, Geom::Translate const &tr)
 */
 
 Geom::Affine
-get_scale_transform_with_uniform_stroke (Geom::Rect const &bbox_visual, gdouble strokewidth, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1)
+get_scale_transform_for_uniform_stroke (Geom::Rect const &bbox_visual, gdouble strokewidth, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1)
 {
     Geom::Affine p2o = Geom::Translate (-bbox_visual.min());
     Geom::Affine o2n = Geom::Translate (x0, y0);
@@ -210,10 +210,10 @@ get_scale_transform_with_uniform_stroke (Geom::Rect const &bbox_visual, gdouble
 /**
  * \brief Calculate the affine transformation required to transform one visual bounding box into another, accounting for a VARIABLE strokewidth
  *
- * Note: Please try to understand get_scale_transform_with_uniform_stroke() first, and read all it's comments carefully. This function
- * (get_scale_transform_with_unequal_stroke) is a bit different because it will allow for a strokewidth that's different for each
+ * Note: Please try to understand get_scale_transform_for_uniform_stroke() first, and read all it's comments carefully. This function
+ * (get_scale_transform_for_variable_stroke) is a bit different because it will allow for a strokewidth that's different for each
  * side of the visual bounding box. Such a situation will arise when transforming the visual bounding box of a selection of objects,
- * each having a different stroke width. In fact this function is a generalized version of get_scale_transform_with_uniform_stroke(), but
+ * each having a different stroke width. In fact this function is a generalized version of get_scale_transform_for_uniform_stroke(), but
  * will not (yet) replace it because it has not been tested as carefully, and because the old function is can serve as an introduction to
  * understand the new one.
  *
@@ -236,7 +236,7 @@ get_scale_transform_with_uniform_stroke (Geom::Rect const &bbox_visual, gdouble
 */
 
 Geom::Affine
-get_scale_transform_with_unequal_stroke (Geom::Rect const &bbox_visual, Geom::Rect const &bbox_geom, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1)
+get_scale_transform_for_variable_stroke (Geom::Rect const &bbox_visual, Geom::Rect const &bbox_geom, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1)
 {
     Geom::Affine p2o = Geom::Translate (-bbox_visual.min());
     Geom::Affine o2n = Geom::Translate (x0, y0);
@@ -393,7 +393,7 @@ get_visual_bbox (Geom::OptRect const &initial_geom_bbox, Geom::Affine const &abs
     Geom::Rect new_visual_bbox = new_geom_bbox; 
     if (initial_strokewidth > 0 && initial_strokewidth < Geom::infinity()) {
         if (transform_stroke) {
-            // scale stroke by: sqrt (((w1-r0)/(w0-r0))*((h1-r0)/(h0-r0))) (for visual bboxes, see get_scale_transform_with_stroke)
+            // scale stroke by: sqrt (((w1-r0)/(w0-r0))*((h1-r0)/(h0-r0))) (for visual bboxes, see get_scale_transform_for_stroke)
             // equals scaling by: sqrt ((w1/w0)*(h1/h0)) for geometrical bboxes            
             // equals scaling by: sqrt (area1/area0) for geometrical bboxes
             gdouble const new_strokewidth = initial_strokewidth * sqrt (new_geom_bbox.area() / initial_geom_bbox->area());
diff --git a/src/sp-item-transform.h b/src/sp-item-transform.h
index 47e0ec0ec..4ea8f976f 100644
--- a/src/sp-item-transform.h
+++ b/src/sp-item-transform.h
@@ -9,8 +9,8 @@ void sp_item_scale_rel (SPItem *item, Geom::Scale const &scale);
 void sp_item_skew_rel (SPItem *item, double skewX, double skewY);
 void sp_item_move_rel(SPItem *item, Geom::Translate const &tr);
 
-Geom::Affine get_scale_transform_with_uniform_stroke (Geom::Rect const &bbox_visual, gdouble strokewidth, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1);
-Geom::Affine get_scale_transform_with_unequal_stroke (Geom::Rect const &bbox_visual, Geom::Rect const &bbox_geom, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1);
+Geom::Affine get_scale_transform_for_uniform_stroke (Geom::Rect const &bbox_visual, gdouble strokewidth, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1);
+Geom::Affine get_scale_transform_for_variable_stroke (Geom::Rect const &bbox_visual, Geom::Rect const &bbox_geom, bool transform_stroke, gdouble x0, gdouble y0, gdouble x1, gdouble y1);
 Geom::Rect get_visual_bbox (Geom::OptRect const &initial_geom_bbox, Geom::Affine const &abs_affine, gdouble const initial_strokewidth, bool const transform_stroke);
 
 
diff --git a/src/sp-item.cpp b/src/sp-item.cpp
index c0c23ba8b..3069dcf73 100644
--- a/src/sp-item.cpp
+++ b/src/sp-item.cpp
@@ -791,6 +791,16 @@ Geom::OptRect SPItem::desktopVisualBounds() const
 {
     return visualBounds(i2dt_affine());
 }
+
+Geom::OptRect SPItem::desktopPreferredBounds() const
+{
+    if (Inkscape::Preferences::get()->getInt("/tools/bounding_box") == 0) {
+        return desktopBounds(SPItem::VISUAL_BBOX);
+    } else {
+        return desktopBounds(SPItem::GEOMETRIC_BBOX);
+    }
+}
+
 Geom::OptRect SPItem::desktopBounds(BBoxType type) const
 {
     if (type == GEOMETRIC_BBOX) {
diff --git a/src/sp-item.h b/src/sp-item.h
index b827f6555..5558d3c62 100644
--- a/src/sp-item.h
+++ b/src/sp-item.h
@@ -178,6 +178,7 @@ public:
     Geom::OptRect documentBounds(BBoxType type) const;
     Geom::OptRect desktopGeometricBounds() const;
     Geom::OptRect desktopVisualBounds() const;
+    Geom::OptRect desktopPreferredBounds() const;
     Geom::OptRect desktopBounds(BBoxType type) const;
 
     unsigned pos_in_parent();
diff --git a/src/ui/dialog/transformation.cpp b/src/ui/dialog/transformation.cpp
index 029a83ea5..be60fac20 100644
--- a/src/ui/dialog/transformation.cpp
+++ b/src/ui/dialog/transformation.cpp
@@ -467,7 +467,7 @@ Transformation::updatePageMove(Inkscape::Selection *selection)
 {
     if (selection && !selection->isEmpty()) {
         if (!_check_move_relative.get_active()) {
-            Geom::OptRect bbox = selection->visualBounds();
+            Geom::OptRect bbox = selection->preferredBounds();
             if (bbox) {
                 double x = bbox->min()[Geom::X];
                 double y = bbox->min()[Geom::Y];
@@ -489,7 +489,7 @@ void
 Transformation::updatePageScale(Inkscape::Selection *selection)
 {
     if (selection && !selection->isEmpty()) {
-        Geom::OptRect bbox = selection->visualBounds();
+        Geom::OptRect bbox = selection->preferredBounds();
         if (bbox) {
             double w = bbox->dimensions()[Geom::X];
             double h = bbox->dimensions()[Geom::Y];
@@ -519,7 +519,7 @@ void
 Transformation::updatePageSkew(Inkscape::Selection *selection)
 {
     if (selection && !selection->isEmpty()) {
-        Geom::OptRect bbox = selection->visualBounds();
+        Geom::OptRect bbox = selection->preferredBounds();
         if (bbox) {
             double w = bbox->dimensions()[Geom::X];
             double h = bbox->dimensions()[Geom::Y];
@@ -616,7 +616,7 @@ Transformation::applyPageMove(Inkscape::Selection *selection)
         if (_check_move_relative.get_active()) {
             sp_selection_move_relative(selection, x, y);
         } else {
-            Geom::OptRect bbox = selection->visualBounds();
+            Geom::OptRect bbox = selection->preferredBounds();
             if (bbox) {
                 sp_selection_move_relative(selection,
                                            x - bbox->min()[Geom::X], y - bbox->min()[Geom::Y]);
@@ -637,7 +637,7 @@ Transformation::applyPageMove(Inkscape::Selection *selection)
                      it != selected.end();
                      ++it)
                 {
-                    Geom::OptRect bbox = (*it)->desktopVisualBounds();
+                    Geom::OptRect bbox = (*it)->desktopPreferredBounds();
                     if (bbox) {
                         sorted.push_back(BBoxSort(*it, *bbox, Geom::X, x > 0? 1. : 0., x > 0? 0. : 1.));
                     }
@@ -661,7 +661,7 @@ Transformation::applyPageMove(Inkscape::Selection *selection)
                      it != selected.end();
                      ++it)
                 {
-                    Geom::OptRect bbox = (*it)->desktopVisualBounds();
+                    Geom::OptRect bbox = (*it)->desktopPreferredBounds();
                     if (bbox) {
                         sorted.push_back(BBoxSort(*it, *bbox, Geom::Y, y > 0? 1. : 0., y > 0? 0. : 1.));
                     }
@@ -680,7 +680,7 @@ Transformation::applyPageMove(Inkscape::Selection *selection)
                 }
             }
         } else {
-            Geom::OptRect bbox = selection->visualBounds();
+            Geom::OptRect bbox = selection->preferredBounds();
             if (bbox) {
                 sp_selection_move_relative(selection,
                                            x - bbox->min()[Geom::X], y - bbox->min()[Geom::Y]);
@@ -699,49 +699,54 @@ Transformation::applyPageScale(Inkscape::Selection *selection)
     double scaleY = _scalar_scale_vertical.getValue("px");
 
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
+    int transform_stroke = prefs->getBool("/options/transform/stroke", true) ? 1 : 0;
     if (prefs->getBool("/dialogs/transformation/applyseparately")) {
         for (GSList const *l = selection->itemList(); l != NULL; l = l->next) {
             SPItem *item = SP_ITEM(l->data);
-            Geom::Scale scale (0,0);
-            // the values are increments!
-            if (_units_scale.isAbsolute()) {
-                Geom::OptRect bbox = item->desktopVisualBounds();
-                if (bbox) {
-                    double new_width = scaleX;
-                    if (fabs(new_width) < 1e-6) new_width = 1e-6; // not 0, as this would result in a nasty no-bbox object
-                    double new_height = scaleY;
-                    if (fabs(new_height) < 1e-6) new_height = 1e-6;
-                    scale = Geom::Scale(new_width / bbox->dimensions()[Geom::X], new_height / bbox->dimensions()[Geom::Y]);
-                }
-            } else {
+            Geom::OptRect bbox_pref = item->desktopPreferredBounds();
+            Geom::OptRect bbox_geom = item->desktopGeometricBounds();
+            if (bbox_pref && bbox_geom) {
                 double new_width = scaleX;
-                if (fabs(new_width) < 1e-6) new_width = 1e-6;
                 double new_height = scaleY;
+                // the values are increments!
+                if (!_units_scale.isAbsolute()) { // Relative scaling, i.e in percent
+                    new_width = scaleX/100 * bbox_pref->width();
+                    new_height = scaleY/100  * bbox_pref->height();
+                }
+                if (fabs(new_width) < 1e-6) new_width = 1e-6; // not 0, as this would result in a nasty no-bbox object
                 if (fabs(new_height) < 1e-6) new_height = 1e-6;
-                scale = Geom::Scale(new_width / 100.0, new_height / 100.0);
+
+                double x0 = bbox_pref->midpoint()[Geom::X] - new_width/2;
+                double y0 = bbox_pref->midpoint()[Geom::Y] - new_height/2;
+                double x1 = bbox_pref->midpoint()[Geom::X] + new_width/2;
+                double y1 = bbox_pref->midpoint()[Geom::Y] + new_height/2;
+
+                Geom::Affine scaler = get_scale_transform_for_variable_stroke (*bbox_pref, *bbox_geom, transform_stroke, x0, y0, x1, y1);
+                item->set_i2d_affine(item->i2dt_affine() * scaler);
+                item->doWriteTransform(item->getRepr(), item->transform);
             }
-            sp_item_scale_rel (item, scale);
         }
     } else {
-        Geom::OptRect bbox = selection->visualBounds();
-        if (bbox) {
-            Geom::Point center(bbox->midpoint()); // use rotation center?
-            Geom::Scale scale (0,0);
+        Geom::OptRect bbox_pref = selection->preferredBounds();
+        Geom::OptRect bbox_geom = selection->geometricBounds();
+        if (bbox_pref && bbox_geom) {
             // the values are increments!
-            if (_units_scale.isAbsolute()) {
-                double new_width = scaleX;
-                if (fabs(new_width) < 1e-6) new_width = 1e-6;
-                double new_height = scaleY;
-                if (fabs(new_height) < 1e-6) new_height = 1e-6;
-                scale = Geom::Scale(new_width / bbox->dimensions()[Geom::X], new_height / bbox->dimensions()[Geom::Y]);
-            } else {
-                double new_width = scaleX;
-                if (fabs(new_width) < 1e-6) new_width = 1e-6;
-                double new_height = scaleY;
-                if (fabs(new_height) < 1e-6) new_height = 1e-6;
-                scale = Geom::Scale(new_width / 100.0, new_height / 100.0);
+            double new_width = scaleX;
+            double new_height = scaleY;
+            if (!_units_scale.isAbsolute()) { // Relative scaling, i.e in percent
+                new_width = scaleX/100 * bbox_pref->width();
+                new_height = scaleY/100 * bbox_pref->height();
             }
-            sp_selection_scale_relative(selection, center, scale);
+            if (fabs(new_width) < 1e-6) new_width = 1e-6;
+            if (fabs(new_height) < 1e-6) new_height = 1e-6;
+
+            double x0 = bbox_pref->midpoint()[Geom::X] - new_width/2;
+            double y0 = bbox_pref->midpoint()[Geom::Y] - new_height/2;
+            double x1 = bbox_pref->midpoint()[Geom::X] + new_width/2;
+            double y1 = bbox_pref->midpoint()[Geom::Y] + new_height/2;
+            Geom::Affine scaler = get_scale_transform_for_variable_stroke (*bbox_pref, *bbox_geom, transform_stroke, x0, y0, x1, y1);
+
+            sp_selection_apply_affine(selection, scaler);
         }
     }
 
@@ -792,7 +797,7 @@ Transformation::applyPageSkew(Inkscape::Selection *selection)
             } else { // absolute displacement
                 double skewX = _scalar_skew_horizontal.getValue("px");
                 double skewY = _scalar_skew_vertical.getValue("px");
-                Geom::OptRect bbox = item->desktopVisualBounds();
+                Geom::OptRect bbox = item->desktopPreferredBounds();
                 if (bbox) {
                     double width = bbox->dimensions()[Geom::X];
                     double height = bbox->dimensions()[Geom::Y];
@@ -801,7 +806,7 @@ Transformation::applyPageSkew(Inkscape::Selection *selection)
             }
         }
     } else { // transform whole selection
-        Geom::OptRect bbox = selection->visualBounds();
+        Geom::OptRect bbox = selection->preferredBounds();
         boost::optional<Geom::Point> center = selection->center();
 
         if ( bbox && center ) {
@@ -886,7 +891,7 @@ Transformation::onMoveRelativeToggled()
 
     //g_message("onMoveRelativeToggled: %f, %f px\n", x, y);
 
-    Geom::OptRect bbox = selection->visualBounds();
+    Geom::OptRect bbox = selection->preferredBounds();
 
     if (bbox) {
         if (_check_move_relative.get_active()) {
@@ -1026,7 +1031,7 @@ Transformation::onClear()
             _scalar_move_horizontal.setValue(0);
             _scalar_move_vertical.setValue(0);
         } else {
-            Geom::OptRect bbox = selection->visualBounds();
+            Geom::OptRect bbox = selection->preferredBounds();
             if (bbox) {
                 _scalar_move_horizontal.setValue(bbox->min()[Geom::X], "px");
                 _scalar_move_vertical.setValue(bbox->min()[Geom::Y], "px");
diff --git a/src/widgets/select-toolbar.cpp b/src/widgets/select-toolbar.cpp
index 7b8b54fee..38346ce56 100644
--- a/src/widgets/select-toolbar.cpp
+++ b/src/widgets/select-toolbar.cpp
@@ -248,11 +248,13 @@ sp_object_layout_any_value_changed(GtkAdjustment *adj, SPWidget *spw)
 
         Geom::Affine scaler;
         if (bbox_type == SPItem::VISUAL_BBOX) {
-            scaler = get_scale_transform_with_unequal_stroke (*bbox_vis, *bbox_geom, transform_stroke, x0, y0, x1, y1);
+            scaler = get_scale_transform_for_variable_stroke (*bbox_vis, *bbox_geom, transform_stroke, x0, y0, x1, y1);
         } else {
-            // get_scale_transform_with_stroke() is intended for visual bounding boxes, not geometrical ones!
+            // 1) We could have use the newer get_scale_transform_for_variable_stroke() here, but to avoid regressions
+            // we'll just use the old get_scale_transform_for_uniform_stroke() for now.
+            // 2) get_scale_transform_for_uniform_stroke() is intended for visual bounding boxes, not geometrical ones!
             // we'll trick it into using a geometric bounding box though, by setting the stroke width to zero
-            scaler = get_scale_transform_with_uniform_stroke (*bbox_geom, 0, false, x0, y0, x1, y1);
+            scaler = get_scale_transform_for_uniform_stroke (*bbox_geom, 0, false, x0, y0, x1, y1);
         }
 
         sp_selection_apply_affine(selection, scaler);