Merge lp:~inkscape.dev/inkscape/lastApplied into lp:~inkscape.dev/inkscape/trunk

[Jabiertxof]

Add a dropdown item to the path stroke list in pen/cil mode. It get the last applied path from the clipboard, making unnecesary retain the desired path in the clipboard.

[Krzysztof Kosinski (tweenk)]

1. This feature is misleadingly named. It will apply the first stroke shape encountered in the defs. As far as I know, there is no guarantee on the order of defs.

2. The line "if (pattern == (const gchar *)"M 0,0 1,0") continue;" is wrong - you can't test for pointer equality even when one of the strings is constant.

3. Confusing control flow around the "shape_applied" variable. It should default to false and be set to true inside the if.

review: Disapprove

[Jabiertxof (jabiertxof)]

Thanks for the review Krzysztof.

Anybody confirm about first point? If is wrong with no manual XML
modification...
Second point. Any body can help me?
Third point is bad because i like the strokes like the original, I
dislike force the style to black and always filled.
If is posibol to continue with it i need a better name...you are
welcome.

Hi, Jabier.

If anybody confirm
El lun, 09-09-2013 a las 15:54 +0000, Krzysztof Kosinski escribió:
> Review: Disapprove
>
> 1. This feature is misleadingly named. It will apply the first stroke shape encountered in the defs. As far as I know, there is no guarantee on the order of defs.
>
> 2. The line "if (pattern == (const gchar *)"M 0,0 1,0") continue;" is wrong - you can't test for pointer equality even when one of the strings is constant.
>
> 3. Confusing control flow around the "shape_applied" variable. It should default to false and be set to true inside the if.

[lp:~inkscape.dev/inkscape/lastApplied]

12507. By Jabiertxof <email address hidden> on 2013-09-11

Update whith the big help of Krzysztof

12508. By Jabiertxof <email address hidden> on 2013-09-11

Update to trunk

[Martin Owens (doctormo)]

The code looks small, compact and useful. Is there any reason not to merge?

review: Approve (code review)

[Krzysztof Kosinski (tweenk)]

The new behavior looks a lot better, but there are 2 more things to fix:

1. The static global variable 'pathv' is declared in the header, which is unacceptable - it should be declared in the cpp file, preferably in the function itself.
2. The name should be "Last applied".
3. Other cases in the switch should also set 'pathv' to the stroke shape which they applied.

review: Needs Fixing

[lp:~inkscape.dev/inkscape/lastApplied]

12509. By Jabiertxof <email address hidden> on 2013-09-20

Krzysztof review done

[Jabiertxof (jabiertxof)]

El jue, 19-09-2013 a las 14:02 +0000, Krzysztof Kosinski escribió:
> Review: Needs Fixing
>
> The new behavior looks a lot better, but there are 2 more things to fix:
>
> 1. The static global variable 'pathv' is declared in the header, which is unacceptable - it should be declared in the cpp file, preferably in the function itself.
> 2. The name should be "Last applied".
> 3. Other cases in the switch should also set 'pathv' to the stroke shape which they applied.

Hi Krzysztof thanks for the review.
I made a update to the branch whith your improvements.
The only thing not full adopt is the point tree, because the case 1 and
2 not use a pathvector, use the live effect "Power Stroke" and the case
3 depends is a ellipse calculated whith the data of the stroke. I made
another approach. Tell me your opinion.

[lp:~inkscape.dev/inkscape/lastApplied]

12510. By Jabiertxof <email address hidden> on 2013-09-20

Fix and update to trunk

[su_v (suv-lp)]

r12510 fails to compile (OS X 10.7.5, llvm-gcc-4.2, glib 2.36.4, glibmm 2.36.2):

  CXX draw-context.o
