Feature: polygon drawing in the Labels layer

Description
This PR implements a new way of drawing in the Labels layer by creating polygons instead of just using the brush. It is quite useful when working with objects that have straight lines or in other scenarios when it is easier to specify an object mask by following its perimeter.
It is implemented by introducing a new mode in the Labels layer called DRAW_POLYGON:

Polygon drawing is controlled by the following actions:

Create a new point (left mouse click)
Delete the latest point of the polygon (right mouse click)
Complete the polygon creation and adding the polygon mask to the labels (left mouse double click or Enter, which can be specified in the Labels shortcuts)
Cancel the polygon creation (Escape,  can be specified in the Labels shortcuts)

It is important to note that switching modes does not complete or cancel the polygon creation, which is done on purpose to be able to use PAN_ZOOM during the process (it is extremely useful when working with large objects as in the video below).
The color of the polygon matches the current label color and it takes opacity from the layers opacity. There is only one exception: when you use background color, the polygon is always transparent and has red boundaries.
Also, the polygon implementation is completely independent of the Shapes layer and uses its own polygon visualization based on an overlay, which has better performance and, more importantly, is substantially easier and more straightforward to implement.
https://user-images.githubusercontent.com/5459767/236333440-9bf07491-c328-4509-91ef-3ae07fe7950c.mp4
https://user-images.githubusercontent.com/5459767/236326736-902322b1-bde5-46be-b8c2-d5c2e0ea42bd.mp4
Type of change

[X] New feature (non-breaking change which adds functionality)

How has this been tested?

[X] all tests pass with my change

Final checklist:

[X] My PR is the minimum possible work for the desired functionality
[X] I have commented my code, particularly in hard-to-understand areas
[X] I have made corresponding changes to the documentation
[X] I have added tests that prove my fix is effective or that my feature works
[X] If I included new strings, I have used trans. to make them localizable.
For more information see our translations guide.

Looks awesome, and super useful!
Some general thoughts after a first pass:

we should definitely try to abstract some of the mouse callback and math logic out so we don't have to maintain 2 different ones for shapes and labels
I don't love adding extra state onto the layer; we're trying to move away from layers holding any state entrangled with the current Dims, so I would prever if this was kept elsewhere. I really like the use of a callable class instead of a simple function; this would actually make it easier to move all those methods (_complete_polygon_drawing etc) on the callback instead. But let's see what others think as well.

I don't love adding extra state onto the layer; we're trying to move away from layers holding any state entrangled with the current Dims, so I would prever if this was kept elsewhere.

I couldn't agree more and I think that the design in which tools are integral parts of a specific layer is not very good as it makes almost impossible to share them across different layers. There should be a separate abstraction for tools, in which all tool-related logic should be implemented and kept,  beginning with visualizations (overlays, etc.) and ending with mouse/keyboard callbacks.

this would actually make it easier to move all those methods (_complete_polygon_drawing etc) on the callback instead.

I don't think that would be good. Mouse callbacks must only be concerned about mouse interactions and retain only the states required for its effective operation. Otherwise, it will blur the distinction between various abstractions.
In the case of _complete_polygon_drawing, it can be either in the layer, or moved to the overlay if you don't like to keep this method inside the layer. It can't be in the mouse callback since it can also be called by the keybinding (Enter), which is unrelated to the mouse callback.

we should definitely try to abstract some of the mouse callback and math logic out so we don't have to maintain 2 different ones for shapes and labels

As I mentioned above, I think there should be another abstraction for that, as it goes beyond just callback abstractions. If we want to share some tools or interactors between Shapes and Labels, at least complete rethinking of the Shapes layer is required: getting rid of the custom implementations of shapes, as it is all already more efficiently and flexibly implemented in VisPy; disentangling the layers and the tools and so on. It would be a very big refactoring, which goes far outside of this PR.

I love this tool, it does blur the boundary between labels and shapes, so the idea of layers and tools at some point is very attractive to me, but obviously requires some deep rethinking.
See also: https://github.com/napari/napari/pull/5555 which is very similar in behavior, but a shapes tool
Also @ksofiyuk you're 🔥 !

Hey @ksofiyuk!
Just btw, I know I owe you some more review on #5732, apologies for the delay! Just got back from a hiking holiday, that's high on my list.
Anyway, regarding this PR, I'm a bit apprehensive about it, for the reasons that I think @brisvag has articulated... The functionality looks great (!), but as already noted, with #5555, this is now a lot of duplicated functionality between the shapes and labels layers. I appreciate that unifying the functionality is a lot of work and "out of scope" for this PR, but at the same time, not unifying for this PR just amounts to more technical debt, which we are already floundering under. Unlike your other optimisation PRs, which actually simplified the code, this adds a lot of complexity to the code. Not to speak of the UX issues, such as adding the 7 shortcut to the button, which breaks the sequence (while at the same time reordering the shortcuts would break muscle memory for existing users).
None of this is to say that this won't go in. But I'd like to take a step back and really think about the best way forward with this. (Note, btw, that we have already in the past proposed a "paintbrush" widget for the shapes layer that would "paint" a polygon — which has the advantage that it works in multiscale data, so the idea of unifying the "drawing" API is very appealing.) I hope that it's ok to take things slow here and you can get by with your local fork in general?
And btw if other core devs want to jump in and say I'm being too cautious here I'm also happy to hear it! No one likes being the spoil-sport! 😅

