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

[Jabiertxof]

This branch improve the Knot and the Mesh gradient code:
Knot:
Add a new knot type "TRIANGLE".
Add a new knot property called Angle to turn triangle knots.
Mesh:
Improve the size of corners (DIAMOND) knots.
Update to angled triangle knots the handles of selected corners.

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

14388. By jabiertxof<email address hidden> on 2015-09-26

Update knot angle while dragging

14389. By jabiertxof<email address hidden> on 2015-09-26

fix bug on extra highlight of knot in radial mesh gradient

14390. By jabiertxof<email address hidden> on 2015-09-27

highligt draggers of corner on hover

14391. By jabiertxof<email address hidden> on 2015-09-28

Remove the memory problem on hover knots.
Added highlight active hover/drag line on draggers
todo: add highlight on node tool hover a knot

14392. By jabiertxof<email address hidden> on 2015-09-28

update to trunk

14393. By Jabiertxof on 2016-04-08

update to trunk

14394. By Jabiertxof on 2016-11-17

Update to trunk

14395. By Jabiertxof on 2016-11-17

Fixes to new trunk

14396. By Jabiertxof on 2016-11-18

Allow update knots on mousedown

14397. By Jabiertxof on 2016-11-18

Siplify the code

14398. By Jabiertxof <jtx@jtx> on 2016-11-26

Update to trunk

14399. By Jabiertxof on 2016-11-29

Update to trunk

14400. By Jabiertxof on 2016-11-29

Update knot colors

14401. By Jabiertxof on 2016-11-29

Update to trunk

14402. By Jabiertxof on 2016-11-29

Update knot colors

14403. By Jabiertxof <jtx@jtx> on 2016-11-29

fix knot colors

14404. By Jabiertxof on 2016-12-01

Update to trunk

14405. By Jabiertxof on 2016-12-01

Improve knots triangle

14406. By Jabiertxof on 2016-12-01

Update to trunk

14407. By Jabiertxof on 2016-12-01

Some imrovements to render triangles and fix angle knots pointed in IRC

14408. By Jabiertxof on 2016-12-01

Some imrovements to render triangle knots

14409. By Jabiertxof on 2016-12-01

NOT WORKING -Some imrovements to render triangle knots

14410. By Jabiertxof <jtx@jtx> on 2016-12-01

Polish triangle knot

14411. By Jabiertxof on 2016-12-02

Update to trunk

[Tavmjong Bah (tavmjong-free)]

Code could use some formatting attention: see https://inkscape.org/en/develop/coding-style/ :
  For example: in sodipodi-ctrl.cpp the opening bracket of the 'for' should be on the same line.

Also, avoid adding or removing blank lines unless it is a real improvement in readability.

I am wondering about the hard-coding of the sizes[] array. Wouldn't it be better to allow the user to set the size directly? This should somehow be coordinated with the node tool knot sizes. And the hard coded array should not appear three different places in the code. (Can GrDragger::updateControlSizes() be called instead?)

Why the special knot-size factor for the SP_KNOT_SHAPE_DIAMOND knot?

GR_KNOT_COLOR_HIGHLIGTH should be GR_KNOT_COLOR_HIGHLIGHT (spelling error).

In GrDragger::highlightNode

  Why the test for node->draggable < G_MAXUINT ?

What does the highlight flag do in "addCurve"?

Does enum GrDraggerPositions need to be in the header?

Not your problem... but why do we have SPKnotShapeType and SPCtrlShapeType?

This code seems to be doing two separate things:
  1. Implement the triangle knot.
  2. Implement variable size knots.

It might be better if the two were separated into different patches.

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

14412. By Jabiertxof on 2016-12-02

Some Tavmjong fixes

[Jabiertxof (jabiertxof)]

I just commit a new version with your improvements:
but:

*Why the test for node->draggable < G_MAXUINT ?
When coding sometimes this value give a > G_MAXUINT and break, maybe a
comment to check or remove on trunk and retain in 0.92 to be sure.

*What does the highlight flag do in "addCurve"?
Update the helperline active color on hover a knot.

*Does enum GrDraggerPositions need to be in the header?
Im returning this object by a typedef in a header class so I think I
need here, maybe I could put inside the class.

