tuxpaint-devel

[Tuxpaint-devel] Polygon fill messing with other tools drawing

[Pere P. i C. <per...@gm...>]

Hi all,

Tested the new polyfill stuff, it is nice :)

Just found a minor problem:
Start Tux Paint, select the Polygon Fill tool.
select another tool and draw something.
Return to the Polygon Fill tool, see how everything you had drawn is lost.

The problem is polyfill_snapshot being kept and later blit into the canvas via
set_color() -> draw_preview() before switchin()

We could clean up polyfill_snapshot at switchout() and recreate it at each swcithin() or
we could set some marker, mark it at switchout, clean it up at switchin() and only blit polyfill_snapshot
if the marker allows it, that would allow to reuse polyfill_snapshot and not destroy/recreate it
at each switchout()/switchin() cycle.

HTH
Pere

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Bill K. <nb...@so...>]

On Sun, Apr 07, 2024 at 03:11:09PM +0200, Pere Pujal i Carabantes wrote:
> Hi all,
> 
> Tested the new polyfill stuff, it is nice :)
> 
> Just found a minor problem:

"Minor" X-D


> Start Tux Paint, select the Polygon Fill tool.
> select another tool and draw something.
> Return to the Polygon Fill tool, see how everything you had drawn is lost.
> 
> The problem is polyfill_snapshot being kept and later blit into the canvas via
> set_color() -> draw_preview() before switchin()
> 
> We could clean up polyfill_snapshot at switchout() and recreate it at each swcithin() or
> we could set some marker, mark it at switchout, clean it up at switchin() and only blit polyfill_snapshot
> if the marker allows it, that would allow to reuse polyfill_snapshot and not destroy/recreate it
> at each switchout()/switchin() cycle.

For now, I've set a variable to tell whether switchin() has been
called yet.  To me, it seems strange that I'm calling a Magic tool's
set_color() BEFORE its switchin() has been called.  But rather than
change that (and potentially break lots of other tools), I've opened a
ticket. :-)

  https://sourceforge.net/p/tuxpaint/bugs/286/

I also addressed an issue I already knew about, where the in-progress
polygon preview (the lines and big red & green boxes) would remain on
the canvas when switching to a different tool.

I believe I solved that by simply blitting the tool's own snapshot
back to the canvas on switchout().  (And since it makes a fresh
snapshot after every polygon has been completed, this works.)

Adjustments like this may help some of the other Magic tools that
"act weird" when you're in the middle of using them, but then
switch away.

I've also thought it might be nice to provide a way for Magic
tools to say "I can handle the 'Undo' command myself", e.g.
for Filled Polygon tool, it would remove the most-recently
added point.  Then, if there's no more work for them to do,
they can tell Tux Paint that it should do it's normal Undo
process.

Prior to that, I also thought it might be interesting to provide a
way for Magic tools to provide an 'overlay' SDL_Surface which
Tux Paint would draw on top everything else, rather than having
to abuse the main `canvas` surface to draw previews, crosshairs,
grids, etc.

Things are getting tricky. :)

-bill!

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Pere P. i C. <per...@gm...>]

El dl. 08 de 04 de 2024 a les 21:06 -0700, en/na Bill Kendrick va escriure:
> On Sun, Apr 07, 2024 at 03:11:09PM +0200, Pere Pujal i Carabantes wrote:
> > Hi all,
> > 
> > Tested the new polyfill stuff, it is nice :)
> > 
> > Just found a minor problem:
> 
> "Minor" X-D

Well, polyfill is WIP, I don't consider bugs critical if we are not near a release :)

> 
> 
> > Start Tux Paint, select the Polygon Fill tool.
> > select another tool and draw something.
> > Return to the Polygon Fill tool, see how everything you had drawn is lost.
> > 
> > The problem is polyfill_snapshot being kept and later blit into the canvas via
> > set_color() -> draw_preview() before switchin()
> > 
> > We could clean up polyfill_snapshot at switchout() and recreate it at each swcithin() or
> > we could set some marker, mark it at switchout, clean it up at switchin() and only blit polyfill_snapshot
> > if the marker allows it, that would allow to reuse polyfill_snapshot and not destroy/recreate it
> > at each switchout()/switchin() cycle.
> 
> For now, I've set a variable to tell whether switchin() has been
> called yet.  To me, it seems strange that I'm calling a Magic tool's
> set_color() BEFORE its switchin() has been called.  But rather than
> change that (and potentially break lots of other tools), I've opened a
> ticket. :-)
> 
>   https://sourceforge.net/p/tuxpaint/bugs/286/
> 
> I also addressed an issue I already knew about, where the in-progress
> polygon preview (the lines and big red & green boxes) would remain on
> the canvas when switching to a different tool.


