Improvemets to GeomPathstroke and Offset LPE

6 files changed, 210 insertions, 215 deletions

diff --git a/src/helper/geom-pathstroke.cpp b/src/helper/geom-pathstroke.cpp
index ead8bfb3b..e996acb40 100644
--- a/src/helper/geom-pathstroke.cpp
+++ b/src/helper/geom-pathstroke.cpp
@@ -657,46 +657,6 @@ void extrapolate_join_alt2(join_data jd) { extrapolate_join_internal(jd, 2); }
 void extrapolate_join_alt3(join_data jd) { extrapolate_join_internal(jd, 3); }
 
 
-void join_inside(join_data jd)
-{
-    Geom::Path &res = jd.res;
-    Geom::Path const& temp = jd.outgoing;
-    Geom::Crossings cross = Geom::crossings(res, temp);
-
-    int solution = -1; // lol, really hope there aren't more than INT_MAX crossings
-    if (cross.size() == 1) solution = 0;
-    else if (cross.size() > 1) {
-        // I am not sure how well this will work -- we pick the join node closest
-        // to the cross point of the paths
-        /*Geom::Point original = res.finalPoint()+Geom::rot90(jd.in_tang)*jd.width;
-        Geom::Coord trial = Geom::L2(res.pointAt(cross[0].ta)-original);
-        solution = 0;
-        for (size_t i = 1; i < cross.size(); ++i) {
-            //printf("Trying %d\n", i);
-            Geom::Coord test = Geom::L2(res.pointAt(cross[i].ta)-original);
-            if (test < trial) {
-                trial = test;
-                solution = i;
-                //printf("Found improved solution: %f\n", trial);
-            }
-        }*/
-    }
-
-    if (solution != -1) {
-        Geom::Path d1 = res.portion(0., cross[solution].ta);
-        Geom::Path d2 = temp.portion(cross[solution].tb, temp.size());
-
-        // Watch for bugs in 2geom crossing regarding severe inflection points
-        res.clear();
-        res.append(d1);
-        res.setFinal(d2.initialPoint());
-        res.append(d2);
-    } else {
-        res.appendNew<Geom::LineSegment>(temp.initialPoint());
-        res.append(temp);
-    }
-}
-
 void tangents(Geom::Point tang[2], Geom::Curve const& incoming, Geom::Curve const& outgoing)
 {
     Geom::Point tang1 = Geom::unitTangentAt(reverse(incoming.toSBasis()), 0.);
@@ -1103,19 +1063,6 @@ Geom::Path half_outline(
     const size_t k = (are_near(closingline.initialPoint(), closingline.finalPoint()) && input.closed() )
             ?input.size_open():input.size_default();
     
-    size_t outside = 0;
-    for (size_t u = 0; u < k; u += 2) {
-        if (u != 0) {
-            tangents(tang, input[u-1], input[u]);
-            if (Geom::cross(tang[0], tang[1]) > 0) {
-                outside --;
-            } else {
-                outside ++;
-            }
-        }
-    }
-    bool on_outside = outside >= 0;
-
     for (size_t u = 0; u < k; u += 2) {
         temp.clear();
 
@@ -1126,7 +1073,7 @@ Geom::Path half_outline(
             res.append(temp);
         } else {
             tangents(tang, input[u-1], input[u]);
-            outline_join(res, temp, tang[0], tang[1], width, miter, on_outside, join);
+            outline_join(res, temp, tang[0], tang[1], width, miter, join);
         }
 
         // odd number of paths
@@ -1134,7 +1081,7 @@ Geom::Path half_outline(
             temp.clear();
             offset_curve(temp, &input[u+1], width, tolerance);
             tangents(tang, input[u], input[u+1]);
-            outline_join(res, temp, tang[0], tang[1], width, miter, on_outside, join);
+            outline_join(res, temp, tang[0], tang[1], width, miter, join);
         }
     }
     if (input.closed()) {
@@ -1145,7 +1092,7 @@ Geom::Path half_outline(
         Geom::Path temp2;
         temp2.append(c2);
         tangents(tang, input.back(), input.front());
-        outline_join(temp, temp2, tang[0], tang[1], width, miter, on_outside, join);
+        outline_join(temp, temp2, tang[0], tang[1], width, miter, join);
         res.erase(res.begin());
         res.erase_last();
         res.append(temp);
@@ -1154,7 +1101,7 @@ Geom::Path half_outline(
     return res;
 }
 
-void outline_join(Geom::Path &res, Geom::Path const& temp, Geom::Point in_tang, Geom::Point out_tang, double width, double miter, bool on_outside, Inkscape::LineJoinType join)
+void outline_join(Geom::Path &res, Geom::Path const& temp, Geom::Point in_tang, Geom::Point out_tang, double width, double miter, Inkscape::LineJoinType join)
 {
     if (res.size() == 0 || temp.size() == 0)
         return;
@@ -1167,39 +1114,37 @@ void outline_join(Geom::Path &res, Geom::Path const& temp, Geom::Point in_tang,
     }
 
     join_data jd(res, temp, in_tang, out_tang, miter, width);
-
-    if (on_outside) {
-        join_func *jf;
-        switch (join) {
-            case Inkscape::JOIN_BEVEL:
-                jf = &bevel_join;
-                break;
-            case Inkscape::JOIN_ROUND:
-                jf = &round_join;
-                break;
-            case Inkscape::JOIN_EXTRAPOLATE:
-                jf = &extrapolate_join;
-                break;
-            case Inkscape::JOIN_EXTRAPOLATE1:
-                jf = &extrapolate_join_alt1;
-                break;
-            case Inkscape::JOIN_EXTRAPOLATE2:
-                jf = &extrapolate_join_alt2;
-                break;
-            case Inkscape::JOIN_EXTRAPOLATE3:
-                jf = &extrapolate_join_alt3;
-                break;
-            case Inkscape::JOIN_MITER_CLIP:
-                jf = &miter_clip_join;
-                break;
-            default:
-                jf = &miter_join;
-        }
-        jf(jd);
-    } else {
-        join_inside(jd);
+    if (!(Geom::cross(in_tang, out_tang) > 0)) {
+        join = Inkscape::JOIN_ROUND;
     }
-}
+    join_func *jf;
+    switch (join) {
+        case Inkscape::JOIN_BEVEL:
+            jf = &bevel_join;
+            break;
+        case Inkscape::JOIN_ROUND:
+            jf = &round_join;
+            break;
+        case Inkscape::JOIN_EXTRAPOLATE:
+            jf = &extrapolate_join;
+            break;
+        case Inkscape::JOIN_EXTRAPOLATE1:
+            jf = &extrapolate_join_alt1;
+            break;
+        case Inkscape::JOIN_EXTRAPOLATE2:
+            jf = &extrapolate_join_alt2;
+            break;
+        case Inkscape::JOIN_EXTRAPOLATE3:
+            jf = &extrapolate_join_alt3;
+            break;
+        case Inkscape::JOIN_MITER_CLIP:
+            jf = &miter_clip_join;
+            break;
+        default:
+            jf = &miter_join;
+    }
+    jf(jd);
+ }
 
 } // namespace Inkscape
 
diff --git a/src/helper/geom-pathstroke.h b/src/helper/geom-pathstroke.h
index 8224b59b8..73d35b402 100644
--- a/src/helper/geom-pathstroke.h
+++ b/src/helper/geom-pathstroke.h
@@ -91,7 +91,7 @@ Geom::Path half_outline(
  * @param[in]    miter
  * @param[in]    join
  */
-void outline_join(Geom::Path &res, Geom::Path const& outgoing, Geom::Point in_tang, Geom::Point out_tang, double width, double miter, bool on_outside, LineJoinType join);
+void outline_join(Geom::Path &res, Geom::Path const& outgoing, Geom::Point in_tang, Geom::Point out_tang, double width, double miter, LineJoinType join);
 
 } // namespace Inkscape
 
diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp
index 82d89cc28..fa3acfede 100644
--- a/src/live_effects/effect.cpp
+++ b/src/live_effects/effect.cpp
@@ -657,7 +657,7 @@ const EnumEffectData<EffectType> LPETypeData[] = {
         , N_("Offset the path, optionally keeping cusp corners cusp.") //description
         , true  //on_path
         , true  //on_shape
-        , false //on_group
+        , true //on_group
         , false //on_image
         , false //on_text
         , false //experimental
diff --git a/src/live_effects/lpe-offset.cpp b/src/live_effects/lpe-offset.cpp
index cc03891d1..8ae89bca4 100644
--- a/src/live_effects/lpe-offset.cpp
+++ b/src/live_effects/lpe-offset.cpp
@@ -30,6 +30,7 @@
 #include "object/sp-shape.h"
 #include "knot-holder-entity.h"
 #include "knotholder.h"
+#include "util/units.h"
 #include "knot.h"
 #include <algorithm>
 // this is only to flatten nonzero fillrule
@@ -73,6 +74,7 @@ static const Util::EnumDataConverter<unsigned> JoinTypeConverter(JoinTypeData, s
 
 LPEOffset::LPEOffset(LivePathEffectObject *lpeobject) :
     Effect(lpeobject),
+    unit(_("Unit"), _("Unit of measurement"), "unit", &wr, this, "mm"),
     offset(_("Offset:"), _("Offset)"), "offset", &wr, this, 0.0),
     linejoin_type(_("Join:"), _("Determines the shape of the path's corners"),  "linejoin_type", JoinTypeConverter, &wr, this, JOIN_ROUND),
     miter_limit(_("Miter limit:"), _("Maximum length of the miter join (in units of stroke width)"), "miter_limit", &wr, this, 4.0),
@@ -81,6 +83,7 @@ LPEOffset::LPEOffset(LivePathEffectObject *lpeobject) :
 {
     show_orig_path = true;
     registerParameter(&linejoin_type);
+    registerParameter(&unit);
     registerParameter(&offset);
     registerParameter(&miter_limit);
     registerParameter(&attempt_force_join);
@@ -88,7 +91,6 @@ LPEOffset::LPEOffset(LivePathEffectObject *lpeobject) :
     offset.param_set_increments(0.1, 0.1);
     offset.param_set_digits(4);
     offset_pt = Geom::Point(Geom::infinity(), Geom::infinity());
-    origin = Geom::Point();
     evenodd = true;
     _knot_entity = nullptr;
     _provides_knotholder_entities = true;
@@ -125,15 +127,42 @@ sp_flatten(Geom::PathVector &pathvector, bool evenodd)
     pathvector  = sp_svg_read_pathv(res_d);
 }
 
+Geom::Point 
+LPEOffset::get_nearest_point(Geom::PathVector pathv, Geom::Point point)  const
+{
+    Geom::Point res = Geom::Point(Geom::infinity(), Geom::infinity());
+    boost::optional< Geom::PathVectorTime > pathvectortime = pathv.nearestTime(point);
+    if (pathvectortime) {
+        Geom::PathTime pathtime = pathvectortime->asPathTime();
+        res = pathv[(*pathvectortime).path_index].pointAt(pathtime.curve_index + pathtime.t);
+    }
+    return res;
+}
+
+Geom::Point 
+LPEOffset::get_default_point(Geom::PathVector pathv)  const
+{
+    if (SP_IS_GROUP(sp_lpe_item)) {
+        return Geom::Point(Geom::infinity(), Geom::infinity());
+    }
+    Geom::Point origin = Geom::Point(Geom::infinity(), Geom::infinity());
+    Geom::OptRect bbox = pathv.boundsFast();
+    if (bbox) {
+        origin = Geom::Point((*bbox).midpoint()[Geom::X],(*bbox).top());
+        origin = get_nearest_point(pathv, origin);
+    }
+    return origin;
+}
+
 double
-LPEOffset::sp_get_offset(Geom::Point &origin)
+LPEOffset::sp_get_offset(Geom::Point origin)
 {
     int winding_value = filled_rule_pathv.winding(origin); 
     bool inset = false;
     if (winding_value % 2 != 0) {
         inset = true;
     }
-    double offset = 0;
+    double ret_offset = 0;
     boost::optional< Geom::PathVectorTime > pathvectortime = filled_rule_pathv.nearestTime(origin);
     Geom::Point nearest = origin;
     double distance = 0;
@@ -141,33 +170,67 @@ LPEOffset::sp_get_offset(Geom::Point &origin)
         Geom::PathTime pathtime = pathvectortime->asPathTime();
         nearest = filled_rule_pathv[(*pathvectortime).path_index].pointAt(pathtime.curve_index + pathtime.t);
     }
-    offset = Geom::distance(origin, nearest);
+    ret_offset = Geom::distance(origin, nearest);
     if (inset) {
-        offset *= -1;
+        ret_offset *= -1;
     }
-    return offset;
+    return Inkscape::Util::Quantity::convert(ret_offset, display_unit.c_str(), unit.get_abbreviation());
 }
 
 void
-LPEOffset::doOnApply(SPLPEItem const* lpeitem)
+LPEOffset::doBeforeEffect (SPLPEItem const* lpeitem)
 {
-    if (!SP_IS_SHAPE(lpeitem)) {
-        g_warning("LPE measure line can only be applied to shapes (not groups).");
-        SPLPEItem * item = const_cast<SPLPEItem*>(lpeitem);
-        item->removeCurrentPathEffect(false);
+    original_bbox(lpeitem);
+    SPDocument * document = SP_ACTIVE_DOCUMENT;
+    if (!document) {
         return;
     }
+    display_unit = document->getDisplayUnit()->abbr.c_str();
 }
 
-void
-LPEOffset::doBeforeEffect (SPLPEItem const* lpeitem)
+int offset_winding(Geom::PathVector pathvector, Geom::Path path)
 {
-    if (!hp.empty()) {
-        hp.clear();
+    int wind = 0;
+    Geom::Point p = path.initialPoint();
+    for (auto i:pathvector) {
+        if (i == path)  continue;
+        if (!i.boundsFast().contains(p)) continue;
+        wind += i.winding(p);
+    }
+    return wind;
+}
+
+Geom::Path 
+sp_get_outer(Geom::Path path) {
+    Geom::PathVector pathv;
+    pathv.push_back(path);
+    sp_flatten(pathv, true);
+    Geom::Path out = pathv[0];
+    Geom::OptRect bounds = out.boundsFast();
+    for (auto path_child:pathv) {
+        Geom::OptRect path_bounds = path_child.boundsFast();
+        if (path_bounds.contains(bounds)) {
+            bounds = path_bounds;
+            out = path_child;
+        }
+    }
+    pathv.clear();
+    return out;
+}
+
+Geom::PathVector 
+LPEOffset::doEffect_path(Geom::PathVector const & path_in)
+{
+    Geom::PathVector original_pathv = pathv_to_linear_and_cubic_beziers(path_in);
+    filled_rule_pathv = original_pathv;
+    sp_flatten(filled_rule_pathv, evenodd);
+    if (offset == 0.0) {
+        return path_in;
+    }
+    SPItem * item = SP_ITEM(current_shape);
+    if (!item) {
+        return path_in;
     }
-    original_bbox(lpeitem);
-    origin = Geom::Point(boundingbox_X.middle(), boundingbox_Y.min());
-    SPItem * item = SP_ITEM(sp_lpe_item);
     SPCSSAttr *css;
     const gchar *val;
     css = sp_repr_css_attr (item->getRepr() , "style");
@@ -187,31 +250,6 @@ LPEOffset::doBeforeEffect (SPLPEItem const* lpeitem)
         }
         evenodd = true;
     }
-    original_pathv = pathv_to_linear_and_cubic_beziers(pathvector_before_effect);
-    filled_rule_pathv = original_pathv;
-    sp_flatten(filled_rule_pathv, evenodd);
-    origin = offset_pt;
-}
-
-int offset_winding(Geom::PathVector pathvector, Geom::Path path)
-{
-    int wind = 0;
-    Geom::Point p = path.initialPoint();
-    for (auto i:pathvector) {
-        if (i == path)  continue;
-        if (!i.boundsFast().contains(p)) continue;
-        wind += i.winding(p);
-    }
-    return wind;
-}
-
-Geom::PathVector 
-LPEOffset::doEffect_path(Geom::PathVector const & path_in)
-{
-    if (offset == 0.0) {
-        offset_pt = original_pathv[0].initialPoint();
-        return original_pathv;
-    }
     Geom::PathVector work;
     Geom::PathVector ret;
     Geom::PathVector ret_outline;
@@ -223,7 +261,7 @@ LPEOffset::doEffect_path(Geom::PathVector const & path_in)
         }
         bool added = false;
         for (auto cross:Geom::self_crossings(original)) {
-            if (cross.ta != cross.tb) {
+            if (!Geom::are_near(cross.ta, cross.tb)) {
                 Geom::PathVector tmp;
                 tmp.push_back(original);
                 sp_flatten(tmp, false);
@@ -237,7 +275,6 @@ LPEOffset::doEffect_path(Geom::PathVector const & path_in)
         } 
     }
     for (Geom::PathVector::const_iterator path_it = work.begin(); path_it != work.end(); ++path_it) {
-        
         Geom::Path original = (*path_it);
         if (original.empty()) {
             continue;
@@ -245,75 +282,98 @@ LPEOffset::doEffect_path(Geom::PathVector const & path_in)
         int wdg = offset_winding(work, original);
         bool path_inside = wdg % 2 != 0;
         double gap = path_inside ? 0.05 :-0.05;
-        Geom::PathVector outline_gap = Inkscape::outline(original, std::abs(offset) * 2 + gap , 
-                               (attempt_force_join ? std::numeric_limits<double>::max() : miter_limit),
+        bool closed = original.closed();
+        double to_offset = Inkscape::Util::Quantity::convert(std::abs(offset), unit.get_abbreviation(), display_unit.c_str());
+        Geom::Path with_dir = half_outline(original, 
+                                to_offset,
+                                (attempt_force_join ? std::numeric_limits<double>::max() : miter_limit),
                                 static_cast<LineJoinType>(linejoin_type.get_value()),
                                 static_cast<LineCapType>(BUTT_FLAT));
-        Geom::PathVector outline = Inkscape::outline(original, std::abs(offset) * 2 , 
-                               (attempt_force_join ? std::numeric_limits<double>::max() : miter_limit),
+        Geom::Path against_dir = half_outline(original.reversed(), 
+                                to_offset,
+                                (attempt_force_join ? std::numeric_limits<double>::max() : miter_limit),
                                 static_cast<LineJoinType>(linejoin_type.get_value()),
                                 static_cast<LineCapType>(BUTT_FLAT));
-        if (outline.size() < 2) {
-            continue;
+        Geom::Path with_dir_gap = half_outline(original, 
+                                to_offset + gap,
+                                (attempt_force_join ? std::numeric_limits<double>::max() : miter_limit),
+                                static_cast<LineJoinType>(linejoin_type.get_value()),
+                                static_cast<LineCapType>(BUTT_FLAT));
+        Geom::Path against_dir_gap = half_outline(original.reversed(), 
+                                to_offset + gap,
+                                (attempt_force_join ? std::numeric_limits<double>::max() : miter_limit),
+                                static_cast<LineJoinType>(linejoin_type.get_value()),
+                                static_cast<LineCapType>(BUTT_FLAT));
+    
+        Geom::PathVector tmp;
+        Geom::PathVector outline;
+        Geom::OptRect original_bounds = original.boundsFast();
+        Geom::Path big;
+        Geom::Path big_gap;
+        Geom::Path small;
+        Geom::Path small_gap;
+        Geom::OptRect big_bounds;
+        Geom::OptRect small_bounds;
+        Geom::OptRect big_bounds_gap;
+        Geom::OptRect small_bounds_gap;
+        Geom::OptRect against_dir_bounds = against_dir.boundsFast();
+        Geom::OptRect against_dir_bounds_gap = against_dir_gap.boundsFast();
+        Geom::OptRect with_dir_bounds = with_dir.boundsFast();
+        Geom::OptRect with_dir_bounds_gap = with_dir_gap.boundsFast();
+        if (against_dir_bounds.contains(with_dir_bounds)) {
+            big  = against_dir;
+            big_gap = against_dir_gap;
+            small = with_dir;
+            small_gap = with_dir_gap;
+            big_bounds = against_dir_bounds;
+            small_bounds = with_dir_bounds;
+            big_bounds_gap = against_dir_bounds_gap;
+            small_bounds_gap = with_dir_bounds_gap;
+        } else {
+            big = with_dir;
+            big_gap   = with_dir_gap;
+            small = against_dir;
+            small_gap = against_dir_gap;
+            big_bounds = with_dir_bounds;
+            small_bounds = against_dir_bounds;
+            big_bounds_gap = with_dir_bounds_gap;
+            small_bounds_gap = against_dir_bounds_gap;
         }
-        Geom::PathVector big_gap_ret;
-        Geom::PathVector big;
-        big.push_back(outline[0]);
-        Geom::PathVector big_gap;
-        big_gap.push_back(outline_gap[0]);
-        Geom::PathVector small;
-        small.insert(small.end(), outline.begin() + 1, outline.end());
-        Geom::PathVector small_gap;
-        small_gap.insert(small_gap.end(), outline_gap.begin() + 1, outline_gap.end());
-        Geom::OptRect big_bounds = big.boundsFast();
-        Geom::OptRect small_bounds = small.boundsFast();
-        bool reversed = small_bounds.contains(big_bounds);
-        if (reversed) {
-            big.clear();
-            big.insert(big.end(), outline.begin() + 1, outline.end());
-            big_gap.clear();
-            big_gap.insert(big_gap.end(), outline_gap.begin() + 1, outline_gap.end());
-            small.clear();
-            small.push_back(outline[0]);
-            small_gap.clear();
-            small_gap.push_back(outline[0]);
+        sp_get_outer(big);
+        sp_get_outer(big_gap);
+        outline.push_back(with_dir);
+        outline.push_back(against_dir);
+        tmp.clear();
+        if (!closed) {
+            if (offset < 0) {
+                ret.push_back(small);
+                return ret;
+            } else {
+                ret.push_back(big);
+                return ret;
+            }
         }
-        big_bounds = big.boundsFast();
-        small_bounds = small.boundsFast();
-        Geom::OptRect big_gap_bounds = big.boundsFast();
-        Geom::OptRect small_gap_bounds = small.boundsFast();
-        Geom::OptRect original_bounds = original.boundsFast();
-        bool fix_reverse = ((*original_bounds).width() + (*original_bounds).height()) / 2.0  > std::abs(offset) * 2;
+        bool fix_reverse = ((*original_bounds).width() + (*original_bounds).height()) / 2.0  > to_offset * 2;
         if (offset < 0) {
             if (fix_reverse) {
-                big_gap_ret.insert(big_gap_ret.end(), big_gap.begin(), big_gap.end());
+                tmp.push_back(big_gap);
             }
         } else {
             if (path_inside) {
                 if (fix_reverse) {
-                    big_gap_ret.insert(big_gap_ret.end(),small_gap.begin(), small_gap.end());
+                    tmp.push_back(small);
                 }
             } else {
-                big_gap_ret.insert(big_gap_ret.end(), big_gap.begin(), big_gap.end());
+                tmp.push_back(big);
             } 
         }
-
         if (path_inside) {
-            Geom::OptRect bounds;
-            Geom::Path tmp = outline[0];
-            for (auto path:outline) {
-                Geom::OptRect path_bounds = path.boundsFast();
-                if (path_bounds.contains(bounds)) {
-                    tmp = path;
-                }
-                bounds = path_bounds;
-            }
             outline.clear();
-            outline.push_back(tmp);
+            outline.push_back(big);
         } else {
             sp_flatten(outline, false);
         }
-        ret.insert(ret.end(), big_gap_ret.begin(),big_gap_ret.end());
+        ret.insert(ret.end(), tmp.begin(), tmp.end());
         ret_outline.insert(ret_outline.end(), outline.begin(), outline.end());
     }
     sp_flatten(ret_outline, false);
@@ -324,26 +384,12 @@ LPEOffset::doEffect_path(Geom::PathVector const & path_in)
         }
     }
     sp_flatten(ret, false);
+    if (offset_pt == Geom::Point(Geom::infinity(), Geom::infinity())) {
+        offset_pt = get_default_point(ret);
+    }
     return ret;
 }
 
-void
-LPEOffset::addCanvasIndicators(SPLPEItem const */*lpeitem*/, std::vector<Geom::PathVector> &hp_vec)
-{
-    hp_vec.push_back(hp);
-}
-
-void
-LPEOffset::drawHandle(Geom::Point p)
-{
-    double r = 3.0;
-    char const * svgd;
-    svgd = "M 0.7,0.35 A 0.35,0.35 0 0 1 0.35,0.7 0.35,0.35 0 0 1 0,0.35 0.35,0.35 0 0 1 0.35,0 0.35,0.35 0 0 1 0.7,0.35 Z";
-    Geom::PathVector pathv = sp_svg_read_pathv(svgd);
-    pathv *= Geom::Scale(r) * Geom::Translate(p - Geom::Point(0.35*r,0.35*r));
-    hp.push_back(pathv[0]);
-}
-
 void LPEOffset::addKnotHolderEntities(KnotHolder *knotholder, SPItem *item)
 {
     _knot_entity = new OfS::KnotHolderEntityOffsetPoint(this);
@@ -362,6 +408,9 @@ void KnotHolderEntityOffsetPoint::knot_ungrabbed(Geom::Point const &p, Geom::Poi
 void KnotHolderEntityOffsetPoint::knot_set(Geom::Point const &p, Geom::Point const& /*origin*/, guint state)
 {
     using namespace Geom;
+    if (SP_IS_GROUP(item)) {
+        return;
+    }
     LPEOffset* lpe = dynamic_cast<LPEOffset *>(_effect);
     Geom::Point s = snap_knot_position(p, state);
     double offset = lpe->sp_get_offset(s);
@@ -375,13 +424,16 @@ void KnotHolderEntityOffsetPoint::knot_set(Geom::Point const &p, Geom::Point con
 
 Geom::Point KnotHolderEntityOffsetPoint::knot_get() const
 {
+    if (SP_IS_GROUP(item)) {
+        return Geom::Point(Geom::infinity(), Geom::infinity());
+    }
     LPEOffset const * lpe = dynamic_cast<LPEOffset const*> (_effect);
+    Geom::Point nearest = lpe->offset_pt;
     if (lpe->offset_pt == Geom::Point(Geom::infinity(), Geom::infinity())) {
-        if(boost::optional< Geom::Point > offset_point = SP_SHAPE(item)->getCurve()->first_point()) {
-            return *offset_point;
-        }
+        Geom::PathVector out = SP_SHAPE(item)->getCurve()->get_pathvector();
+        nearest = lpe->get_default_point(out);
     }
-    return lpe->offset_pt;
+    return nearest;
 }
 
 } // namespace OfS
diff --git a/src/live_effects/lpe-offset.h b/src/live_effects/lpe-offset.h
index bbd192860..3fc5cc458 100644
--- a/src/live_effects/lpe-offset.h
+++ b/src/live_effects/lpe-offset.h
@@ -20,6 +20,7 @@
 #include "live_effects/lpegroupbbox.h"
 #include "live_effects/effect.h"
 #include "live_effects/parameter/parameter.h"
+#include "live_effects/parameter/unit.h"
 
 namespace Inkscape {
 namespace LivePathEffect {
@@ -35,27 +36,24 @@ public:
     ~LPEOffset() override;
     void doBeforeEffect (SPLPEItem const* lpeitem) override;
     Geom::PathVector doEffect_path (Geom::PathVector const & path_in) override;
-    void doOnApply(SPLPEItem const* lpeitem) override;
-    void calculateOffset (Geom::PathVector const & path_in);
-    void drawHandle(Geom::Point p);
     void addKnotHolderEntities(KnotHolder * knotholder, SPItem * item) override;
-    double sp_get_offset(Geom::Point &origin);
+    void calculateOffset (Geom::PathVector const & path_in);
+    Geom::Point get_default_point(Geom::PathVector pathv) const;
+    Geom::Point get_nearest_point(Geom::PathVector pathv, Geom::Point point)  const;
+    double sp_get_offset(Geom::Point origin);
     friend class OfS::KnotHolderEntityOffsetPoint;
-protected:
-    void addCanvasIndicators(SPLPEItem const */*lpeitem*/, std::vector<Geom::PathVector> &hp_vec) override;
 
 private:
+    UnitParam unit;
     ScalarParam offset;
     EnumParam<unsigned> linejoin_type;
     ScalarParam miter_limit;
     BoolParam attempt_force_join;
     BoolParam update_on_knot_move;
-    Geom::PathVector hp;
     Geom::Point offset_pt;
-    Geom::Point origin;
     bool evenodd;
+    Glib::ustring display_unit;
     KnotHolderEntity * _knot_entity;
-    Geom::PathVector original_pathv;
     Geom::PathVector filled_rule_pathv;
     Inkscape::UI::Widget::Scalar *offset_widget;
 
diff --git a/src/object/sp-lpe-item.cpp b/src/object/sp-lpe-item.cpp
index b43a0b926..ec7b6469e 100755
--- a/src/object/sp-lpe-item.cpp
+++ b/src/object/sp-lpe-item.cpp
@@ -237,7 +237,7 @@ bool SPLPEItem::performOnePathEffect(SPCurve *curve, SPShape *current, Inkscape:
             return false;
         }
         //if is not clip or mask or LPE apply to clip and mask
-        if (!(is_clip_or_mask && !lpe->apply_to_clippath_and_mask)) {
+        if (!is_clip_or_mask || lpe->apply_to_clippath_and_mask) {
             lpe->setCurrentShape(current);
             if (!SP_IS_GROUP(this)) {
                 lpe->pathvector_before_effect = curve->get_pathvector();