../../src/draw-context.cpp: In function ‘void spdc_check_for_and_apply_waiting_LPE(SPDrawContext*, SPItem*, SPCurve*)’:
../../src/draw-context.cpp:305: error: ‘ClipboardManager’ is not a member of ‘Inkscape::UI’
../../src/draw-context.cpp:305: error: ‘cm’ was not declared in this scope
../../src/draw-context.cpp:305: error: ‘Inkscape::UI::ClipboardManager’ has not been declared
make[3]: *** [draw-context.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2

[lp:~inkscape.dev/inkscape/lastApplied]

12511. By Jabiertxof <email address hidden> on 2013-09-20

~suv review: Fix for error compiling with clipboard

[Jabiertxof (jabiertxof)]

El vie, 20-09-2013 a las 17:02 +0000, ~suv escribió:
> r12510 fails to compile (OS X 10.7.5, llvm-gcc-4.2, glib 2.36.4, glibmm 2.36.2):
>
> CXX draw-context.o
> ../../src/draw-context.cpp: In function ‘void spdc_check_for_and_apply_waiting_LPE(SPDrawContext*, SPItem*, SPCurve*)’:
> ../../src/draw-context.cpp:305: error: ‘ClipboardManager’ is not a member of ‘Inkscape::UI’
> ../../src/draw-context.cpp:305: error: ‘cm’ was not declared in this scope
> ../../src/draw-context.cpp:305: error: ‘Inkscape::UI::ClipboardManager’ has not been declared
> make[3]: *** [draw-context.o] Error 1
> make[2]: *** [all] Error 2
> make[1]: *** [all-recursive] Error 1
> make: *** [all] Error 2
>
Hi ~suv, could you test now? Thanks.

[su_v (suv-lp)]

On 2013-09-20 19:33 +0200, Jabiertxo Arraiza Cenoz wrote:
> El vie, 20-09-2013 a las 17:02 +0000, ~suv escribió:
>> r12510 fails to compile (OS X 10.7.5, llvm-gcc-4.2, glib 2.36.4, glibmm 2.36.2):
>>
>> CXX draw-context.o
>> ../../src/draw-context.cpp: In function ‘void spdc_check_for_and_apply_waiting_LPE(SPDrawContext*, SPItem*, SPCurve*)’:
>> ../../src/draw-context.cpp:305: error: ‘ClipboardManager’ is not a member of ‘Inkscape::UI’
>> ../../src/draw-context.cpp:305: error: ‘cm’ was not declared in this scope
>> ../../src/draw-context.cpp:305: error: ‘Inkscape::UI::ClipboardManager’ has not been declared
>> make[3]: *** [draw-context.o] Error 1
>> make[2]: *** [all] Error 2
>> make[1]: *** [all-recursive] Error 1
>> make: *** [all] Error 2
>>
> Hi ~suv, could you test now? Thanks.

Confirmed fixed (r12511) - build succeeds once the missing header file is included :)

[lp:~inkscape.dev/inkscape/lastApplied]

12512. By Jabiertxof <email address hidden> on 2013-09-20

update to trunk

[Jabiertxof (jabiertxof)]

El vie, 20-09-2013 a las 17:42 +0000, ~suv escribió:
> On 2013-09-20 19:33 +0200, Jabiertxo Arraiza Cenoz wrote:
> > El vie, 20-09-2013 a las 17:02 +0000, ~suv escribió:
> >> r12510 fails to compile (OS X 10.7.5, llvm-gcc-4.2, glib 2.36.4, glibmm 2.36.2):
> >>
> >> CXX draw-context.o
> >> ../../src/draw-context.cpp: In function ‘void spdc_check_for_and_apply_waiting_LPE(SPDrawContext*, SPItem*, SPCurve*)’:
> >> ../../src/draw-context.cpp:305: error: ‘ClipboardManager’ is not a member of ‘Inkscape::UI’
> >> ../../src/draw-context.cpp:305: error: ‘cm’ was not declared in this scope
> >> ../../src/draw-context.cpp:305: error: ‘Inkscape::UI::ClipboardManager’ has not been declared
> >> make[3]: *** [draw-context.o] Error 1
> >> make[2]: *** [all] Error 2
> >> make[1]: *** [all-recursive] Error 1
> >> make: *** [all] Error 2
> >>
> > Hi ~suv, could you test now? Thanks.
>
> Confirmed fixed (r12511) - build succeeds once the missing header file is included :)

Thanks and sorry for the mistake ~suv

[lp:~inkscape.dev/inkscape/lastApplied]

12513. By Jabiertxof <email address hidden> on 2013-09-26

update to trunk

12514. By Jabiertxof <email address hidden> on 2013-09-26

Update to trunk

[Jabiertxof (jabiertxof)]

Hi Krzysztof i requet for another review with the web gui. Thanks again for your time.

[lp:~inkscape.dev/inkscape/lastApplied]

12515. By Jabiertxof <email address hidden> on 2013-10-02

Start

12516. By Jabiertxof <email address hidden> on 2013-10-05

update to trunk

12517. By Jabiertxof <email address hidden> on 2013-10-13

Update to trunk

12518. By Jabiertxof <email address hidden> on 2013-10-26

Update to trunk

12519. By Jabiertxof <email address hidden> on 2013-11-03

Update to trunk

12520. By Jabiertxof <email address hidden> on 2013-11-13

Update to trunk

12521. By Jabiertxof <email address hidden> on 2013-11-16

Update to trunk

12522. By Jabiertxof <email address hidden> on 2013-11-20

Update to trunk

12523. By Jabiertxof <email address hidden> on 2014-01-12

update to trunk

12524. By Jabiertxof <email address hidden> on 2014-01-12

fixes released thanks to Johan Engelen in today bughunt

12525. By Jabiertxof <email address hidden> on 2014-03-23

update to trunk

12526. By Jabiertxof <email address hidden> on 2014-03-23

update to trunk

12527. By Jabiertxof <email address hidden> on 2014-03-29

update to trunk

[Jabiertxof (jabiertxof)]

Hi Krzysztof Kosinski. I update the code whith your changes and whith the big help of Johan Engelen.

review: Needs Resubmitting

[lp:~inkscape.dev/inkscape/lastApplied]

12528. By root <email address hidden> on 2014-05-30

update to trunk

12529. By Jabiertxof <email address hidden> on 2014-06-13

update to trunk

12530. By Jabiertxof <email address hidden> on 2014-06-13

Use of enum instead integers, pointed by Adib.

