1j01 / jspaint

🎨 Classic MS Paint, REVIVED + ✨Extras
https://jspaint.app/about
MIT License
7.22k stars 562 forks source link

Curve tool preview with right mouse button #157

Closed 1j01 closed 2 years ago

1j01 commented 4 years ago

The curve tool is showing the correct color when the right mouse button is down but switching back to the foreground color once you release.

aadame3311 commented 4 years ago

Hi, I am interested in working on this issue if it's still relevant

1j01 commented 4 years ago

Great! Let me know if you have any questions about code or strategies.

aadame3311 commented 4 years ago

I found a possible path to fixing this issue was to avoid "reversing" the stroke_color when using the curve tool by referencing the "stroke_only" property of the Curve, Line and Pencil tools and only reversing when this property isn't present. However, this causes an issue when using these stroke_only tools with a Brush as it causes undesired behavior as the brush tool pointer color will remain as the foreground color as opposed to it's behavior when used individually.

Another possible solution is to have a flag set to true while the curve is in "preview" and not reverse the stroke color until that flag is set to false (once the curve has been completed). The drawing curve flag would be reset to false if a tool is selected. The problem here is that, again, when using in conjunction with the brush tool, the brush tool pointer will remain as the foreground color until the curve is finished drawing.

Any thoughts on these two possible solutions are welcome.

https://github.com/1j01/jspaint/blob/7d002432a79797f76836c1af6f09fdec0b5442eb/src/app.js#L836-L858

aadame3311 commented 4 years ago

I think a better strategy would be to isolate the fix to only the curve tool so it doesn't interfere with the colors of the other tools that might be selected/used at the same time

1j01 commented 4 years ago

Another possible solution is to have a flag set to true while the curve is in "preview" and not reverse the stroke color until that flag is set to false (once the curve has been completed). The drawing curve flag would be reset to false if a tool is selected.

Yep, that could definitely work.

I think a better strategy would be to isolate the fix to only the curve tool so it doesn't interfere with the colors of the other tools that might be selected/used at the same time

Yeah, you can just check if the tool's name === "Curve", don't need to try to make it more general. The other tool this might have applied to is the Polygon tool, but it doesn't have this problem.

The Polygon doesn't have this problem because it uses a temporary canvas of its own, and controls when that gets redrawn, which could be another solution.

1j01 commented 4 years ago

I don't get what you mean by the first solution. How would it work with both left for foreground color and right for background color, if it doesn't reverse? Can you clarify? I can't tell if it makes sense or not :slightly_smiling_face:

aadame3311 commented 4 years ago

Sorry, what I meant was for it to stay reversed until the curve is finished drawing. We can do this by setting the reverse flag in the pointerup() event of the Curve tool. i.e.:

pointerup(ctx, x, y) {
        if(this.points.length >= 4){
            undoable({
                name: "Curve",
                icon: get_icon_for_tool(this),
            }, ()=> {
                this.draw_curve(ctx);
            });
            this.points = [];
        }
        else {
                        // prevent it from reversing here until all points have been drawn
            !(reverse) && (reverse = true);
        }
    },
aadame3311 commented 4 years ago

The Polygon doesn't have this problem because it uses a temporary canvas of its own, and controls when that gets redrawn, which could be another solution.

I'm a bigger fan of this solution. I think I found a working implementation by adding a preview_canvas to the curve tool and drawing on that before it draws on the actual canvas once draw_curve gets called.

PR: #162