Oh, I thouth it was on purpose, to let children with something painted 
on the screen, in fact the Undo tool still recovers them.


> 
> I believe I solved that by simply blitting the tool's own snapshot
> back to the canvas on switchout().  (And since it makes a fresh
> snapshot after every polygon has been completed, this works.)
> 
> Adjustments like this may help some of the other Magic tools that
> "act weird" when you're in the middle of using them, but then
> switch away.
> 
> I've also thought it might be nice to provide a way for Magic
> tools to say "I can handle the 'Undo' command myself", e.g.
> for Filled Polygon tool, it would remove the most-recently
> added point.  Then, if there's no more work for them to do,
> they can tell Tux Paint that it should do it's normal Undo
> process.
> 
> Prior to that, I also thought it might be interesting to provide a
> way for Magic tools to provide an 'overlay' SDL_Surface which
> Tux Paint would draw on top everything else, rather than having
> to abuse the main `canvas` surface to draw previews, crosshairs,
> grids, etc.
> 
> Things are getting tricky. :)
> 
> -bill!


Currently magic tools that need multiple clicks to do their work 
are tricky, especially when faced to the Undo tool.

Pere


> 

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Bill K. <nb...@so...>]

On Sun, Apr 07, 2024 at 03:11:09PM +0200, Pere Pujal i Carabantes wrote:
> Hi all,
> 
> Tested the new polyfill stuff, it is nice :)

Hi Pere (and everyone),

I'd appreciate if you could test the Filled Polygon tool some more.
Also, please enjoy (and report any issues with!) some sound effects
that I just added.

There are different sounds for

 * adding a new point
 * dragging the new point / moving an existing one
 * dragging either end point next to the other, when 3+ points exist
   (you are "at risk" of completing the polygon ;) )
 * dragging one point into another, thus merging them
   (removing a point)
 * completing the polygon


I'm hoping to release a new version of Tux Paint soon,
ahead of a busy summer.  I'll post to -i18n and -maintainers
about this fact. :)

-bill!

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Pere P. i C. <per...@gm...>]

Hi Bill, and all,

A problem, start drawing a polygon and do as many points as possible (17?) 
then you are blocked, you can either switch out of the tool or drag the 
latest point to the origin closing the polygon, which is not very intuitive.
Assuming the only thing you can do in the tool is to close the polygon, 
I suggest to do this automatically.

Then there is the interaction with the Undo tool: draw say 5 points and go 
to another tool without closing the polygon, the helper lines and points 
disappear (by design?), hit Undo and they reappear, 
I'd suggest to either keep them in the drawing or erase them from the undo stack.

HTH
Pere

El dc. 29 de 05 de 2024 a les 00:05 -0700, en/na Bill Kendrick va escriure:
> On Sun, Apr 07, 2024 at 03:11:09PM +0200, Pere Pujal i Carabantes wrote:
> > Hi all,
> > 
> > Tested the new polyfill stuff, it is nice :)
> 
> Hi Pere (and everyone),
> 
> I'd appreciate if you could test the Filled Polygon tool some more.
> Also, please enjoy (and report any issues with!) some sound effects
> that I just added.
> 
> There are different sounds for
> 
>  * adding a new point
>  * dragging the new point / moving an existing one
>  * dragging either end point next to the other, when 3+ points exist
>    (you are "at risk" of completing the polygon ;) )
>  * dragging one point into another, thus merging them
>    (removing a point)
>  * completing the polygon
> 
> 
> I'm hoping to release a new version of Tux Paint soon,
> ahead of a busy summer.  I'll post to -i18n and -maintainers
> about this fact. :)
> 
> -bill!
> 
> 
> _______________________________________________
> Tuxpaint-devel mailing list
> Tux...@li...
> https://lists.sourceforge.net/lists/listinfo/tuxpaint-devel

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Pere P. i C. <per...@gm...>]

El dc. 29 de 05 de 2024 a les 10:06 +0200, en/na Pere Pujal i Carabantes va escriure:
> Hi Bill, and all,
> 
> A problem, start drawing a polygon and do as many points as possible (17?) 
> then you are blocked, you can either switch out of the tool or drag the 
> latest point to the origin closing the polygon, which is not very intuitive.
> Assuming the only thing you can do in the tool is to close the polygon, 
> I suggest to do this automatically.