*This code seems to be doing two separate things:
1 create triangle knot
2 apply it to mesh tool
Create 2 branches?

Thanks for the review.

[Tavmjong Bah (tavmjong-free)]

Looks good!

I am still worried about the need for G_MAXUINT. It sounds like it's hiding a bug.

I missed the line high-lighting. Nice!

Two branches would be better... but I'm not going to insist.

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

14413. By Jabiertxof <jtx@jtx> on 2016-12-02

split on two branches

[Jabiertxof (jabiertxof)]

Splited into 2 branches. Removed G_MAXUINT. this branch be first. mesh_improvements after

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

14414. By Jabiertxof <jtx@jtx> on 2016-12-03

Update to trunk

[Tavmjong Bah (tavmjong-free)]

Merged triangle knot code into trunk after making a few changes:

* Reordered enum values.
* Used M_PI rather than custom PI constant.
* Moved "NULL" to end of arg list of sp_canvas_item_new (where it must be).

[Jabiertxof (jabiertxof)]

merged

review: Approve

=== modified file 'src/display/sodipodi-ctrl.cpp'
--- src/display/sodipodi-ctrl.cpp	2014-08-24 15:20:54 +0000
+++ src/display/sodipodi-ctrl.cpp	2016-12-03 02:40:30 +0000
@@ -22,7 +22,8 @@
     ARG_FILL_COLOR,
     ARG_STROKED,
     ARG_STROKE_COLOR,
-    ARG_PIXBUF
+    ARG_PIXBUF,
+    ARG_ANGLE
 };
 
 static void sp_ctrl_destroy(SPCanvasItem *object);
@@ -55,6 +56,8 @@
     g_object_class_install_property (g_object_class,
             ARG_PIXBUF, g_param_spec_pointer ("pixbuf", "pixbuf", "Pixbuf", (GParamFlags) G_PARAM_READWRITE));
     g_object_class_install_property (g_object_class,
+            ARG_ANGLE, g_param_spec_double ("angle", "angle", "Angle", -G_MAXDOUBLE, G_MAXDOUBLE, 0.0, (GParamFlags) G_PARAM_READWRITE));
+    g_object_class_install_property (g_object_class,
             ARG_FILLED, g_param_spec_boolean ("filled", "filled", "Filled", TRUE, (GParamFlags) G_PARAM_READWRITE));
     g_object_class_install_property (g_object_class,
             ARG_FILL_COLOR, g_param_spec_int ("fill_color", "fill_color", "Fill Color", G_MININT, G_MAXINT, 0x000000ff, (GParamFlags) G_PARAM_READWRITE));
@@ -119,6 +122,9 @@
             g_object_unref(pixbuf);
         }
         break;
+    case ARG_ANGLE:
+        ctrl->angle = (double)g_value_get_double(value);
+        break;
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
         return; // Do not do an update
@@ -171,6 +177,10 @@
             g_value_set_pointer(value, ctrl->pixbuf);
             break;
 
+        case ARG_ANGLE:
+            g_value_set_double(value, ctrl->angle);
+            break;
+
         default:
             G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
             break;
@@ -192,6 +202,7 @@
     ctrl->stroked = 0;
     ctrl->fill_color = 0x000000ff;
     ctrl->stroke_color = 0x000000ff;
+    ctrl->angle = 0.0;
 
     new (&ctrl->box) Geom::IntRect(0,0,0,0);
     ctrl->cache = NULL;
@@ -291,6 +302,23 @@
     return 1e18;
 }
 
