Add Polypath

[hiankun]

This code is trying to add polygon/polyline function as mentioned in issue #10 .

What I have done is to use left clicks to add points and right click to complete the polygon.

The polygon was drawn by creating a string which was formatted as a fabric.Path. (My reference is Introduction to Fabric.js)

Because I have no idea about how to control the fabric.Path, so finally my workaround is to create temporary line segments to show the polygon's strokes, and temporary polygons to "update" during the drawing process. All of the temporary objects will be removed when the polygon is completed.

The path json data shown in the test app is in the form of [['M', 176, 109], ['L', 293, 26], ['L', 356, 75], ['L', 300, 106], ['L', 293, 75], ['z']], for example.

[andfanilo]

Finally had a look :) It is awesome! I never expected to do more than freedrawing with this component you know 🌟 and I think your approach is good.

Some thoughts:

• Do you think it is easy to implement double-click to delete the figure being drawn? If not just create a new issue and save it for later, I think focusing on just creating the polypath is already great
• Did you forget to commit your lib/index.ts to add the polypath to the list of accessible tools?

import CircleTool from "./circle"
import FabricTool from "./fabrictool"
import FreedrawTool from "./freedraw"
import PolypathTool from "./polypath"  // <-- ADDED
import LineTool from "./line"
import RectTool from "./rect"
import TransformTool from "./transform"

// TODO: Should make TS happy on the Map of selectedTool --> FabricTool
const tools: any = {
  circle: CircleTool,
  freedraw: FreedrawTool,
  line: LineTool,
  polypath: PolypathTool,   // <-- ADDED
  rect: RectTool,
  transform: TransformTool,
}

export { tools, FabricTool }

• also in __init__.py, don't forget to add polypath in the documentation of possible choices. (I was also thinking, I should really constrain the choices for drawing_mode to {'freedraw', 'transform', 'line', 'rect', 'circle'} using a Python Enum, let me create an issue for this)

About the "send to Streamlit on every redraw or canvas.renderAll

DrawableCanvas.tsx line 183:

        <UpdateStreamlit
          canvasHeight={canvasHeight}
          canvasWidth={canvasWidth}
          shouldSendToStreamlit={
            realtimeUpdateStreamlit || forceSendToStreamlit
          }
          stateToSendToStreamlit={currentState}
        />

is responsible for sending the current drawing back to Streamlit at every rerender/redrawing. It is currently controlled by:

• realtimeUpdateStreamlit: argument on the Python side, that you manually set to False to draw your polypath.
• forceSendToStreamlit: goes to True when you click on the download button in the toolbar. You can see it's changed by the forceStreamlitUpdate callback in the CanvasToolbar, whose only goal is passing a forceSendToStreamlit: True argument from the reducer in DrawableCanvasState.

There are multiple ways of changing this,

Method 1: controlling force update Streamlit in Python, right-click to send state to Streamlit

maybe the easier way for now is, when polypath is active to deactivate the realtimeUpdateStreamlit (like you were doing manually) and use the forceSendToStreamlit callback on a right-click to close the polygon and fire the Streamlit return:

1. in __init_.py, let's stop streamlit update for polypath: realtimeUpdateStreamlit=update_streamlit and (drawing_mode!="polypath")
2. in DrawableCanvas.tsx around line 145, in canvas.on("mouse:up") but for right click, run forceStreamlitUpdate() to force the Streamlit return. Just need to make sure we're not running into an infinite loop of forceStreamlitUpdate() --> redraws --> reruns forceStreamlitUpdate() --> redraws...

If you add the stopContextMenu: False then I think we can have this right click to send to Streamlit for all tools, for those who don't want to use the toolbar to run the forceSendToStreamlit.

Method 2: in Typescript, checking if tool is polypath

One other way I was thinking of is, instead of controlling logic in Python we control logic in Typescript, thus in const selectedTool = new tools[drawingMode](canvas) as FabricTool on DrawableCanvas.tsx line 138 making the selectedTool a global reference using the React useRef and then check if it's a "polypath" in the render:

shouldSendToStreamlit={
      realtimeUpdateStreamlit || forceSendToStreamlit || selectedTool != 'polypath'
}

though you would still need a way to specify you have finished drawing your polypath to send data to Streamlit...and that I did not think about.

I have to admit I'm less fan of this, as in terms of responsibility I would really like the Typescript part to focus on drawing and preserving history, and Python control most of the logic through arguments sent as props to the React component. But if you run into problems implementing the first solution, maybe the second one would work.

---

Hope I was clear in everything :) thanks for the great job! It's really cool to see people doing Python and React stuff at the same time.

Have a nice weekend,
Fanilo

[hiankun]

Hi, I have submitted a new commit but then I found that I haven't keep my __init__.py and DrawableCanvas.tsx with the develop branch. Give me some time to fix those two files.

Back to the implement. Now the Polypath will show a starting circle as a visual assistance. The reason is that now we can use double-click to cancel previous points. If we keep doing double-clicks, the polygon will go back to the beginning when it has only one point. If we double-click again, the starting point will disappear and it means we can start a new polygon at other new position.

Right-click now can complete the polygon and update the canvas as well. I adopted Method 1 mentioned by @andfanilo above. The problem now is that the right-click will create shapes (except for freedraw) with zero width and height. Also, for polypath, after clearing the canvas by pressing the bin icon, the canvas won't update automatically.

The following screenshot is a simple demo of current status. The second drawing shows double-clicks (cannot be seen in the animation) to cancel previous points until the starting one. In the final part, we can see that the canvas didn't update, and then right-click created a zero-sized polypath.

[hiankun]

@andfanilo I don't fully understand the part of "to implement double-click to delete the figure being drawn", but it gave me the idea of "cancelling previous points". Hope the implementation makes sense.

[andfanilo]

but it gave me the idea of "cancelling previous points". Hope the implementation makes sense.

Oh yeah that's much better than my initial idea, that's super cool 🔥 !

The problem now is that the right-click will create shapes (except for freedraw) with zero width and height.

It seems this problem disappears with this._canvas.fireRightClick = false in every tool's configureCanvas except for polypath obviously (i'm thinking canvas.isDrawingMode=true automatically deactivates the right click which is why we don't see this on freedraw). I'll let you test on your side.

• Oh I just noticed, if you right click in polypath mode when there's no path yet, an empty path is created on the canvas.

Also, for polypath, after clearing the canvas by pressing the bin icon, the canvas won't update automatically.

Urrrf 😿

Tell me what you think, it makes sense that when we press the bin, we delete all history and then send the empty state to Streamlit even if the user has specified he/she doesn't want realtime update. I don't think it's an issue to update Streamlit on pressing the bin, it's not a costly operation like when drawing multiple rectangles fast. Anything I'm missing in this line of thought?

To implement this, you can add in DrawableCanvasState.tsx around L36:

const RELOAD_AND_SEND_TO_STREAMLIT: CanvasAction = {
  shouldReloadCanvas: true,
  forceSendToStreamlit: true,
}

and L173:

    case "reset":
      if (!action.state) throw new Error("No action state to store in reset")
      return {
        history: {
          undoStack: [],
          redoStack: [],
        },
        action: { ...RELOAD_AND_SEND_TO_STREAMLIT },  // <--- CHANGE HERE
        initialState: action.state,
        currentState: action.state,
      }

[hiankun]

Hi, sorry for so late reply! My work is overflowing this week. Orz

Thanks for the suggestions and instructions. Hope I can try them this weekend. :-)

[hiankun]

Tell me what you think, it makes sense that when we press the bin, we delete all history and then send the empty state to Streamlit even if the user has specified he/she doesn't want realtime update.

I totally agree with this, and I think I have solved it by following your instructions. :-)

Also, during the implementation, I found some other issues and tried to fix them. I have given some comments on the new code to explain my idea and considerations.

[andfanilo]

I'll retest later and address your other comments, I just saw those 2 typos so at least that's done

[andfanilo]

RELOAD_ADN_SEND_TO_STREAMLIT should be RELOAD_AND_SEND_TO_STREAMLIT

[andfanilo]

Apart from a very minor fix, I think we're all good to merge and make a release :) almost there!

BTW when I'm going to promote this release on Twitter/Linkedin, if you have a Twitter/Linkedin profile and want to appear in the post don't hesitate to mail them to me 😄 (my mail should be in my Github profile)

Have a good we 🎈
Fanilo