I spoke too fast :( 
There is also the possibility of removing one or more points
that will let you be able to add some new ones.

Removing points is not mentioned in the UI tool description.


Pere


> 
> Then there is the interaction with the Undo tool: draw say 5 points and go 
> to another tool without closing the polygon, the helper lines and points 
> disappear (by design?), hit Undo and they reappear, 
> I'd suggest to either keep them in the drawing or erase them from the undo stack.
> 
> HTH
> Pere
> 
> El dc. 29 de 05 de 2024 a les 00:05 -0700, en/na Bill Kendrick va escriure:
> > On Sun, Apr 07, 2024 at 03:11:09PM +0200, Pere Pujal i Carabantes wrote:
> > > Hi all,
> > > 
> > > Tested the new polyfill stuff, it is nice :)
> > 
> > Hi Pere (and everyone),
> > 
> > I'd appreciate if you could test the Filled Polygon tool some more.
> > Also, please enjoy (and report any issues with!) some sound effects
> > that I just added.
> > 
> > There are different sounds for
> > 
> >  * adding a new point
> >  * dragging the new point / moving an existing one
> >  * dragging either end point next to the other, when 3+ points exist
> >    (you are "at risk" of completing the polygon ;) )
> >  * dragging one point into another, thus merging them
> >    (removing a point)
> >  * completing the polygon
> > 
> > 
> > I'm hoping to release a new version of Tux Paint soon,
> > ahead of a busy summer.  I'll post to -i18n and -maintainers
> > about this fact. :)
> > 
> > -bill!
> > 
> > 
> > _______________________________________________
> > Tuxpaint-devel mailing list
> > Tux...@li...
> > https://lists.sourceforge.net/lists/listinfo/tuxpaint-devel
> 

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Bill K. <nb...@so...>]

On Wed, May 29, 2024 at 10:06:07AM +0200, Pere Pujal i Carabantes wrote:
> Hi Bill, and all,
> 
> A problem, start drawing a polygon and do as many points as possible (17?) 
> then you are blocked, you can either switch out of the tool or drag the 
> latest point to the origin closing the polygon, which is not very intuitive.
> Assuming the only thing you can do in the tool is to close the polygon, 
> I suggest to do this automatically.

I think we can raise the number of points.  In the meantime, I updated it
so that clicking again will simply move the end point (rather than silently
and confusingly doing _nothing_).


> Then there is the interaction with the Undo tool: draw say 5 points and go 
> to another tool without closing the polygon, the helper lines and points 
> disappear (by design?), hit Undo and they reappear, 
> I'd suggest to either keep them in the drawing or erase them from the undo stack.

Okay, I've made an attempt to prevent the previews from landing in the
undo history.  This is via a new Magic API function that magic tools may
call: api->retract_undo().  It basically just rolls back `cur_undo` and
`newest_undo` by one, so whatever was just snapped (during a mousedown
event, just prior to the Magic tool's `_click()` from being called)
gets "forgotten".

So for example, I can click a bunch of times to make a new polygon,
and when I hit Undo, it goes back to the canvas as it appeared before
I added the very first point.  And when I hit Redo, the entire polygon
reappears in its final (non-preview) form.

HOPEFULLY this works well.  It's almost 2am, and I was tired many hours
_before_ I tried tackling this. :-D

Before I go to bed I'm going to add info about this to the API docs.
Hopefully in the next few days I can see about utilizing this with
other Magic tools that offer "previews" that live beyond the
(1) click, (2) drag, (3) release lifespan (e.g., Clone, and the
Perspective drawing tools)... so they will stop "corrupting" the
Undo history with their outline and crosshair nonsense. ;-)

This is quite a hack, I admit, so wish me luck!

And PLEASE TEST!  Bang on it as much as you can, and report back!

Thanks,

PS - I also updated the description to mention that points may be
merged.  (Sorry to translators for asking for updated localizations,
and then immediately going in and messing with strings :-[ )

PPS - The commit (so far) that adds this new functionality is:
https://sourceforge.net/p/tuxpaint/tuxpaint/ci/01afb5846c2b9d7c58961d38b27cd597d367d766/
("api->retract_undo() added to Magic API; used by Filled Polygon")

-bill!

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Pere P. i C. <per...@gm...>]

El ds. 01 de 06 de 2024 a les 01:48 -0700, en/na Bill Kendrick va escriure:
> On Wed, May 29, 2024 at 10:06:07AM +0200, Pere Pujal i Carabantes wrote:
> > Hi Bill, and all,
> > 
> > A problem, start drawing a polygon and do as many points as possible (17?) 
> > then you are blocked, you can either switch out of the tool or drag the 
> > latest point to the origin closing the polygon, which is not very intuitive.
> > Assuming the only thing you can do in the tool is to close the polygon, 
> > I suggest to do this automatically.
> 
> I think we can raise the number of points.  In the meantime, I updated it
> so that clicking again will simply move the end point (rather than silently
> and confusingly doing _nothing_).

Better :)
Could be improved if a click in the starting point closes the polygon, 
as it is now you have to click outside the starting point and drag into it,
the following works for me.


git diff
diff --git a/magic/src/polyfill.c b/magic/src/polyfill.c
index 25e4cc436..6f1d51bd6 100644
--- a/magic/src/polyfill.c
+++ b/magic/src/polyfill.c
@@ -401,7 +401,7 @@ polyfill_release(magic_api * api, int which ATTRIBUTE_UNUSED,
   /* If they simply clicked the first point (without
      drawing to move it), and there are enough points, consider
      it a final placement of a new point! */
-  if (polyfill_editing == 0 && polyfill_dragged == 0 && polyfill_num_pts > 2 && polyfill_num_pts < MAX_PTS)
+  if (polyfill_editing == 0 && polyfill_dragged == 0 && polyfill_num_pts > 2 && polyfill_num_pts <= MAX_PTS)
   {
 #ifdef DEBUG
     printf("Clicked first point to end polygon!\n");



> 
> > Then there is the interaction with the Undo tool: draw say 5 points and go 
> > to another tool without closing the polygon, the helper lines and points 
> > disappear (by design?), hit Undo and they reappear, 
> > I'd suggest to either keep them in the drawing or erase them from the undo stack.
> 
> Okay, I've made an attempt to prevent the previews from landing in the
> undo history.  This is via a new Magic API function that magic tools may
> call: api->retract_undo().  It basically just rolls back `cur_undo` and
> `newest_undo` by one, so whatever was just snapped (during a mousedown
> event, just prior to the Magic tool's `_click()` from being called)
> gets "forgotten".
> 
> So for example, I can click a bunch of times to make a new polygon,
> and when I hit Undo, it goes back to the canvas as it appeared before
> I added the very first point.  And when I hit Redo, the entire polygon
> reappears in its final (non-preview) form.
> 
> HOPEFULLY this works well.  It's almost 2am, and I was tired many hours
> _before_ I tried tackling this. :-D
> 
> Before I go to bed I'm going to add info about this to the API docs.
> Hopefully in the next few days I can see about utilizing this with
> other Magic tools that offer "previews" that live beyond the
> (1) click, (2) drag, (3) release lifespan (e.g., Clone, and the
> Perspective drawing tools)... so they will stop "corrupting" the
> Undo history with their outline and crosshair nonsense. ;-)
> 
> This is quite a hack, I admit, so wish me luck!
> 
> And PLEASE TEST!  Bang on it as much as you can, and report back!

Works fine as far as I've tested :), the only improvement I could imagine 
is to use the first Undo just to clear the auxiliary stuff, 
withouth undoing previous work, other Undo acting as of now.

Best
Pere

2am, why this doesn't surprises me? XDD

> 
> Thanks,
> 
> PS - I also updated the description to mention that points may be
> merged.  (Sorry to translators for asking for updated localizations,
> and then immediately going in and messing with strings :-[ )
> 
> PPS - The commit (so far) that adds this new functionality is:
> https://sourceforge.net/p/tuxpaint/tuxpaint/ci/01afb5846c2b9d7c58961d38b27cd597d367d766/
> ("api->retract_undo() added to Magic API; used by Filled Polygon")
> 
> -bill!
> 
> 
> 
> _______________________________________________
> Tuxpaint-devel mailing list
> Tux...@li...
> https://lists.sourceforge.net/lists/listinfo/tuxpaint-devel

Re: [Tuxpaint-devel] Polygon fill messing with other tools drawing

[Bill K. <nb...@so...>]

On Sat, Jun 01, 2024 at 03:48:09PM +0200, Pere Pujal i Carabantes wrote:
<snip>
> Better :)
> Could be improved if a click in the starting point closes the polygon, 
> as it is now you have to click outside the starting point and drag into it,
> the following works for me.

Thanks!  For safety, I made the two arrays larger by one, since the
last thing that happens is one _more_ point is created, which has
the same position as the first point (index 0).  So if I were to add
a new point at MAX_PTS index into the array, that would go beyond
its capacity, otherwise.

I also bumped the number of points up to 100. (I did that last night
but forgot to commit. Zzzz)


<snip>
> Works fine as far as I've tested :), the only improvement I could imagine 
> is to use the first Undo just to clear the auxiliary stuff, 
> withouth undoing previous work, other Undo acting as of now.

I think I understand what you mean.  I need to play with it
more to make sure, and then I'll see what I can do.  At this
point, I'm glad that it seems to be working _at all_, though...
so any further improvements are just a bonus, IMO. :-D

 
> 2am, why this doesn't surprises me? XDD

It's the only time I get to really hack on Tux Paint these days.
Maybe once I retire, I can stop being an open source vampire. ;)

-bill!