Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lasso tool for faster drawing of polygonal Shapes #5555

Merged
merged 93 commits into from
May 27, 2023

Conversation

LucaMarconato
Copy link
Contributor

@LucaMarconato LucaMarconato commented Feb 11, 2023

Description

This PR adds a polygon lasso tool for drawing complex polygons which are added to a shape layer. The tool does not require the user to click for adding each specific vertex as is the case with the polygon tool. This PR features the following:
• A new shape layer button which can be activated by keyboard shortcut shift + P
• A polygon lasso mode which can be used for drawing with the mouse and tablet

For mouse draw mode the sequence of events by the user is as follows:

  1. Mouse press
  2. Move mouse while not mouse pressed
  3. Mouse press to finish draw

For tablet mode, the shape is created by a mouse (ie stylus) click+drag event. Drawing finishes when the stylus is lifted / on mouse release.

For both modes, vertices are added only if the vertex to be added is at least 10 screen pixels away from the previous vertex. This is a setting configurable in advanced settings.

An implementation of the Ramer–Douglas–Peucker algorithm is used to reduce the number of vertices when a drawing is finished. The aggressiveness of reducing the number of vertices is dependent on a parameter epsilon. If set to 0, no vertices are removed. The higher the epsilon, the more vertices are removed. The epsilon parameter can be configured in napari preferences -> experimental -> rdp_epsilon.

Lastly, a refactor has been performed for various functions related to polygon drawing.

References

We acknowledge @yozhikoff who made a first prototype of this new feature.

@github-actions github-actions bot added design Design discussion qt Relates to qt labels Feb 11, 2023
@codecov
Copy link

codecov bot commented Feb 11, 2023

Codecov Report

Merging #5555 (37ab653) into main (48626e8) will increase coverage by 0.03%.
The diff coverage is 99.41%.

@@            Coverage Diff             @@
##             main    #5555      +/-   ##
==========================================
+ Coverage   89.99%   90.02%   +0.03%     
==========================================
  Files         618      618              
  Lines       52699    52827     +128     
==========================================
+ Hits        47424    47557     +133     
+ Misses       5275     5270       -5     
Impacted Files Coverage Δ
napari/utils/shortcuts.py 100.00% <ø> (ø)
napari/layers/shapes/_shapes_mouse_bindings.py 89.58% <98.43%> (+0.77%) ⬆️
napari/_qt/layer_controls/qt_shapes_controls.py 95.91% <100.00%> (+0.08%) ⬆️
napari/layers/shapes/_shapes_constants.py 100.00% <100.00%> (ø)
napari/layers/shapes/_shapes_key_bindings.py 100.00% <100.00%> (ø)
napari/layers/shapes/_shapes_utils.py 85.43% <100.00%> (+0.91%) ⬆️
...layers/shapes/_tests/test_shapes_mouse_bindings.py 92.41% <100.00%> (+1.09%) ⬆️
napari/layers/shapes/_tests/test_shapes_utils.py 98.11% <100.00%> (+0.89%) ⬆️
napari/layers/shapes/shapes.py 92.29% <100.00%> (+0.04%) ⬆️
napari/settings/_experimental.py 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

@melonora
Copy link
Contributor

I was wondering when I would see you add this one haha. Neat! Maybe one thing that we could think about is adding a mode that can be activated by a key press, which conserves the very smooth drawing polygon experience, but calculates an approximation of the vertices by means of the RDP algorithm after the drawing is completed in order to reduce the number of data points. What do you think?

@LucaMarconato
Copy link
Contributor Author

Hi @melonora, well spotted! 😁 Good idea to use the RDP algorithm! (Maybe that could even be the default behavior?)

@melonora
Copy link
Contributor

Hi @melonora, well spotted! 😁 Good idea to use the RDP algorithm! (Maybe that could even be the default behavior?)

I would like that as the default behaviour, but lets also see what the others say.

@melonora
Copy link
Contributor

Perhaps we could even do this before the drawing is completed by means of a moving window approach. Not certain whether this would be possible but it could improve the performance of drawing large polygons using this approach. I notice the performance dropping significantly as the polygons get larger while drawing. Would you like to look into this yourself? I would also be happy to explore. I would have time on Wednesday.