In the case of _complete_polygon_drawing, it can be either in the layer, or moved to the overlay if you don't like to keep this method inside the layer. It can't be in the mouse callback since it can also be called by the keybinding (Enter), which is unrelated to the mouse callback. Even keeping it in the overlay blurs the overlay abstraction because the overlay's function is to manage visual primitives, not to handle the tool-specific logic.

I think the VispyOverlay would be fine for this. Take a look at the TransformOverlay, which blurs the line in a similar way. Unless we opt for a new abstraction for layer tools, I'd say that's the least disruptive path.

Unlike your other optimisation PRs, which actually simplified the code, this adds a lot of complexity to the code.

@jni That is something I cannot agree with you on. This PR is does not changes any mechanics of napari, it effectevily does not add anything new. If you take a look at the code, it is completely built using the components that are already presented in napari and VisPy:

It only uses @brisvag's overlays interface to add it to the layers and render it on the top of the сanvas. No extra code on that part at all.
It utilizes solely VisPy's visual primitives without any custom implementation and reinventing the wheel. This PR doesn't have any low level visual code. What I can't say about the Shapes layer, that has a thousand lines of code only for visual primitives. And on top of that, VisPy's polygon is much better polished, less buggy and has better performance.
With the latest refactoring, it is almost completely decoupled from the Labels layer and can be added to another layer with very little effort. This is unification. I believe that any previous @brisvag's concerns should be resolved.

If you think that this PR changes a lot of code, it does not in fact, out of 584 lines, almost half corresponds to tests and documentation. The heart of this PR is VispyLabelsPolygonOverlay that implements all the logic: visualization, mouse events and user interactions. And it is only 168 lines of code for implementing a completely new tool, how could it be simpler? :slightly_smiling_face:

@brisvag By the way, I found and fixed a bug related to the overlays attached to layers. When a layer is removed, the overlays are not removed and overlay.close() is not called, leading to failing napari/_tests/test_adding_removing.py when you bind callbacks in the overlay. The fix is here: https://github.com/napari/napari/pull/5806/commits/12137f22a34033c9f2324a266e93420ccff03114

One small thing, but this new feature needs to also be documented in the user facing docs at napari/docs.
Probably in this section of labels file: https://napari.org/stable/howtos/layers/labels.html#painting-in-the-labels-layer
Can take inspiration from:
https://github.com/napari/docs/pull/149
If you have limited time, I can help draft something.

Another consideration here is perhaps whether we want to be able draw the label polygon on a 2d plane. In that case when viewer.dims.ndisplay == 2 and n edit dim == 3 a different logic is required. This behaviour mimics the behaviour of the polygon tool in the shapes layer.

When you finish a polygon it has to be added to a labels image, which requires rasterization of the vector polygon. This PR uses the rasterization algorithm from scikit-image (skimage.draw.polygon), which only supports 2D coordinates. To make it working  on a 2D plane in 3D space we need to implement a custom rasterization algorithm, which is definitely not an easy task because the algorithm itself is not simple and it cannot be implemented in pure Python code (skimage utilizes Cython for that).

Another consideration here is perhaps whether we want to be able draw the label polygon on a 2d plane. In that case when viewer.dims.ndisplay == 2 and n edit dim == 3 a different logic is required. This behaviour mimics the behaviour of the polygon tool in the shapes layer.

When you finish a polygon it has to be added to a labels image, which requires rasterization of the vector polygon. This PR uses the rasterization algorithm from scikit-image (skimage.draw.polygon), which only supports 2D coordinates. To make it working on a 2D plane in 3D space we need to implement a custom rasterization algorithm, which is definitely not an easy task because the algorithm itself is not simple and it cannot be implemented in pure Python code because of performance issues (skimage utilizes Cython for that).

I see, makes sense. Perhaps this would be good to include in the documentation. @psobolewskiPhD do you agree?

@brisvag I've just found another very insidious bug when working with layer overlays.
If an overlay modifies its states during its creation in create_vispy_overlay in Layer._on_overlays_change, it causes double creation of the overlay because it triggers Layer.events._overlays event, which is connected to  Layer._on_overlays_change, which is then called once again. As the first call of  Layer._on_overlays_change hasn't had time to add the first created overlay to self.overlays, it leads to creating another copy of the overlay.
The solution is very simple, the overlay events should be blocked during the overlay creation:
https://github.com/napari/napari/blob/0c7bc5695d2e4342677f1f3f813d44d078534dde/napari/_vispy/layers/base.py#L170-L173
I started to debug it as several factors indicated to me that the overlay was operating with several copies at the same time. For example, the opacity of the overlay didn't correspond to the real opacity. And as it turned out it was because of several overlapping polygons at the same time!
I managed to find it out by analyzing the call stack:

Nice job catching bugs! I'd love if you could pull those out of this PR into a small one so we can just merge those quickly while we figure out what to do with this one :)

From my perspective, it appears to be ready. Please let me know if there are any additional issues that I can address.

Unlike your other optimisation PRs, which actually simplified the code, this adds a lot of complexity to the code.

@jni That is something I cannot agree with you on. This PR does not change any mechanics of napari, it essentially does not add anything new. If you take a look at the code, it is completely built on the components that are already present in napari and VisPy:

