MarginallyClever / Makelangelo-software

Software for plotters - especially the wall-hanging polargraph also called Makelangelo.
http://www.marginallyclever.com/
GNU General Public License v2.0
347 stars 150 forks source link

"New" in menu do not clear the previous drawing #378

Closed coliss86 closed 2 years ago

coliss86 commented 2 years ago

Steps to make the bug happen

  1. Go to 'Generate Art' with the MarlinInterface, keep it open
  2. Click on 'File New'
  3. Go to 'Generate Art' again
  4. Draw it, the first art is drawn

What was supposed to happen The second art should be drawn

Platform (please complete the following information):

Log file log.txt

Additional context

i-make-robots commented 2 years ago

I suspect the generate art menu is supposed to modal (blocking everything until closed). the challenge is that I still want to move around the canvas and look at the art. so simply making the dialog box modal is too aggressive.

PPAC37 commented 2 years ago

For me this is due to the fact that at each generation the Makelangelo class propertie Turtle myTurtle is affected with a new instance of a Turtle.

https://github.com/MarginallyClever/Makelangelo-software/blob/fb47a0ba78a043e8d3ec32f8210f79651b9b269b/src/main/com/marginallyclever/makelangelo/Makelangelo.java#L111

https://github.com/MarginallyClever/Makelangelo-software/blob/fb47a0ba78a043e8d3ec32f8210f79651b9b269b/src/main/com/marginallyclever/makelangelo/Makelangelo.java#L767-L770

and with

https://github.com/MarginallyClever/Makelangelo-software/blob/fb47a0ba78a043e8d3ec32f8210f79651b9b269b/src/main/com/marginallyclever/makelangelo/Makelangelo.java#L262-L263

the PlotterControls work/keep a reference to the Turtle instance used at it creation.

https://github.com/MarginallyClever/Makelangelo-software/blob/78d087275a172165f61c70ed7bee26922a67ac7f/src/main/com/marginallyclever/makelangelo/plotter/plotterControls/PlotterControls.java#L40-L42

so unless recreating the PlotterControls (close it to reopen it) you are working with a posible "outdated" Turtle instance.

To solve this, the only way i see for the moment, is somthing like a ChangeListener paterne to force the PlotterControls to change/update the Turtle is using.

PlotterControls have to do something like "myTurtle=Makelangelo.getTortle();" when reciving a change event ( that the class Makelangelo will have to "fire" at the end of this setMyTurtle() ...)

(But for me this could be the time to think on creating a kind of Turtle generation history/layer ... i would like to cumulate multiple Turtle generation on the same "page"/g-code without having to merge / modify g-code files ... )

i-make-robots commented 2 years ago

The concept of layers, each with a single turtle, has crossed my mind... I think if you get to the stage where you need a multi-layer drawing tool you should make your art in Inkscape and then bring it to makelangelo software as an SVG. Our time would be better spent trying to add missing SVG features like filling closed shapes.

PPAC37 commented 2 years ago

You are right ! focusing on the basics that are missing or bug seems more reasonable. (Sorry ! I was dreaming out loud. :) And yes inkscape is a way ... I just need to train myself to use it...)

i-make-robots commented 2 years ago

The reason plotter control keeps a copy of the turtle is so that you cannot change the turtle while drawing connected to USB. the results would be... unpredictable. In fact, you shouldn't be able to hit "new" while in the plotter control dialog.

i-make-robots commented 2 years ago

Following the steps in the OP I have this: Capture

i-make-robots commented 2 years ago

Fibonacci was drawn second, fwiw. New cleared the screen ok. As before... being able to move the view around means that you can have more than one panel open. this is annoying but unavoidable.

coliss86 commented 2 years ago

Hum, I see. What do you think if all the menus get disabled when the plotter control is open ?

i-make-robots commented 2 years ago

bcc9b820 like this?

coliss86 commented 2 years ago

good job !

i-make-robots commented 2 years ago

So.... does that mean the ticket is closed?