@LucaMarconato
Copy link
Contributor Author

Interesting, it would be worth to give it a try! You are right about the performance. I won't have time to look into this soon unfortunately, if you have time to continue the work it would be super! Regarding the user experience, I think that modifying the default shape tool without adding a new lasso button would be great. This requires to respond differently to the mouse events.

@LucaMarconato LucaMarconato marked this pull request as draft February 13, 2023 11:45
Copy link
Contributor

@brisvag brisvag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this! I was actually trying to do the same a few weeks ago, but got stuck, so it's great to see this :)

In general, I would prever to have this as an alternative behaviour of the polygon tool. I love option 1., it feels very intuitive. It's indeed a bit harder to implement, but hopefully you can get past where I got stuck :P

One thing to note is that I recently added spline interpolation to the polygon shape, which is currently not exposed by should work if exposed. There is some discussion in that PR (and its predecessors) about how to expose this exactly, but nothing conclusive. It might be useful to consider here, since I expect most use cases where lasso drawing is useful would prefer to have smooth shapes rather than jagged. (note however that the smoothing is only visual for now, and there is no way to currently get it out at higher levels, for example to generate labels).

_move(layer, coordinates)

now = datetime.datetime.now()
MAX_POINTS_PER_SECOND = 60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My instinct tells me that this should be distance based rather than time-based. Specifically, canvas distance (say, 5 pixels), to make sure zooming in and out stays intuitive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree

@brisvag
Copy link
Contributor

brisvag commented Feb 13, 2023

I notice the performance dropping significantly as the polygons get larger while drawing

This is really only the case when the poligon is filled. This is because it needs to generate a bunch of triangles that are given as a mesh to vispy to do the visualisation... We could probably handle all this in the shader, but we'd have to create a new visual :)

@melonora
Copy link
Contributor

Love this! I was actually trying to do the same a few weeks ago, but got stuck, so it's great to see this :)

In general, I would prever to have this as an alternative behaviour of the polygon tool. I love option 1., it feels very intuitive. It's indeed a bit harder to implement, but hopefully you can get past where I got stuck :P

One thing to note is that I recently added spline interpolation to the polygon shape, which is currently not exposed by should work if exposed. There is some discussion in that PR (and its predecessors) about how to expose this exactly, but nothing conclusive. It might be useful to consider here, since I expect most use cases where lasso drawing is useful would prefer to have smooth shapes rather than jagged. (note however that the smoothing is only visual for now, and there is no way to currently get it out at higher levels, for example to generate labels).

I will experiment with both, dependent on the epsilon setting of RDP my experience has been that it is not as jagged, but if we can easily improve that even more that would be nice. From my understanding it would require storing this

I notice the performance dropping significantly as the polygons get larger while drawing

This is really only the case when the poligon is filled. This is because it needs to generate a bunch of triangles that are given as a mesh to vispy to do the visualisation... We could probably handle all this in the shader, but we'd have to create a new visual :)

I see, yeah I saw it in vispy.geometry.add_tri . There would be a usecase for this though when annotating large tissue regions this way, but that would perhaps be something for a separate pr. In any case less data points would be nice.

@LucaMarconato
Copy link
Contributor Author

Thanks for the feedback. Regarding the filled shape, one option could be to fill the shape only when the drawing ends. I think the filling could actually disturb the user when annotating complex anatomical regions because it could partially hide what's behind the shape.

@melonora
Copy link
Contributor

Thanks for the feedback. Regarding the filled shape, one option could be to fill the shape only when the drawing ends. I think the filling could actually disturb the user when annotating complex anatomical regions because it could partially hide what's behind the shape.

One thing that I also had been thinking of is using the opacity only for the filling but then keeping the outline visual. In general in tissues any kind of cell annotation like this becomes more challenging otherwise, particularly when dealing with overlapping cells. Right now opacity both affects the filling and edge.

@melonora
Copy link
Contributor

melonora commented Feb 22, 2023