Sorry, I didn't mean to say anything about the quality of the code, which is indeed great — and thanks for finding an existing bug and pulling out the fix into its own PR, that's super great. 😊 🙏
I'm talking instead about UX inconsistencies that are currently being worked on and that this PR makes worse... No one seems to have commented on the issue of putting a new icon in the tools row but keeping the shortcuts unchanged. For background on why the shortcuts are numbered the way they are, see #2800, #3072, and #3133. Now an additional goal would be, if the same tool (draw polygon) exists in multiple layers, the shortcut should be the same, and probably the position of the icon should be the same or similar.
Personally, I think at a minimum we should figure out what we want to do there and then make the icon and shortcut consistent with that, rather than saddle ourselves with UX state that is both inconsistent and temporary. One potential solution: make a new row of icons and use the QWERTY keys as shortcuts. (And update the Shapes layer accordingly.)
Thoughts?

The UI aspect was discussed a bit at the last EU community meeting (https://hackmd.io/BXWDZ3i8Q6OAEASrkaSNIQ)
There's multiple tools/icons that are inconsistent between layers.
This PR was adressing it but stalled apparently:
https://github.com/napari/napari/pull/5021
Getting that in would improve things, but also result in changes if someone has memorized the current buttons.
At some point may need to make a move towards a tool bar that is always present and some tools are not available in certain layers.
In the spirit of #5021 could move the new tool to the end of the row?
keybind 6 is Transform which doesn't have a tool bar icon, not sure how many people use it on 6 vs rebinding it something they like, so swapping those two would be 🤷♂

The UI aspect was discussed a bit at the last EU community meeting

wow looks like a great meeting 😂 timezones suck 😭

I decided to remove finishing drawing of a polygon with left mouse double-click as I think it causes more harm than good. After several weeks of using of this tool, I've found this behavior very annoying as it often triggers premature polygon completion when you annotate at a pretty fast pace. So, now it can be finished only by pressing Enter.

I decided to remove finishing drawing of a polygon with left mouse double-click as I think it causes more harm than good. After several weeks of using of this tool, I've found this behavior very annoying as it often triggers premature polygon completion when you annotate at a pretty fast pace. So, now it can be finished only by pressing Enter.

How is the experience in terms of ergonomics? Personally for me on the laptop I have it is not really nice to draw with a mouse and then pressing enter to finish the drawing. Maybe instead of enter, c for complete would be nicer as it is centered on a keyboard and thus would work equally well for both right-handed and left-handed people. What do you think?

How is the experience in terms of ergonomics?

It feels not bad, in polygon drawing you usually most spend of time by placing points and moving the canvas, move your hand to press Enter at the end doesn't feel like a big deal.

Maybe instead of enter, c for complete would be nicer as it is centered on a keyboard and thus would work equally well for both right-handed and left-handed people. What do you think?

As for me, pressing Enter to finish drawing seems quite intuitive. However, if other users of this tool find this default hotkey inconvenient, I can change it to another key by default. Actually, it is not a big deal as you can change the shortcut in napari preferences if you want to use another key.

How is the experience in terms of ergonomics?

It feels not bad, in polygon drawing you usually most spend of time by placing points and moving the canvas, moving your hand to press Enter at the end doesn't feel like a big deal.

Maybe instead of enter, c for complete would be nicer as it is centered on a keyboard and thus would work equally well for both right-handed and left-handed people. What do you think?

As for me, pressing Enter to finish drawing seems quite intuitive. However, if other users of this tool find this default hotkey inconvenient, I can change it to another key by default. Actually, it is not a big deal as you can change the shortcut in napari preferences if you want to use another key.

Yeah agreed, just to be clear I don't see it as a dealbreaker:) Was just wondering as my experience is but one experience.

I decided to remove finishing drawing of a polygon with left mouse double-click as I think it causes more harm than good. After several weeks of using of this tool, I've found this behavior very annoying as it often triggers premature polygon completion when you annotate at a pretty fast pace. So, now it can be finished only by pressing Enter.

Double-click to finish a polygon is a very standard UI interaction—which includes the napari Shapes layer.
I strongly feel that default behavior should try to stick to universal expectations and be the same as Shapes, but we can enable more tailored/power-user things in Preferences.

I strongly feel that default behavior should try to stick to universal expectations and be the same as Shapes, but we can enable more tailored/power-user things in Preferences.

I know that it is the standard behavior of the Shapes layer, and that's why I initially implemented the finishing polygons with this way. However, after actively using this tool for the past few weeks, I can say for sure that it is the bad UI choice as it often leads to premature completion. It happened to me at least ten to twenty times in a single week and it annoyed me so much that I decided to remove it completely.
It is really frustrating when you draw a complex polygon of over 30 vertices and at some point it completes it in the middle of the process, often resulting in a completely incorrect polygon that must be redrawn from scratch.
I'm pretty sure that if we keep it and it is merged, you will start getting bug reports from befuddled people who don't understand why polygon is unexpectedly finished in the middle of the process.

Double-click to finish a polygon is a very standard UI interaction

I wouldn't be very sure about that. CVAT (by which I was inspired to add the drawing polygon tool in napari) doesn't complete polygons with double-click (I guess for the same reason I mentioned above), Photoshop and GIMP do not bind any actions on double-click in the path tool (which is the closest to the polygon/shapes tool in napari). Do you know any kind of annotation software that is actively used for labelling and implements that kind of UI interaction?

And I believe that the fact that you're not getting such kind of bug reports for the Shapes layer actually means that no one seriously uses this layer for annotating purposes.

I have been thinking about this PR for the past few days and I remain uncomfortable with it. I can imagine that some would also find it handy to draw circles, rectangles, and paths with straight lines in a Labels layer. Are we just going to replicate the whole Shapes layer interaction toolkit in Labels?
I think the right way to implement this would be to add better linkage between the Shapes and Labels layer — this could be for example a plugin that adds a callback to the "shape completed" event and fills in the corresponding area of the Labels layer.
I totally see your points about double-click @ksofiyuk but in that case I would ask that you suggest a different interaction in Shapes. And in that case the polygon-completion shortcut would indeed need to be a preference.
For me the double-click interaction seems familiar but I can't place from where right now. @psobolewskiPhD @melonora any examples come to mind?

Yeah typically when having a lasso select tool the interaction ends with a double click. This is also the case in GIMP. @jni When in #5555 I combined the interaction of the polygon button and the lasso polygon functionality which was based on the interaction as present in GIMP.
@ksofiyuk I am not certain regarding annotations using the shape layer not being used much. Potentially if this is the case the reason could be that for quite a few segmentation algorithms they return a label image in which overlap of segmented objects is not present, the exception being the extension of Stardist called multistar. When looking at the difference between a shapes and a labels layer (other than rasterization) it is that it can have overlapping objects. Personally, I prefer the shapes layer for defining ROIs because of that, but because people are more familiar with segment results being returned as a single channel integer image they would be more inclined to use the labels layer as well as libraries using more the label image.
@jni I agree that the distinction between shapes and labels gets quite blurred. @jni @ksofiyuk Should it be considered to have one annotation layer instead where the user ultimately decides whether shapes get immediately rasterized or not? This would be a significant API change, but would solve a lot of the discussion I think.

I can imagine that some would also find it handy to draw circles, rectangles, and paths with straight lines in a Labels layer. Are we just going to replicate the whole Shapes layer interaction toolkit in Labels?

@jni I consider this as highly unlikely at least for real-world annotating applications. I've been working in the field of computer vision  for a long time, and I've never seen a real need for drawing circles, rectangles or any kind of right geometric shapes for segmentation annotation.  They are useless for one simple reason: perfect geometric shapes are almost never found in real-world images.  Of all the tools mentioned, I can think only of paths to be potentially useful in segmentation (straight line is just a special case of the path consisting of 2 points).
Can you think of a possible use case for drawing a circle in the Labels layer (other than for fun)?
Polygon drawing, on the other hand, is a highly sought-after tool in segmentation annotation. It is a real pain in the neck to annotate something without it.
Just to better understand what I'm talking about, please try to annotate cars, trees, road and traffic lights in the following image using only the brush tool:

Without this PR, napari is just not suitable for very standard computer vision segmentation annotation.

I agree with @ksofiyuk  regarding the need for drawing circles and so on in a labels layer for the reasons that he stated. From my experience all required shapes are more complex, with maybe the exception of annotating satellite imaging.
Circles and bounding boxes are mostly used if the intention is to get a crop of the image. That being said with #5555 merged, I do think napari does provide capabilities for computer vision segmentation annotation since it provides the ability to draw complex polygons using both mouse and tablet and the shapes layer can be rasterized to create a labels layer. However, this was only recently merged and thus has not been advertised as a feature of napari yet. @ksofiyuk correct me if you think I am wrong in this.
What I would propose is the part I suggested above, where we would make use of the polygon overlay created here and thus marry the best of both worlds where ultimately the user determines to either keep the shapes or to rasterize it on the fly (both having a potential use case for the reason of allowing for overlap versus not allowing for overlap).

I am not certain regarding annotations using the shape layer not being used much.

To be honest, I highly doubt that someone really uses it for annotation, and I'm talking not about annotating 1-3 images with 2 classes, but using it for annotating something complex and on a relatively massive scale (a hundred images or more).
I find the Shapes layer unsuitable for it for many reasons:

It doesn't support label classes, so it cannot be used for multiclass annotation.
When you draw a polygon you cannot cancel the last points in the process and then just continue drawing this polygon. If you make a mistake in the process, you literally have no choice rather than delete the whole polygon and redraw it from scratch. And even for that action you need to switch to another tool (select shapes), select the polygon and delete it, than switch back to the polygon tool.
Adding or removing points to the existent shapes is really painful and disrupting for the workflow. To simply remove a point from an existent polygon you need to do a LOT of unnecessary actions: (1) switch to select vertices or shape, (2) select the shape, (3) select the remove vertex tool, (4) click a vertex you want to remove.

2, 3 are just terrible UI choices, making it near to impossible to use the Shapes layer for something practical. And If no one loudly complains about it, I can only presume that no one actually uses it.
Please do not take it as offense, as I believe that napari has a potential for annotation and can be much better, and as you can see, I'm doing my best to make it better (I'm doing it in my free time as a hobby, no one pays me for it).