+bool 
+sp_point_inside_line(Geom::Point a, Geom::Point b, Geom::Point c, double tolerance = 0.1){
+    //http://stackoverflow.com/questions/328107/how-can-you-determine-a-point-is-between-two-other-points-on-a-line-segment
+    return Geom::are_near(Geom::distance(a,c) + Geom::distance(c,b) , Geom::distance(a,b), tolerance);
+}
+
+bool 
+sp_point_inside_triangle(Geom::Point p1,Geom::Point p2,Geom::Point p3, Geom::Point point){
+    using Geom::X;
+    using Geom::Y;
+    double denominator = (p1[X]*(p2[Y] - p3[Y]) + p1[Y]*(p3[X] - p2[X]) + p2[X]*p3[Y] - p2[Y]*p3[X]);
+    double t1 = (point[X]*(p3[Y] - p1[Y]) + point[Y]*(p1[X] - p3[X]) - p1[X]*p3[Y] + p1[Y]*p3[X]) / denominator;
+    double t2 = (point[X]*(p2[Y] - p1[Y]) + point[Y]*(p1[X] - p2[X]) - p1[X]*p2[Y] + p1[Y]*p2[X]) / -denominator;
+    double see = t1 + t2;
+    return 0 <= t1 && t1 <= 1 && 0 <= t2 && t2 <= 1 && see <= 1;
+}
+
 static void
 sp_ctrl_build_cache (SPCtrl *ctrl)
 {
@@ -316,7 +344,7 @@
     } else {
         stroke_color = fill_color;
     }
-
+    gint32 stroke_color_smooth =  SP_RGBA32_F_COMPOSE(SP_RGBA32_R_F(stroke_color), SP_RGBA32_G_F(stroke_color), SP_RGBA32_B_F(stroke_color), 0.15);
     width = (ctrl->width * 2 +1);
     height = (ctrl->height * 2 +1);
     c = ctrl->width; // Only used for pre-set square drawing
@@ -325,7 +353,25 @@
 
     if (ctrl->cache) delete[] ctrl->cache;
     ctrl->cache = new guint32[size];
-
+    Geom::Point point;
+    Geom::Point p1;
+    Geom::Point p2;
+    Geom::Point p3;
+    if(ctrl->shape == SP_CTRL_SHAPE_TRIANGLE){
+        const double PI  =3.14159;
+        Geom::Affine m = Geom::Translate(Geom::Point(-width/2.0,-height/2.0));
+        m *= Geom::Rotate(-ctrl->angle);
+        m *= Geom::Translate(Geom::Point(width/2.0, height/2.0));
+        p1 = Geom::Point(0,height/2);
+        p2 = Geom::Point(width - (width/PI), height/PI);
+        p3 = Geom::Point(width - (width/PI), height-(height/PI));
+        p1 *= m;
+        p2 *= m;
+        p3 *= m;
+        p1 = p1.floor();
+        p2 = p2.floor();
+        p3 = p3.floor();
+    }
     switch (ctrl->shape) {
         case SP_CTRL_SHAPE_SQUARE:
             p = ctrl->cache;
@@ -407,6 +453,29 @@
             ctrl->build = TRUE;
             break;
 
+        case SP_CTRL_SHAPE_TRIANGLE:
+            p = ctrl->cache;
+            for(y = 0; y < height; y++) {
+                for(x = 0; x < width; x++) {
+                    point = Geom::Point(x,y);
+                    if (sp_point_inside_triangle(p1, p2, p3, point)) {
+                        p[(y*width)+x] = fill_color;
+                    } else if (point == p1 || point == p2 || point == p3 || sp_point_inside_line(p1, p2, point, 0.2) ||
+                               sp_point_inside_line(p3, p1, point, 0.2))
+                    {
+                        p[(y*width)+x] = stroke_color;
+                    } else if (sp_point_inside_line(p1, p2, point, 0.5) ||
+                               sp_point_inside_line(p3, p1, point, 0.5))
+                    {
+                        p[(y*width)+x] = stroke_color_smooth;
+                    } else {
+                        p[(y*width)+x] = 0;
+                    }
+                }
+            }
+            ctrl->build = TRUE;
+            break;
+
         case SP_CTRL_SHAPE_CROSS:
             p = ctrl->cache;
             for (y = 0; y < height; y++) {

=== modified file 'src/display/sodipodi-ctrl.h'
--- src/display/sodipodi-ctrl.h	2014-03-27 01:33:44 +0000
+++ src/display/sodipodi-ctrl.h	2016-12-03 02:40:30 +0000
@@ -24,7 +24,8 @@
     SP_CTRL_SHAPE_CIRCLE,
     SP_CTRL_SHAPE_CROSS,
     SP_CTRL_SHAPE_BITMAP,
-    SP_CTRL_SHAPE_IMAGE
+    SP_CTRL_SHAPE_IMAGE,
+    SP_CTRL_SHAPE_TRIANGLE
 } SPCtrlShapeType;
 
 
@@ -46,6 +47,7 @@
     guint stroked : 1;
     guint32 fill_color;
     guint32 stroke_color;
+    gdouble angle;
 
     Geom::IntRect box;   /* NB! x1 & y1 are included */
     guint32 *cache;

=== modified file 'src/knot-enums.h'
--- src/knot-enums.h	2011-10-25 07:45:35 +0000
+++ src/knot-enums.h	2016-12-03 02:40:30 +0000
@@ -21,7 +21,8 @@
     SP_KNOT_SHAPE_CIRCLE,
     SP_KNOT_SHAPE_CROSS,
     SP_KNOT_SHAPE_BITMAP,
-    SP_KNOT_SHAPE_IMAGE
+    SP_KNOT_SHAPE_IMAGE,
+    SP_KNOT_SHAPE_TRIANGLE
 } SPKnotShapeType;
 
 typedef enum {

=== modified file 'src/knot.cpp'
--- src/knot.cpp	2016-08-04 11:26:03 +0000
+++ src/knot.cpp	2016-12-03 02:40:30 +0000
@@ -76,6 +76,7 @@
     this->anchor = SP_ANCHOR_CENTER;
     this->shape = SP_KNOT_SHAPE_SQUARE;
     this->mode = SP_KNOT_MODE_XOR;
+    this->angle = 0;
     this->tip = NULL;
     this->_event_handler_id = 0;
     this->pressure = 0;
@@ -116,7 +117,8 @@
                                     "stroked", TRUE,
                                     "stroke_color", 0x01000000,
                                     "mode", SP_KNOT_MODE_XOR,
-                                    NULL);
+                                    NULL,
+                                    "angle",0.0);
 
     this->_event_handler_id = g_signal_connect(G_OBJECT(this->item), "event",
                                                  G_CALLBACK(sp_knot_handler), this);
