From 07f64b7682e91c3fa77f07ee0e6f29e7c4eb7e22 Mon Sep 17 00:00:00 2001
From: David Capello <david@igarastudio.com>
Date: Sat, 8 Jun 2019 13:28:56 -0300
Subject: [PATCH] Fix Jumble tool brush behavior and improve IntertwineAsLines
 impl

* Fixed Jumble tool behavior: When brush is circular, all the brush
  bounds (square) where randomized. And at the end all the pixels were
  re-randomized.
* Solved the double-point in IntertwineAsLines from
  62802cfbdfbd85edd5a339361de70c36d74925dc in an alternative way: Just
  don't draw the first point when we join consecutive 2 points
  strokes (the issue here is that freehand controller generates
  strokes of two points, so every call on joinStroke() of
  IntertwineAsLines has only two points, we have to check if we're
  drawing the first stroke, and if we are drawing the second 2 points
  stroke, we skip the first pixel to avoid drawing it two times,
  solved with IntertwineAsLines::m_firstStroke).
---
 src/app/tools/controllers.h         |  4 ++++
 src/app/tools/intertwiners.h        | 31 +++++++++++++++--------------
 src/app/tools/tool_loop_manager.cpp |  4 ----
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/src/app/tools/controllers.h b/src/app/tools/controllers.h
index fd6bb57bb..e66406433 100644
--- a/src/app/tools/controllers.h
+++ b/src/app/tools/controllers.h
@@ -75,6 +75,10 @@ public:
       output.addPoint(input[0]);
     }
     else if (input.size() >= 2) {
+      // The freehand controller returns only the last two points to
+      // interwine because we accumulate (TracePolicy::Accumulate) the
+      // previously painted points (i.e. don't want to redraw all the
+      // stroke from the very beginning)
       output.addPoint(input[input.size()-2]);
       output.addPoint(input[input.size()-1]);
     }
diff --git a/src/app/tools/intertwiners.h b/src/app/tools/intertwiners.h
index a21575703..5336fca05 100644
--- a/src/app/tools/intertwiners.h
+++ b/src/app/tools/intertwiners.h
@@ -73,14 +73,20 @@ class IntertwineAsLines : public Intertwine {
   // user confirms a line draw while he is holding down the SHIFT key), so
   // we have to ignore printing the first pixel of the line.
   bool m_retainedTracePolicyLast = false;
-  Stroke m_pts;
+
+  // In freehand-like tools, on each mouse movement we draw only the
+  // line between the last two mouse points in the stroke (the
+  // complete stroke is not re-painted again), so we want to indicate
+  // if this is the first stroke of all (the only one that needs the
+  // first pixel of the line algorithm)
+  bool m_firstStroke = true;
 
 public:
   bool snapByAngle() override { return true; }
 
   void prepareIntertwine() override {
-    m_pts.reset();
     m_retainedTracePolicyLast = false;
+    m_firstStroke = true;
   }
 
   void joinStroke(ToolLoop* loop, const Stroke& stroke) override {
@@ -89,41 +95,35 @@ public:
     // new joinStroke() is like a fresh start.  Without this fix, the
     // first stage on LineFreehand will draw a "star" like pattern
     // with lines from the first point to the last point.
-    if (loop->getTracePolicy() == TracePolicy::Last) {
+    if (loop->getTracePolicy() == TracePolicy::Last)
       m_retainedTracePolicyLast = true;
-      m_pts.reset();
-    }
 
     if (stroke.size() == 0)
       return;
     else if (stroke.size() == 1) {
-      if (m_pts.empty())
-        m_pts = stroke;
       doPointshapePoint(stroke[0].x, stroke[0].y, loop);
-      return;
     }
     else {
+      Stroke pts;
       doc::AlgoLineWithAlgoPixel lineAlgo = getLineAlgo(loop);
       for (int c=0; c+1<stroke.size(); ++c) {
         lineAlgo(stroke[c].x, stroke[c].y,
                  stroke[c+1].x, stroke[c+1].y,
-                 (void*)&m_pts,
+                 (void*)&pts,
                  (AlgoPixel)&addPointsWithoutDuplicatingLastOne);
       }
 
       // Don't draw the first point in freehand tools (this is to
       // avoid painting above the last pixel of a freehand stroke,
       // when we use Shift+click in the Pencil tool to continue the
-      // old stroke). When filled is true we are talking about the
-      // contour tool, so we do all the points.
+      // old stroke).
       // TODO useful only in the case when brush size = 1px
       const int start =
         (loop->getController()->isFreehand() &&
-         !loop->getFilled() &&
-         m_retainedTracePolicyLast ? 1: 0);
+         (m_retainedTracePolicyLast || !m_firstStroke) ? 1: 0);
 
-      for (int c=start; c<m_pts.size(); ++c)
-        doPointshapePoint(m_pts[c].x, m_pts[c].y, loop);
+      for (int c=start; c<pts.size(); ++c)
+        doPointshapePoint(pts[c].x, pts[c].y, loop);
 
       // Closed shape (polygon outline)
       // Note: Contour tool was getting into the condition with no need, so
@@ -137,6 +137,7 @@ public:
                          stroke[0].x, stroke[0].y, loop);
       }
     }
+    m_firstStroke = false;
   }
 
   void fillStroke(ToolLoop* loop, const Stroke& stroke) override {
diff --git a/src/app/tools/tool_loop_manager.cpp b/src/app/tools/tool_loop_manager.cpp
index 3afe49a03..b5beecd36 100644
--- a/src/app/tools/tool_loop_manager.cpp
+++ b/src/app/tools/tool_loop_manager.cpp
@@ -255,10 +255,6 @@ void ToolLoopManager::doLoopStep(bool lastStep)
       m_toolLoop->validateDstImage(gfx::Region(m_toolLoop->getDstImage()->bounds()));
     }
   }
-  else if (m_toolLoop->getBrush()->type() == kImageBrushType) {
-    m_toolLoop->invalidateDstImage();
-    m_toolLoop->validateDstImage(gfx::Region(m_toolLoop->getDstImage()->bounds()));
-  }
 
   m_toolLoop->validateDstImage(m_dirtyArea);