12531. By Jabiertxof <email address hidden> on 2014-06-13

a bit refactor on enum shapes

12532. By Jabiertxof <email address hidden> on 2014-07-23

update to trunk

12533. By Jabiertxof <email address hidden> on 2014-07-30

update to trunk

[su_v (suv-lp)]

lastApplied was added to experimental in r13449
http://bazaar.launchpad.net/~inkscape.dev/inkscape/experimental/revision/13449

experimental was merged into trunk in r13641
http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/13641

Changing status to 'Merged'.

=== modified file 'src/ui/tools/freehand-base.cpp'
--- src/ui/tools/freehand-base.cpp	2014-07-29 01:47:11 +0000
+++ src/ui/tools/freehand-base.cpp	2014-07-30 00:15:34 +0000
@@ -44,6 +44,8 @@
 #include "live_effects/lpe-powerstroke.h"
 #include "style.h"
 #include "ui/control-manager.h"
+// clipboard support
+#include "ui/clipboard.h"
 #include "ui/tools/freehand-base.h"
 
 #include <gdk/gdkkeysyms.h>
@@ -272,7 +274,13 @@
             Effect::createAndApply(SPIRO, dc->desktop->doc(), item);
         }
 
-        int shape = prefs->getInt(tool_name(dc) + "/shape", 0);
+        //Store the clipboard path to apply in the future without the use of clipboard
+        static Geom::PathVector previous_shape_pathv;
+        enum shapeType { NONE, TRIANGLE_IN, TRIANGLE_OUT, ELLIPSE, CLIPBOARD, LAST_APPLIED };
+        static shapeType previous_shape_type = NONE;
+
+
+        shapeType shape = (shapeType)prefs->getInt(tool_name(dc) + "/shape", 0);
         bool shape_applied = false;
         SPCSSAttr *css_item = sp_css_attr_from_object(item, SP_STYLE_FLAG_ALWAYS);
         const char *cstroke = sp_repr_css_property(css_item, "stroke", "none");
@@ -280,11 +288,18 @@
 #define SHAPE_LENGTH 10
 #define SHAPE_HEIGHT 10
 
+        if(shape == LAST_APPLIED){
+            shape = previous_shape_type;
+            if(shape == CLIPBOARD){
+                shape = LAST_APPLIED;
+            }
+        }
+
         switch (shape) {
-            case 0:
+            case NONE:
                 // don't apply any shape
                 break;
-            case 1:
+            case TRIANGLE_IN:
             {
                 // "triangle in"
                 std::vector<Geom::Point> points(1);
@@ -294,7 +309,7 @@
                 shape_applied = true;
                 break;
             }
-            case 2:
+            case TRIANGLE_OUT:
             {
                 // "triangle out"
                 guint curve_length = curve->get_segment_count();
@@ -305,7 +320,7 @@
                 shape_applied = true;
                 break;
             }
-            case 3:
+            case ELLIPSE:
             {
                 // "ellipse"
                 SPCurve *c = new SPCurve();
@@ -318,22 +333,40 @@
                 c->closepath();
                 spdc_paste_curve_as_freehand_shape(c, dc, item);
                 c->unref();
+
                 shape_applied = true;
                 break;
             }
-            case 4:
+            case CLIPBOARD:
             {
                 // take shape from clipboard; TODO: catch the case where clipboard is empty
                 Effect::createAndApply(PATTERN_ALONG_PATH, dc->desktop->doc(), item);
                 Effect* lpe = SP_LPE_ITEM(item)->getCurrentLPE();
                 static_cast<LPEPatternAlongPath*>(lpe)->pattern.on_paste_button_click();
+                Inkscape::UI::ClipboardManager *cm = Inkscape::UI::ClipboardManager::get();
+                Glib::ustring svgd = cm->getPathParameter(SP_ACTIVE_DESKTOP);
+                previous_shape_pathv = sp_svg_read_pathv(svgd.data());
 
                 shape_applied = true;
                 break;
             }
+            case LAST_APPLIED:
+            {
+                if(previous_shape_pathv.size() != 0){
+                    SPCurve * c = new SPCurve();
+                    c->set_pathvector(previous_shape_pathv);
+                    spdc_paste_curve_as_freehand_shape(c, dc, item);
+                    c->unref();
+
+                    shape_applied = true;
+                }
+                break;
+            }
             default:
                 break;
         }
+        previous_shape_type = shape;
+
         if (shape_applied) {
             // apply original stroke color as fill and unset stroke; then return
             SPCSSAttr *css = sp_repr_css_attr_new();

=== modified file 'src/widgets/pencil-toolbar.cpp'
--- src/widgets/pencil-toolbar.cpp	2014-03-26 21:14:16 +0000
+++ src/widgets/pencil-toolbar.cpp	2014-07-30 00:15:34 +0000
@@ -157,6 +157,7 @@
     glist = g_list_append (glist, _("Triangle out"));
     glist = g_list_append (glist, _("Ellipse"));
     glist = g_list_append (glist, _("From clipboard"));
+    glist = g_list_append (glist, _("Last applied"));
 
     return glist;
 }