@@ -214,7 +216,7 @@
         if ((event->button.button == 1) && knot->desktop && knot->desktop->event_context && !knot->desktop->event_context->space_panning) {
             Geom::Point const p = knot->desktop->w2d(Geom::Point(event->button.x, event->button.y));
             knot->startDragging(p, (gint) event->button.x, (gint) event->button.y, event->button.time);
-
+            knot->grabbed_signal.emit(knot, event->button.state);
             consumed = TRUE;
         }
         break;
@@ -450,6 +452,7 @@
     g_object_set(this->item, "mode", this->mode, NULL);
     g_object_set(this->item, "size", (gdouble) this->size, NULL);
     g_object_set(this->item, "anchor", this->anchor, NULL);
+    g_object_set(this->item, "angle", this->angle, NULL);
 
     if (this->pixbuf) {
         g_object_set(this->item, "pixbuf", this->pixbuf, NULL);
@@ -492,6 +495,10 @@
     pixbuf = p;
 }
 
+void SPKnot::setAngle(double i) {
+    angle = i;
+}
+
 void SPKnot::setFill(guint32 normal, guint32 mouseover, guint32 dragging) {
     fill[SP_KNOT_STATE_NORMAL] = normal;
     fill[SP_KNOT_STATE_MOUSEOVER] = mouseover;

=== modified file 'src/knot.h'
--- src/knot.h	2014-10-08 02:22:03 +0000
+++ src/knot.h	2016-12-03 02:40:30 +0000
@@ -58,6 +58,7 @@
 
     SPKnotShapeType shape;   /**< Shape type. */
     SPKnotModeType mode;
+    double angle;
 
     guint32 fill[SP_KNOT_VISIBLE_STATES];
     guint32 stroke[SP_KNOT_VISIBLE_STATES];
@@ -92,6 +93,7 @@
     void setAnchor(unsigned int i);
     void setMode(unsigned int i);
     void setPixbuf(void* p);
+    void setAngle(double i);
 
     void setFill(guint32 normal, guint32 mouseover, guint32 dragging);
     void setStroke(guint32 normal, guint32 mouseover, guint32 dragging);