@ksofiyuk No worries, I am also a contributor and not a core dev. I do see your points. What I am asking is for the longer term whether it would be worth considering to have an annotation layer which either rasterizes or not. Personally I think it is. The user interaction can be discussed and I do agree that we could improve on user interaction.

@ksofiyuk napari needs improvement for many workflows. Nobody is debating that. We're simply exploring the best way to improve it while keeping long term maintainability and simplicity. Our code base is already too complex and we actively want to simplify it. Duplicating polygon drawing in different layers with slightly different behaviour moves us away from that goal so it is a step we should be very careful in taking.
I certainly don't disagree in principle that the Shapes layer is awkward (I rarely use it myself, but others in the community certainly do use it), but my reaction to that is that we should improve polygon drawing in the Shapes layer to meet your needs — classes can be implemented as Shape Features, btw, see examples/add_shapes_with_features.py — rather than adding polygon drawing to the Labels layer. If this PR was instead "different polygon drawing controls for Shapes layer and dramatic speedups", it might be easier to get in!
I'm certainly not discarding it out of hand, but I'm still not convinced that the duplication and the UX issues are worthwhile...

but my reaction to that is that we should improve polygon drawing in the Shapes layer to meet your needs — classes can be implemented as Shape Features

The Shapes layer doesn't work for me. I work with rasterized segmentation masks, I store them as images (or using the RLE representation). I use the polygon tool just as an editing tool, to segment a new object, to add something to an existent mask, to remove something from an existent mask using a polygon. I can draw a complex object in several steps and so on. I do not need to store a set of shapes or something.
Take a look at the example below where I segment a complex shape with holes using the polygon tool in the Labels layer, then polish it using the brush tool:
https://github.com/napari/napari/assets/5459767/c2750663-2e55-41a9-a468-14b85ce429d8

I actually use napari, not only to segment from scratch, but to fix errors in existent masks predicted by another model, for example.
Here I fix false positives and false negatives of the mask using the polygon tool:
https://github.com/napari/napari/assets/5459767/1f6b057c-f113-4c58-820b-7fbabd373a40

@jni @ksofiyuk Perhaps this would be good to discuss during a meeting sometime what a potential roadmap could be if both of you are up for it (or perhaps discuss with more people during one of the community meetings). I do think that in the short term, this PR provides a currently unfulfilled need, but in the long term something needs to be done about the codebase. @jni Would there be objections to accepting the PR for now, advertising it as a temporary feature while establishing a roadmap? In any case, the polygon overlays could be used I believe for both shapes and labels layer as @ksofiyuk previously suggested.

@jni Shapes and labels are quite different representations and I don't think that we need to mix them together. In labels, I don't need shapes as it is, I just need another tool to edit masks, because using the brush is not always the best choice.
Shapes cannot replace masks because, first, there is no good way to represent objects with holes like scissors above using polygons. And second, segmentation networks predict rasterized masks not polygons, and if you want to work with them, then you need to stick to rasterized masks.
I don't like the idea of using the Shapes layer to draw polygons to rasterize them afterwards to Labels layer, because it will inevitably disrupt the Labels workflow. For example, I need to just use the polygon tool to fix some errors like in the last example with scissors:

Will I need to switch each time to the Shape layer whenever I want to draw something and then back to the Labels to use the brush?
How does the Shapes layer understand to which layer I want to rasterize the shape as I can have several Labels layers at the same time?
Can I use  the X hotkey from the Labels layer when I draw shapes to quickly switch between adding and removing?
Will I need to perform other actions rather than pressing Enter to finish the polygon to send it to the Labels layer and clear the polygon points?

What I am proposing is an annotation layer for which the mode (rasterization or not) is defined by the user. If rasterization the mode would be as you implemented it now

@melonora @jni I actually like the idea of a unified annotation layer, but I think the rasterization shouldn't be the mode of the layer, it should be a specific mode of the shapes annotation tool.
I think the Labels layer can be easily upgraded to that kind of an universal annotation layer in the following way:

Add a new property to the layer that stores a list of shapes and an overlay that renders all the shapes from the list.
Add a new shapes tool (a single tool for all shapes).

When you select it, a shapes section is added to the layer controls with a drop-down list that specifies the type of shape (polygon, circle, rectangle, line, path, etc.).
It should have a control that specifies the finishing behavior: either just add it to the shapes store from 1 or to directly rasterize it to the labels image.
In the controls section you can specify some additional parameters for shapes and rasterization, e.g. thickness for rasterizing a line or path.

Add a shape selection tool, which can be used to select existing shapes. I suggest combining "Select vertices", "Select shapes", "Add vertex", "Remove vertex", "Move to back", "Move to front" from the Shapes layer in this single tool.

When is nothing is selected, you can click or draw a selection rectangle to select a single shape or a group of shapes.
When a shape is selected,  its vertices should be displayed and then you can just select and drag a specific vertex without switching to another tool. And if a vertex is selected, it can be deleted by pressing Del.
You can add a new vertex to a selected shape by left mouse click while holding, for example, Control.
You can move selected shapes to foreground/background by pressing specific hotkeys or by pressing special buttons in the layer controls which are added to it when this tool is selected.

So in this way, is is possible to combine 12 (!) different tools from the Shape layer to only 2 new tools in the Labels layer (which we can afterwards rename to just the Annotation layer). It will be able to display two different annotation modalities (shapes, image labels) at the same, it will have a unified labels classes for each shape inherited from the Labels layer. And the shapes tool with automatic rasterization on can be used for my uses cases absolutely seamlessly.

@ksofiyuk Perhaps an extension of the annotation tool would be to have 3 modes. Create, Merge, and Erase.

Create: Create any polygon you want even when starting point is on an existing polygon / label.
Merge: If starting point is existing polygon / label, create union of the 2 after finishing drawing. Otherwise as Create
Erase: If starting point is existing label /  polygon, erase label within created erase polygon or if polygons with holes can be supported in Vispy, either create internal polygon (the hole in the larger polygon) or if on the side, subtract from existing polygon.

There would be no need for a brush tool at all anymore I believe and moving individual vertices would also not be required. Particularly when drawing a lasso polygon this would not make much sense.

As to why I would make it a mode of the layer, perhaps I am wrong about this, but it could make it more backward compatible.

Perhaps an extension of the annotation tool would be to have 3 modes. Create, Merge, and Erase.

@melonora I think this kind of behavior would be very difficult to implement. First, you will need to implement generalized intersections of polygons including subtracting polygons, which is an extremely tricky algorithm with lots of edge cases. Second, a polygon with holes is not a default data structure, it cannot be stored like a list of points, so anyone who wants to use it will need to learn this new data structure and adapt their code to it.
And I don't think that the brush tool can be replaced, there are situations when the brush is much more convenient than polygons and vice versa.

@ksofiyuk @jni
ImageJ uses double-click to end.
Illustrator used to as well—I can't check, I have a clone now (Vectornator) which uses double-click to end.
So does Shapr3D and PowerPoint (lol, but I use it a lot).
Pixelmator, my photoshop clone, also uses double-click to end, but you need to be close to the origin to close.
And our Shapes layer has this.
Anyhow, I checked out 8b7ea758f3d8eb6675e4976297a5696004e50f16 which is before you changed the end behavior and on my mac I spammed as fast as I could and couldn't trigger a premature double-click. I checked in my OS settings I have a pretty fast double-click threshold. Any chance you are you on Linux?
The doc suggests you may have a 400 ms interval, which is pretty slow: https://doc.qt.io/qt-6/qapplication.html#doubleClickInterval-prop
Can we use that setting to tweak Linux and avoid the erroneous behavior?
I'm still in favor of this PR, because I think the functionality is really useful for labeling.
I think ultimately I'd prefer a universal toolbar with tools that adapt to the layer type, so we don't have duplicate polygon tools. But, I'm not sure we should let perfect get in the way of good—or great in this case.
My vote would be to tweak the doubleClickInterval-prop on Linux or make a setting to optionally disable double-click to complete.
Also, we need to make sure a user-facing docs repo PR is made.

@psobolewskiPhD If you think it is right to have it, so let's go with it. I brought back the double-click completion and I added an option to disable it. And it can be disabled in the following way:
label_layer._overlays['draw_polygon'].double_click_completion = True

This perfectly works for me. If this PR was merged, and there would be complains to that behavior, then it can be set to False by default.

I checked in my OS settings I have a pretty fast double-click threshold. Any chance you are you on Linux?

You're right, I mainly use it on Linux. I didn't know that it differs so much between systems. I will play with changing the doubleClickInterval in X11 later to see if it can mitigate the issue.

Hi everyone,
I really like where the discussion has gone. Incidentally the idea of a unified annotation layer (raster vs vector) really goes well together with an old feature request of mine, #2077. Other comments:

@ksofiyuk you've convinced me that two separate layers would be cumbersome for your workflow. I don't think any of your objections are fundamental (they could definitely be addressed with appropriate UX + custom callbacks and keyboard shortcuts — and indeed a unified annotation layer would probably be implemented quite similarly), but I do think that it's not a short-term fix, or a matter of just speeding up the polygon drawing.
(Having said this there's plenty of spherical and elliptical things in scientific imaging, which is napari's primary target demographic — things like planets, bubbles, craters, etc. But spheres at least are well served by the normal paintbrush.)
@psobolewskiPhD "Pixelmator, my photoshop clone, also uses double-click to end, but you need to be close to the origin to close." I think that's pretty nice tbh, what do you think @ksofiyuk?
One complicating factor with unifying shapes and Labels is that 3D interactivity is getting pretty nicely fleshed out with Labels but not so much with Shapes. For 3D+ datasets this is very important. I'm not sure what this means for this PR — is the button simply disabled in 3D view @ksofiyuk?
I'm still wary of the button/shortcut conundrum. However we are approaching 0.5.0 so maybe that's a good time to (a) insert the polygon button to the right of the paintbrush, and (b) shift all the shortcuts around. I don't think we want to go past 6 so I would move "shuffle labels" out of the row (could be a later PR) and start with ` for the eraser and go from there. What do people think about this specifically? (@psobolewskiPhD @melonora)
fwiw, I think #5006 would get us 90% of the way there, and in the case of objects with no/few straight edges is superior, but I dropped the ball on completing it.

Ok so as far as I can tell that removes most of the blockers here (double click — but we should explore a closeness to first vertex option — and button placement / keyboard shortcut). Thoughts?

@jni regarding the buttons, this PR was looking at moving the buttons around so they are more consistent between Shapes and Labels:
https://github.com/napari/napari/pull/5021
Would make some sense in that light to have the two polygon tools have the same shortcut--if not primary then secondary.

One complicating factor with unifying shapes and Labels is that 3D interactivity is getting pretty nicely fleshed out with Labels but not so much with Shapes. For 3D+ datasets this is very important. I'm not sure what this means for this PR — is the button simply disabled in 3D view @ksofiyuk?
Yes it is, in 2D view though it behaves similar to drawing a polygon in the shapes layer. The polygon only exists in the particular z-plane.

(Having said this there's plenty of spherical and elliptical things in scientific imaging, which is napari's primary target demographic — things like planets, bubbles, craters, etc. But spheres at least are well served by the normal paintbrush.)

Even those things you mentioned rarely have perfect geometrical shapes (maybe except for planets). As far as I understand, napari has an inclination towards biomedical data. And as far as I know, cells and tissues almost always have irregular shapes, which is perfectly illustrated by napari's logo and the very first image of the interface in README 🙂 .

"Pixelmator, my photoshop clone, also uses double-click to end, but you need to be close to the origin to close." I think that's pretty nice tbh, what do you think @ksofiyuk?

That's an interesting idea, it is easy to implement, but we need to agree on how to define the radius of completion (fixed, e.g. 30 pixels, or estimate it somehow dynamically?). @psobolewskiPhD what do you think about implementing that kind of restriction on the double-click completion?

One complicating factor with unifying shapes and Labels is that 3D interactivity is getting pretty nicely fleshed out with Labels but not so much with Shapes. For 3D+ datasets this is very important. I'm not sure what this means for this PR — is the button simply disabled in 3D view?

This PR works everywhere where z-coordinate (or any other combination of axes in multidimensional space leaving strictly 2 free dimensions) is known and constant across all polygon vertices, in all other cases the tool is disabled. The limitation is due to the limitation of the rasterization algorithm from skimage, I gave a more detailed explanation here:  https://github.com/napari/napari/pull/5806#issuecomment-1544599736. Actually, the issue will be everywhere, no matter whether is it the Shapes polygon or this one, only if someone decides to implement multidimensional polygon rasterization.

fwiw, I think https://github.com/napari/napari/pull/5006 would get us 90% of the way there, and in the case of objects with no/few straight edges is superior

Polygon drawing actually has way more advantages rather than just drawing straight lines in comparison to #5006:

You can draw very complex shapes while making stops in the process and moving the canvas, zooming in and out;
Polygon drawing is very robust to errors during drawing as you can easily cancel the latest points using a right click, if you accidentally place an inaccurate vertex. In case of #5006, one twitch of the hand in the process and you will need either to completely redraw it or to edit the mask afterwards.
While placing vertices with clicks, it is much easier to follow the exact boundaries of an object rather than by free mouse move (for the same reason as in the previous point). You need to have a really firm hand to be able to draw contours without errors in one pass.

To feel the difference, just try to segment the scissors from https://github.com/napari/napari/pull/5806#issuecomment-1567142227 using these two approaches and compare the quality of the resulting masks and the effort to get them 🙂 .

The logic and UX behind the polygon drawing is much more straightforward in contrast to the indirect logic of filling contours and controlling in what situations you want to fill contour and in what you just need a brush stroke.

I'm still wary of the button/shortcut conundrum. However we are approaching 0.5.0 so maybe that's a good time to (a) insert the polygon button to the right of the paintbrush, and (b) shift all the shortcuts around.

@jni @psobolewskiPhD I'll leave the placement of the button (or the icon itself) and the tool's shortcut up to you. I don't really have any preferences on that for this PR.

Even those things you mentioned rarely have perfect geometrical shapes (maybe except for planets)

But for modelling purposes one usually wants to fit the idealised shape and disregard the imperfections. Even in biology, modelling things as spheres is sometimes useful, e.g. lipid droplets within cells. (I don't have a good image to share right now but it exists! 😂)

As far as I understand, napari has an inclination towards biomedical data.

This is a historical accident. Napari aims to be a general purpose scientific viewer for n-dimensional data across all scientific disciplines.

cells and tissues almost always have irregular shapes, which is perfectly illustrated by napari's logo

A common misconception but one the team is fond of. 😂 The napari logo actually comes from the island of Tabuaeran: https://en.wikipedia.org/wiki/Napari#/media/File:Tabuaeran_Kiribati.jpg

we need to agree on how to define the radius of completion (fixed, e.g. 30 pixels, or estimate it somehow dynamically?)

It should be defined in screen pixels (not data), so that one can zoom in to make more precise contours. Potentially, the "placing another vertex" threshold defined in settings in #5555 could be reused here? ie it could be a general threshold for when we consider two nodes to be "close" to each other in screen space. But maybe that is too small a setting and this should be a different setting. But making it user settable is good, and we could say that e.g. 0 means disabling double click to end.

This PR works everywhere where z-coordinate (or any other combination of axes in multidimensional space leaving strictly 2 free dimensions)

Have you tested this by rolling the dimensions? That's an important point.

To make it working on a 2D plane in 3D space we need to implement a custom rasterization algorithm,

Actually we can probably do something here similar to:

define a new basis in which the points can be reduced to 2D
rasterise in this basis
use scipy.ndimage.map_coordinates with nearest-neighbour interpolation to project back to the oblique plane

Combining this with clipping planes on the labels layer would be pretty badass.
Needless to say this is work for another PR once this goes in. 😂

To feel the difference, just try to segment the scissors from https://github.com/napari/napari/pull/5806#issuecomment-1567142227 using these two approaches and compare the quality of the resulting masks and the effort to get them

I'd say you're right when using the mouse but with a tablet and pencil, contour drawing is more useful. (But @melonora's lasso polygon drawing would remove that advantage.)
@psobolewskiPhD what do you think of my proposal:

I'm still wary of the button/shortcut conundrum. However we are approaching 0.5.0 so maybe that's a good time to (a) insert the polygon button to the right of the paintbrush, and (b) shift all the shortcuts around.

Have you tested this by rolling the dimensions? That's an important point.

@jni Yes, sure, everything should work (rolling, transposing, slicing along z-dimension). You can check it with examples/nD_labels.py.

@jni I checked before and it was working as @ksofiyuk states.

"Pixelmator, my photoshop clone, also uses double-click to end, but you need to be close to the origin to close." I think that's pretty nice tbh, what do you think

It should be defined in screen pixels (not data), so that one can zoom in to make more precise contours.

@jni @psobolewskiPhD  This is done in https://github.com/napari/napari/pull/5806/commits/354239b27238d4b7f2670d9a26096e04faff67ff, now a double-click completes the polygon only within a given radius in screen pixels  (LabelsPolygonOverlay.completion_radius, 20  by default).

Potentially, the "placing another vertex" threshold defined in settings in https://github.com/napari/napari/pull/5555 could be reused here?

I don't think that it is a good idea to reuse this parameter as it has the completely different meaning in the lasso tool ("determines how many screen pixels one has to move before another vertex can be added to the polygon").
Actually, I don't think that someone will ever need to change the default value of the completion radius. It just defines some epsilon radius in which it is assumed that the user tries to perform double-click on the first vertex to complete the drawing. As it affects only double-clicks, you can place as many clicks you want near the first vertex using a simple left click without being concerned about this radius.

@jni I think all of your comments and changes are addressed.

@ksofiyuk Great work! Do you think you could open a PR in the docs repo? If not, I would have time at the end of the week. I don't think much change is anymore required and this would be ready for merge. @jni Correct me if I speak prematurely.

@jni @psobolewskiPhD This is done in 354239b, now a double-click completes the polygon only within a given radius in screen pixels (LabelsPolygonOverlay.completion_radius, 20 by default).

Sorry, I guess I wasn't clear before: the tools where completion fires when close to the origin are ones where it's a path that can be open or closed. In this case, it's always a closed polygon right—just like Shapes polygon. So, personally, I find the similarity of the tools with this behavior difference jarring and confusing.
But, I won't block—seems like everyone is happy.
Instead, maybe we should instead consider changing the Shapes tools to merge Path and Polygon and use this behavior. Double-click far from origin -> path. Double-click close to origin -> close.

@ksofiyuk I have opened a PR in the docs repo #177. Could you have a look?

@ksofiyuk Thanks for your patience! It is going in.


Prototype of adding shared resources for preview mode

Changed structure to include overlays
preview.sh is updating apiexport-patch.yaml with identityHash
added shared-resources and self binding for development environment

Marking as ready for now, with these changes when running preview.sh it will deploy application-service successfully and it's possible to create applications and components.


make the page footer prettier

there is a html file called in til/templates called "til_footer".  It is at bottom of every page.   Maybe this should be an "about", or "credits/about".

This does improve the look of the footer and is a good first step.  Thanks, I am going to merge it into dev.  The footer is too prominent, it should  just be links to separate pages (one for credits, one for about).  I will create a new issue for that.


Morning Session day 5 peer to peer code review

We thank you Felipe for sharing your code with us.
While going through your code, we have to say you did a great job.
Keep up the good work.

Thank you @Tessyrich


Why tensorflow r2.0 still load libcudart_static.a in cuda_configure.bzl

In tensorflow r2.0, I still can see libcudart_static.a copied to local cuda repository in third_party/gpus/cuda_configure.bzl file, but looks like tensorflow using cuda shared object to access Nvidia GPUs, so why still we need load static library here?

Any update?

@chengleiwang ,
Please provide details about what platform you are using (operating system, architecture). Also include your TensorFlow version. Also, did you compile from source or install a binary?
Make sure you also include the exact command if possible to produce the output included in your test case. If you are unclear what to include see the issue template displayed in the Github new issue template.
We ask for this in the issue submission template, because it is really difficult to help without that information.

My system is Ubuntu 18.04 64bit os.
TensorFlow version is r2.0.
Compiled and installed from source.
I follow the TensorFlow document to build the tf. I am ooo at this moment.but I believe you can find the cuda build configuration in /third_part/gpus/cuda/ path. There are many library use libcudart_static.a as dependence.

You can see the related code in /third_party/gpus/cuda_configure.bzl
"cudart_static": _find_cuda_lib(
"cudart_static",
repository_ctx,
cpu_value,
cuda_config.config["cuda_library_dir"],
cuda_config.cuda_version,
static = True,
),
Base on commit: 7c5aed3, tensorflow should use cuda dynamic library instead of static .

@gunan , do you know the history of how tensorflow loading cuda library? what's the purpose for using libcuda_static.a before and now?

I don't think we are using those to build the pip package though. I believe those are there so that if users do want to use the static libraries they are available. @chsigg might also know more history on this.

I agree with Yifei, the file is copied but should not be used during the actual build. I would be happy to merge a PR that removes the file copy.

Actually I tried to remove the dependency for static libraries, but failed to build tensorflow after that. I will provide more detail fail message later.
May I know who can help provide a PR?

I guess it is still needed by NCCL? See here.
Not sure if we could get rid of the static runtime library, though I'd really like to do so.

I see the static runtime dependency has been removed in https://github.com/tensorflow/tensorflow/commit/f819114a2d9d393a60e954d3a3e42d8700ff3b19#diff-063e9e964d14400387e823d7ae9c4b2f
Closing this. Let us know if you are still seeing issues.


fix: properties exported are redefined

export * from './ui/fields/index';
export * from './ui/intro/index';
export * from './ui/index';

uiFields and uiIntro are redefined through export * from ... I remove them them from ./ui/index, because nobody call them from ./ui/index

Thanks!  I agree they aren't needed..