The rdp algorithm is functional now and also distance of current cursor position to last cursor position is used to define when vertices get added. There is currently still a bug when first drawing polygons in 2d and later in 3d and I will change the mouse interaction as we have talked about it.

On top of that I will also add some tests.

elif event.type == 'mouse_press' and layer._mode == Mode.ADD_POLYGON_LASSO:
index = layer._moving_value[0]
vertices = layer._data_view.shapes[index].data
vertices = rdp(vertices, epsilon=0.5)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One point of discussion is whether we should expose this epsilon parameter to the user somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I would expose, but I would make some empirical tests of drawing curves slower or faster and with more or less curvature/sharp corners/wiggling, and empirically find a reasonable epsilon.

Copy link
Contributor

@melonora melonora Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did that for regular cell shapes. However, the choice of this parameter does depend on what you are trying to annotate and how accurate you need it to be.

Drawing slower or faster does not matter at all. The vertices are added based on a distance threshold of 5 pixels.

@melonora
Copy link
Contributor

Love this! I was actually trying to do the same a few weeks ago, but got stuck, so it's great to see this :)

In general, I would prever to have this as an alternative behaviour of the polygon tool. I love option 1., it feels very intuitive. It's indeed a bit harder to implement, but hopefully you can get past where I got stuck :P

One thing to note is that I recently added spline interpolation to the polygon shape, which is currently not exposed by should work if exposed. There is some discussion in that PR (and its predecessors) about how to expose this exactly, but nothing conclusive. It might be useful to consider here, since I expect most use cases where lasso drawing is useful would prefer to have smooth shapes rather than jagged. (note however that the smoothing is only visual for now, and there is no way to currently get it out at higher levels, for example to generate labels).

I had been thinking about this, while the user might not want to have jagged shapes it gives a clearer view of what the polygon data looks like, so I could add it as optional. I would say that if there is the thought of not making it only visual then rdp and interpolation should be mutually exclusive.

@@ -1144,3 +1149,70 @@ def validate_num_vertices(
shape_length=len(shape),
)
)


def perpendicular_distance(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

return np.linalg.norm(t * (line_start - line_end) + line_end - point)


def rdp(vertices: npt.NDArray, epsilon: float) -> npt.NDArray:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the algorithm, so I skip the review of this function.

Copy link
Contributor

@brisvag brisvag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I understanding correctly that the rdp use here is changing the actual points used to define the shape? In that case, I don't think it's mutually exclusive with the spline interpolation (which is only a visualisation tool).

@melonora
Copy link
Contributor

Am I understanding correctly that the rdp use here is changing the actual points used to define the shape? In that case, I don't think it's mutually exclusive with the spline interpolation (which is only a visualisation tool).

Yes you understand correctly:) I agree if the interpolation is purely for visualization purposes then it is not mutually exclusive. If we were to use it to interpolate between datapoints and add those new datapoints then it would be as rdp aims to reduce the number of datapoints while interpolation aims to add.

@andy-sweet
Copy link
Member

Few remarks. I will try to make a more detailed review tomorrow/Friday.

@Czaki : just a gentle reminder about this from the PR party. I also added the async-update label.

In general, there are lots of commenters and comments here, so it would be good to understand if there are any big blockers here.

@jni jni added the ready to merge Last chance for comments! Will be merged in ~24h label May 26, 2023
@jni jni changed the title adding lasso tool for drawing polygonal shapes Add lasso tool for faster drawing of polygonal Shapes May 27, 2023
@jni
Copy link
Member

jni commented May 27, 2023

I think all of @Czaki's concerns have been addressed, including the big one which was about the distance between points being a preference. So, all systems green, in it goes! 🚀 I've updated the PR title and description to describe the final state of this PR at merge time.

Thank you @LucaMarconato and @melonora for your amazing patience and persistence with this PR! I'm really excited to get it into users' hands!

@jni jni merged commit bd69fd2 into napari:main May 27, 2023
31 checks passed
@jni jni removed pr-party-update-async ready to merge Last chance for comments! Will be merged in ~24h labels May 27, 2023
@brisvag
Copy link
Contributor

