Hello all,
I tracked down an annoying problem in one of applications to the Lasso widget I was using. The widget constructor lets you specify a function to call when the lasso operation is complete. So, when I create a Lasso, I set the canvas’s widget lock to the new lasso, and the release function will unlock it when it is done. What would occassionally happen is that the canvas wouldn’t get unlocked and I wouldn’t be able to use any other widget tools.
It turns out that the release function is not called if the number of vertices collected is not more than 2. So, accidental clicks that activate the lasso never get cleaned up. Because of this design, it would be impossible to guarantee a proper cleanup. One could add another button_release callback to clean up if the canvas is still locked, but there is no guarantee that that callback is not called before the lasso’s callback, thereby creating a race condition.
The only solution I see is to guarantee that the release callback will be called regardless of the length of the vertices array. Does anybody see a problem with that?
Cheers!
Ben Root
Not having looked at the Lasso code, wouldn't it be possible to use
one internal callback for the button_release event, and have this
callback call the users' callbacks if points > 2 and always handle the
unlocking of the canvas?
Ryan
···
On Fri, Feb 17, 2012 at 10:14 AM, Benjamin Root <ben.root@...553...> wrote:
Hello all,
I tracked down an annoying problem in one of applications to the Lasso
widget I was using. The widget constructor lets you specify a function to
call when the lasso operation is complete. So, when I create a Lasso, I set
the canvas's widget lock to the new lasso, and the release function will
unlock it when it is done. What would occassionally happen is that the
canvas wouldn't get unlocked and I wouldn't be able to use any other widget
tools.
It turns out that the release function is not called if the number of
vertices collected is not more than 2. So, accidental clicks that activate
the lasso never get cleaned up. Because of this design, it would be
impossible to guarantee a proper cleanup. One could add another
button_release callback to clean up if the canvas is still locked, but there
is no guarantee that that callback is not called before the lasso's
callback, thereby creating a race condition.
The only solution I see is to guarantee that the release callback will be
called regardless of the length of the vertices array. Does anybody see a
problem with that?
--
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma
The problem is that the constructor does not establish the lock. It is the user’s responsibility to establish a lock and release the locks for these widgets. Plus, if the user’s callback has cleanup code (such as mine did), not guaranteeing that the callback is done can leave behind a mess.
Now, if we were to change the paradigm so that the Widget class establishes and releases the lock, and that the user should never handle that, then that might be a partial solution, but still leaves unsolved the user’s cleanup needs.
Ben Root
···
On Fri, Feb 17, 2012 at 11:06 AM, Ryan May <rmay31@…761…> wrote:
On Fri, Feb 17, 2012 at 10:14 AM, Benjamin Root <ben.root@…553…> wrote:
Hello all,
I tracked down an annoying problem in one of applications to the Lasso
widget I was using. The widget constructor lets you specify a function to
call when the lasso operation is complete. So, when I create a Lasso, I set
the canvas’s widget lock to the new lasso, and the release function will
unlock it when it is done. What would occassionally happen is that the
canvas wouldn’t get unlocked and I wouldn’t be able to use any other widget
tools.
It turns out that the release function is not called if the number of
vertices collected is not more than 2. So, accidental clicks that activate
the lasso never get cleaned up. Because of this design, it would be
impossible to guarantee a proper cleanup. One could add another
button_release callback to clean up if the canvas is still locked, but there
is no guarantee that that callback is not called before the lasso’s
callback, thereby creating a race condition.
The only solution I see is to guarantee that the release callback will be
called regardless of the length of the vertices array. Does anybody see a
problem with that?
Not having looked at the Lasso code, wouldn’t it be possible to use
one internal callback for the button_release event, and have this
callback call the users’ callbacks if points > 2 and always handle the
unlocking of the canvas?
Ryan
I just started looking at the Lasso widget for some other changes, and I agree: the points > 2 check should be removed.
As for the design of Lasso, I don’t quite understand why it disconnects the callbacks after a single call. If the onpress event from the lasso demo was made a method of Lasso, then you could reuse the Lasso widget instead of having to create a new one for each selection. Of course, this might complicate the widget lock used in the demo, but that’s true for all other widgets, right?
-Tony
···
On Fri, Feb 17, 2012 at 12:17 PM, Benjamin Root <ben.root@…553…> wrote:
On Fri, Feb 17, 2012 at 11:06 AM, Ryan May <rmay31@…149…> wrote:
On Fri, Feb 17, 2012 at 10:14 AM, Benjamin Root <ben.root@…553…> wrote:
Hello all,
I tracked down an annoying problem in one of applications to the Lasso
widget I was using. The widget constructor lets you specify a function to
call when the lasso operation is complete. So, when I create a Lasso, I set
the canvas’s widget lock to the new lasso, and the release function will
unlock it when it is done. What would occassionally happen is that the
canvas wouldn’t get unlocked and I wouldn’t be able to use any other widget
tools.
It turns out that the release function is not called if the number of
vertices collected is not more than 2. So, accidental clicks that activate
the lasso never get cleaned up. Because of this design, it would be
impossible to guarantee a proper cleanup. One could add another
button_release callback to clean up if the canvas is still locked, but there
is no guarantee that that callback is not called before the lasso’s
callback, thereby creating a race condition.
The only solution I see is to guarantee that the release callback will be
called regardless of the length of the vertices array. Does anybody see a
problem with that?
Not having looked at the Lasso code, wouldn’t it be possible to use
one internal callback for the button_release event, and have this
callback call the users’ callbacks if points > 2 and always handle the
unlocking of the canvas?
Ryan
The problem is that the constructor does not establish the lock. It is the user’s responsibility to establish a lock and release the locks for these widgets. Plus, if the user’s callback has cleanup code (such as mine did), not guaranteeing that the callback is done can leave behind a mess.
Now, if we were to change the paradigm so that the Widget class establishes and releases the lock, and that the user should never handle that, then that might be a partial solution, but still leaves unsolved the user’s cleanup needs.
Ben Root
The Lasso disconnects itself after the button_release event because that’s what indicates that you are done. The user gets back a single Line2D object that is assumed to represent a single path with no breaks. Reusing the Lasso widget would be a situation that would require a different idiom for user interaction. I wouldn’t be against a “MultiLasso” widget that works differently from Lasso, but I really wouldn’t want to make changes to existing user widgets. It is iffy enough about whether to remove the point-count-check.
Ben Root
···
On Mon, Feb 27, 2012 at 9:45 PM, Tony Yu <tsyu80@…149…> wrote:
On Fri, Feb 17, 2012 at 12:17 PM, Benjamin Root <ben.root@…553…> wrote:
On Fri, Feb 17, 2012 at 11:06 AM, Ryan May <rmay31@…149…> wrote:
On Fri, Feb 17, 2012 at 10:14 AM, Benjamin Root <ben.root@…553…> wrote:
Hello all,
I tracked down an annoying problem in one of applications to the Lasso
widget I was using. The widget constructor lets you specify a function to
call when the lasso operation is complete. So, when I create a Lasso, I set
the canvas’s widget lock to the new lasso, and the release function will
unlock it when it is done. What would occassionally happen is that the
canvas wouldn’t get unlocked and I wouldn’t be able to use any other widget
tools.
It turns out that the release function is not called if the number of
vertices collected is not more than 2. So, accidental clicks that activate
the lasso never get cleaned up. Because of this design, it would be
impossible to guarantee a proper cleanup. One could add another
button_release callback to clean up if the canvas is still locked, but there
is no guarantee that that callback is not called before the lasso’s
callback, thereby creating a race condition.
The only solution I see is to guarantee that the release callback will be
called regardless of the length of the vertices array. Does anybody see a
problem with that?
Not having looked at the Lasso code, wouldn’t it be possible to use
one internal callback for the button_release event, and have this
callback call the users’ callbacks if points > 2 and always handle the
unlocking of the canvas?
Ryan
The problem is that the constructor does not establish the lock. It is the user’s responsibility to establish a lock and release the locks for these widgets. Plus, if the user’s callback has cleanup code (such as mine did), not guaranteeing that the callback is done can leave behind a mess.
Now, if we were to change the paradigm so that the Widget class establishes and releases the lock, and that the user should never handle that, then that might be a partial solution, but still leaves unsolved the user’s cleanup needs.
Ben Root
I just started looking at the Lasso widget for some other changes, and I agree: the points > 2 check should be removed.
As for the design of Lasso, I don’t quite understand why it disconnects the callbacks after a single call. If the onpress event from the lasso demo was made a method of Lasso, then you could reuse the Lasso widget instead of having to create a new one for each selection. Of course, this might complicate the widget lock used in the demo, but that’s true for all other widgets, right?
-Tony
I added the point count check according to git blame and I don’t remember why but I agree is doesn’t look like it makes sense to me. I think this lasso is lightly used based on the volume of questions we get about it. One reason it may be lightly used is because it is hard to work with. If we can improve it, even changing functionality, I would be in favor. Eg, allowing the same Lasso to handle repeated selections makes sense to me. If the change looks too onerous, we could call it Lasso2 and deprecate the former.
···
On Tue, Feb 28, 2012 at 10:44 AM, Benjamin Root <ben.root@…553…> wrote:
The Lasso disconnects itself after the button_release event because that’s what indicates that you are done. The user gets back a single Line2D object that is assumed to represent a single path with no breaks. Reusing the Lasso widget would be a situation that would require a different idiom for user interaction. I wouldn’t be against a “MultiLasso” widget that works differently from Lasso, but I really wouldn’t want to make changes to existing user widgets. It is iffy enough about whether to remove the point-count-check.
I think that logic is slightly faulty. Another reason why we don’t get a lot of questions about it could be because it does (almost) exactly what users want. Currently, I use the Lasso widget in my track_maker project to select storm cells in a movie of radar images. Because I have multiple modes to the program, a persistent Lasso object makes little sense. I also want the Lasso’s line to disappear after I am done selecting it so that I can put in a polygon patch of my coloring and hatching.
I am skeptical of any redesign of Lasso. I am willing to see what others have in mind, but I still think that it might better belong in a new class “MultiLasso”. I see no reason to deprecate the current Lasso (only to fix it).
Cheers!
Ben Root
···
On Tue, Feb 28, 2012 at 10:58 AM, John Hunter <jdh2358@…272…149…> wrote:
On Tue, Feb 28, 2012 at 10:44 AM, Benjamin Root <ben.root@…553…> wrote:
The Lasso disconnects itself after the button_release event because that’s what indicates that you are done. The user gets back a single Line2D object that is assumed to represent a single path with no breaks. Reusing the Lasso widget would be a situation that would require a different idiom for user interaction. I wouldn’t be against a “MultiLasso” widget that works differently from Lasso, but I really wouldn’t want to make changes to existing user widgets. It is iffy enough about whether to remove the point-count-check.
I added the point count check according to git blame and I don’t remember why but I agree is doesn’t look like it makes sense to me. I think this lasso is lightly used based on the volume of questions we get about it. One reason it may be lightly used is because it is hard to work with. If we can improve it, even changing functionality, I would be in favor. Eg, allowing the same Lasso to handle repeated selections makes sense to me. If the change looks too onerous, we could call it Lasso2 and deprecate the former.