canvas work part II

[bbelderbos]

Checklist as per meeting:

• get undo/redo working (bob)
• remove store and load (JS and Django/Python) - this is because we do this centralized for all widgets users submits in the assignment view (AKB)
• remove "line" from select box because free draw supports it, just need to say somewhere in UI that it works holding the shift key (AKB)
• relabel cancel drawing mode to something like "move shapes" which is more in line with what it does (AKB)
• add an eraser widget
• remove 2nd row of widgets (html and corresponding JS code, e.g. event listeners) (AKB)
• fix extra curve when user drags mouse while typing (moved to new debugging list below)

[bbelderbos]

@anschelburk I found a plugin 💡 - works now: #44 - it even has ctrl+z and ctrl+y keyboard shortcuts 🎉

[anschelburk]

@anschelburk I found a plugin 💡 - works now: #44 - it even has ctrl+z and ctrl+y keyboard shortcuts 🎉

Confirmed that it works! One question - I merged the pull request and pulled it to my local repo, but noticed that part of the new code for the script.js module is crossed out on VS Code. Do you know why? (Line 232 - screenshot below.)

[anschelburk]

Was able to get most of these things done. Started work on the eraser, but it turns out that an eraser is not included as part of Fabric.js's standard package, so will need to be created. (I took a first stab at implementing this code, but it doesn't yet work. Fabric.js does have a page about this, which I took a look at today, and will continue exploring tomorrow.)

In the time, I've opened a pull request with my work so far - eraser needs to be debugged, and the extra curve still needs to be eliminated when a user drags their mouse while typing. Otherwise, everything seems to work as it should! Will revisit these things tomorrow.

[bbelderbos]

Reviewed before seeing this, that explains why the EraserBrush is not working, because "You will need to create a custom build."

Going to that page http://fabricjs.com/build/ it pre-selects eraser, then you can download a minimized js file, which you can include in your django static files and then link it in your template. I will try that later ...

[bbelderbos]

Not sure about the crossed out code in vs code, I have never seen that, gpt:

[bbelderbos]

@anschelburk great find regarding the custom fabric js rolling option, I did that and included it in static/js, then with a few tweaks it works now, beautiful! #46 (also did a bit of general cleanup)

HTH
Bob

[anschelburk]

We've now implemented all features, but there are some bugs. Let's talk about these tomorrow - I've fixed one, and left some comments on a few more of the bugs. I've also submitted a pull request for where my code is right now.

Bugs to fix:

• Eraser is removing background image when background image is selected
• undo() function is not removing eraser.
• canvas.clear() function is also removing background image
• Extra curve is being traced when a user holds the mouse down on typing mode.

[anschelburk]

• Eraser is removing background image when background image is selected

Fixed this: learned I could change how the background image was being set, so that I could explicitly define its erasable property to be false.

[anschelburk]

• canvas.clear() function is also removing background image

Tried to fix this by defining a new variable toward the beginning of the code,

let isCartesianBackground = false;

toggling that variable's value in the updateCanvasBackground() function,

function updateCanvasBackground(selectedValue) {
    if (selectedValue === 'blank') {
        let isCartesianBackground = false;
        canvas.setBackgroundColor('#ffffff', function() {
            canvas.renderAll();
        });
    } else if (selectedValue === 'cartesian') {

        let isCartesianBackground = true;
        var cartesianImg = new Image()
        cartesianImg.src = '/static/img/cartesian_plane.png'

        cartesianImg.onload = function() {
            var fabricCartesianBackground = new fabric.Image(cartesianImg, {
                left: 0,
                top: 0,
                scaleX: canvas.width / cartesianImg.width,
                scaleY: canvas.height / cartesianImg.height,
                originX: 'left',
                originY: 'top',
                erasable: false
            });
            canvas.setBackgroundImage(fabricCartesianBackground, canvas.renderAll.bind(canvas));
        };
    }
}

and then changing the clearEl.onclick part of the code, from simply saying canvas.clear() to the following:

    clearEl.onclick = function() {
            canvas.clear()
            if (isCartesianBackground) {
                updateCanvasBackground('cartesian')
            }
            else if (isCartesianBackground === false) {
                updateCanvasBackground('blank')
            };
        };

Not working yet though. (I suspect part of this has to do with the scope of the variable - I'm using the same variable na me inside and outside of a function, which may not be the right approach, though I'm unsure what to do at that point. Will revisit tomorrow.

[anschelburk]

• undo() function is not removing eraser.

Let's talk tomorrow about how the new js file for the eraser works - I merged the pull request you sent, and saw that the new fabric.min.js module is being referenced in the drawing.html template, but am otherwise not quite sure how that code is being activated. I think if I can understand that, it would help me debug this issue.

[anschelburk]

• canvas.clear() function is also removing the background image

Fixed - added an extra line to if statement when the background selection from the drop-down list is blank, explicitly defining the background image as null. That fixed the issue.

[anschelburk]

• Extra curve is being traced when a user holds the mouse down on typing mode

Found a workaround for now - the curve is still traced out, but it immediately disappears as soon as the user releases the mouse. Did this by adding in a new if statement to typing mode:

      if (isEraserMode === false) {
        canvas.freeDrawingBrush.width = 0
      };

and then adding a second if statement to freedraw mode:

        if (isEraserMode === false) {
            canvas.freeDrawingBrush.width = 1
        }

I also moved the initial let isEraserMode = false; statement to the global scope at the very top.

I think that these 'null strokes' may still be registering on the undoStack, because after tracing out one of these 'null strokes', I need to click the Undo button a few times for anything to happen.

[anschelburk]

Also, I think your suspicion was correct that Copilot and GPT are different experiences. I want to play around with the inverted feature more, but here's the same question, posed first to Copilot (using GPT 4) and OpenAI's ChatGPT 3.5:

• Copilot
• OpenAI
  • *I originally had a conversation with OpenAI that actually summed it up better, ending with the succinct summary that the inverted feature essentially changes the brush function so it subtracts content instead of adding it. I wasn't logged in, so that conversation didn't save, though this one still makes the point - GPT seemed to give a better response both times than Copilot

[bbelderbos]

Great work! And that's interesting regarding the AI tools.

            if (isCartesianBackground) {
                updateCanvasBackground('cartesian')
            }
            else if (isCartesianBackground === false) {  // <- can this be just `else`?
                updateCanvasBackground('blank')
            };

So only issue left is undo/redo with eraser at this point?

[bbelderbos]

Also, I think your suspicion was correct that Copilot and GPT are different experiences. I want to play around with the inverted feature more, but here's the same question, posed first to Copilot (using GPT 4) and OpenAI's ChatGPT 3.5:

• Copilot

• OpenAI

  • *I originally had a conversation with OpenAI that actually summed it up better, ending with the succinct summary that the inverted feature essentially changes the brush function so it subtracts content instead of adding it. I wasn't logged in, so that conversation didn't save, though this one still makes the point - GPT seemed to give a better response both times than Copilot

Agreed, and that is 3.5! So the tool/interface matters a lot here actually.

[bbelderbos]

Seems it's a known issue that fabric-history (undo/redo) does not work well with eraser: alimozdemir/fabric-history#53

There seems a workaround at the end which I translated through gpt, but not sure if it's worth spending too much time on given that the assignment part will be quite some work to do yet.

[anschelburk]

Got it, that's interesting. I'm going to open a new issue to debug the undo/redo button. I want to keep working on it and talking about questions with it together, but I think that can be done in the background as we shift the focus of our meetings to the multiple widgets feature.

So, I'm going to close this issue as done, and...

• Open a new issue for debugging the undo button.
• Create a new branch, origin/multiple_widgets, for starting work on the multiple widgets feature.