brisvag commented May 30, 2023

Just tried this, friggin awesome 😍! I wanted to use it for paths, but it seems like I can only do polygons? Or is there an option someewhere that I'm missing?

Also, something that I thought would be nice as followup: the ability to modify existing polygon/paths in "lasso mode": basically a combination of "add vertex" and "remove vertex" modes, but dragging. You could "reroute" the curve in a certain radius from the mouse... Probably complicated to implement, but would be way smoother to work with than the add/remove tool.

@melonora
Copy link
Contributor

@brisvag Thanks! Yes at the moment it was only for polygons, but can add it for paths too. At the moment I did not do it yet because of the discussion with extra buttons (should this also be an extra button just as with the polygon lasso tool or not) and the discussion in #5806. What I thought would be simplest for dragging / removing is merge and erase mode for this, but @ksofiyuk thought it would be more difficult to implement. Shapely is able to do this as far as I know, but we are not currently using it.

@jni @brisvag If lasso paths are useful enough and we can quickly decide on the user interface I can have this ready still before next release.

@jni
Copy link
Member

jni commented May 30, 2023

Yes I think path drawing is a great plan. I guess new button is easiest. The editing stuff seems harder to figure out so I'd leave it for a future discussion. Perhaps you can open an issue for that @brisvag ?

@psobolewskiPhD
Copy link
Member

The other option—once two tools are affected—is to add a corner indicator that you can right click the button to change the tool behavior. This keeps the number of buttons more manageable.

psobolewskiPhD added a commit to napari/docs that referenced this pull request May 30, 2023
# Description
This PR adds documentation for the Shapes layer polygon lasso tool that was added
in napari [#5555](napari/napari#5555)

## Type of change
- [x] Fixes or improves existing content

# References
depends on napari/napari#5555

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
@LucaMarconato LucaMarconato deleted the polygon_lasso branch June 2, 2023 13:50
@Czaki Czaki mentioned this pull request Jun 7, 2023
Czaki pushed a commit that referenced this pull request Jun 19, 2023
This PR adds a polygon lasso tool for drawing complex polygons which are
added to a shape layer. The tool does not require the user to click for
adding each specific vertex as is the case with the polygon tool. This
PR features the following:
• A new shape layer button which can be activated by keyboard shortcut
shift + P
• A polygon lasso mode which can be used for drawing with the mouse and
tablet

For mouse draw mode the sequence of events by the user is as follows:

1. Mouse press
2. Move mouse while not mouse pressed
3. Mouse press to finish draw

For tablet mode, the shape is created by a mouse (ie stylus) click+drag
event. Drawing finishes when the stylus is lifted / on mouse release.

For both modes, vertices are added only if the vertex to be added is at
least 10 screen pixels away from the previous vertex. This is a setting
configurable in advanced settings.

An implementation of the Ramer–Douglas–Peucker algorithm is used to
reduce the number of vertices when a drawing is finished. The
aggressiveness of reducing the number of vertices is dependent on a
parameter epsilon. If set to 0, no vertices are removed. The higher the
epsilon, the more vertices are removed. The epsilon parameter can be
configured in napari preferences -> experimental -> rdp_epsilon.

Lastly, a refactor has been performed for various functions related to
polygon drawing.

We acknowledge @yozhikoff who made a first prototype of this new
feature.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: wmv_hpomen <w-mv@hotmail.com>
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
This PR adds a polygon lasso tool for drawing complex polygons which are
added to a shape layer. The tool does not require the user to click for
adding each specific vertex as is the case with the polygon tool. This
PR features the following:
• A new shape layer button which can be activated by keyboard shortcut
shift + P
• A polygon lasso mode which can be used for drawing with the mouse and
tablet

For mouse draw mode the sequence of events by the user is as follows:

1. Mouse press
2. Move mouse while not mouse pressed
3. Mouse press to finish draw

For tablet mode, the shape is created by a mouse (ie stylus) click+drag
event. Drawing finishes when the stylus is lifted / on mouse release.

For both modes, vertices are added only if the vertex to be added is at
least 10 screen pixels away from the previous vertex. This is a setting
configurable in advanced settings.

An implementation of the Ramer–Douglas–Peucker algorithm is used to
reduce the number of vertices when a drawing is finished. The
aggressiveness of reducing the number of vertices is dependent on a
parameter epsilon. If set to 0, no vertices are removed. The higher the
epsilon, the more vertices are removed. The epsilon parameter can be
configured in napari preferences -> experimental -> rdp_epsilon.

Lastly, a refactor has been performed for various functions related to
polygon drawing.

We acknowledge @yozhikoff who made a first prototype of this new
feature.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: wmv_hpomen <w-mv@hotmail.com>
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
This PR adds a polygon lasso tool for drawing complex polygons which are
added to a shape layer. The tool does not require the user to click for
adding each specific vertex as is the case with the polygon tool. This
PR features the following:
• A new shape layer button which can be activated by keyboard shortcut
shift + P
• A polygon lasso mode which can be used for drawing with the mouse and
tablet

For mouse draw mode the sequence of events by the user is as follows:

1. Mouse press
2. Move mouse while not mouse pressed
3. Mouse press to finish draw

For tablet mode, the shape is created by a mouse (ie stylus) click+drag
event. Drawing finishes when the stylus is lifted / on mouse release.

For both modes, vertices are added only if the vertex to be added is at
least 10 screen pixels away from the previous vertex. This is a setting
configurable in advanced settings.

An implementation of the Ramer–Douglas–Peucker algorithm is used to
reduce the number of vertices when a drawing is finished. The
aggressiveness of reducing the number of vertices is dependent on a
parameter epsilon. If set to 0, no vertices are removed. The higher the
epsilon, the more vertices are removed. The epsilon parameter can be
configured in napari preferences -> experimental -> rdp_epsilon.

Lastly, a refactor has been performed for various functions related to
polygon drawing.

We acknowledge @yozhikoff who made a first prototype of this new
feature.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: wmv_hpomen <w-mv@hotmail.com>
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
This PR adds a polygon lasso tool for drawing complex polygons which are
added to a shape layer. The tool does not require the user to click for
adding each specific vertex as is the case with the polygon tool. This
PR features the following:
• A new shape layer button which can be activated by keyboard shortcut
shift + P
• A polygon lasso mode which can be used for drawing with the mouse and
tablet

For mouse draw mode the sequence of events by the user is as follows:

1. Mouse press
2. Move mouse while not mouse pressed
3. Mouse press to finish draw

For tablet mode, the shape is created by a mouse (ie stylus) click+drag
event. Drawing finishes when the stylus is lifted / on mouse release.

For both modes, vertices are added only if the vertex to be added is at
least 10 screen pixels away from the previous vertex. This is a setting
configurable in advanced settings.

An implementation of the Ramer–Douglas–Peucker algorithm is used to
reduce the number of vertices when a drawing is finished. The
aggressiveness of reducing the number of vertices is dependent on a
parameter epsilon. If set to 0, no vertices are removed. The higher the
epsilon, the more vertices are removed. The epsilon parameter can be
configured in napari preferences -> experimental -> rdp_epsilon.

Lastly, a refactor has been performed for various functions related to
polygon drawing.

We acknowledge @yozhikoff who made a first prototype of this new
feature.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: wmv_hpomen <w-mv@hotmail.com>
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
Czaki pushed a commit that referenced this pull request Jun 22, 2023
# Description
This PR adds documentation for the Shapes layer polygon lasso tool that was added
in napari [#5555](#5555)

## Type of change
- [x] Fixes or improves existing content

# References
depends on #5555

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Czaki pushed a commit that referenced this pull request Jun 22, 2023
# Description
This PR adds documentation for the Shapes layer polygon lasso tool that was added
in napari [#5555](#5555)

## Type of change
- [x] Fixes or improves existing content

# References
depends on #5555

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/request-for-testing-napari-0-4-18-release-candidate/82833/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design discussion feature New feature or request highlight PR that should be mentioned in next release notes preferences Issues relating to the creation of new preference fields/panels qt Relates to qt tests Something related to our tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants