Feature/complex polygon

[TrevorBurgoyne]

Complex Polygons

Description

• Added click-and-drag functionality when drawing polygons or polylines.
• Changed polygon representation from that of a simple polygon to a complex polygon. This allows for polygons to contain holes and/or multiple disjoint regions.
  • NOTE: Self-intersecting polygons are not supported
• Hold shift when closing a polygon to continue annotating a new region or hole.
• Hold shift when moving the cursor inside a polygon to begin annotating a new region or hole.
• Press Escape or crtl+z to cancel the start of a new region or hole.
• Added fill to polygons by default, excluding any holes in the polygon.
• Added check to ensure that deprecated simple polygons are auto converted to complex polygons when loaded by ULabel.

PR Checklist

• Merged latest main
• Version number in package.json has been bumped since last release
• Version numbers match between package package.json and src/version.js
• Ran npm install and npm run build AFTER bumping the version number
• Updated documentation if necessary (currently just in api_spec.md)
• Added changes to changelog.md

Breaking API Changes

Polygon spatial_payload arrays are always Complex Polygon arrays, which is to say that instead of being an Array of points (aka a simple polygon), the spatial_payload is an Array of arrays of points (an array of simple polygons, which together make a complex polygon).

This means that the previous simple polygon format is now deprecated. However, old annotations will be auto converted upon being loaded into an instance of ULabel running on the latest version. Just note that polygon annotations will be saved in this format, so any use of the labels external to ULabel will have to accommodate this change.

Added @turf/turf, polygon-clipping as npm dependencies for some polygon operations.

[CarterSolberg]

Bugs:
The edit_suggestions dialog is currently broken when using the new complex polygons. It currently works as expected while mounted to an annotation's corners, and does not behave while editing a single segment of a polygon. If you try to edit a segment in such a way, visually nothing will happen. Then if you press ctrl + z to undo the un-rendered edit, it will delete one side of the polygon. From there using the edit_suggestion dialog, on one of the corners which used to belong to the deleted polygon segment, will cause the two segments which should not be connected to connect to each other. Pressing ctrl + z will not undo this action.

Code Suggestions:
Throughout most of the modified code in this pr, whenever you declare a new variable in a Typescript file, it has not been typed. It is recommended to do so as that's the main benefit of using Typescript in the first place. I do realize that throughout the code base we have been quite sloppy in typing every variable appropriately, however we should try to uphold the standard whenever we can.
Additionally I've noticed that you always use the == equality operator instead of the === equality operator. == will try to convert values before comparing them which can often result in weird bugs. For example:
null == undefined -> true
null === undefined -> false
"5" == 5 -> true
"5" === 5 -> false
In short === will only ever return true if the two values it's comparing are of the same type so its recommended to use it instead.

[TrevorBurgoyne]

Bugs: The edit_suggestions dialog is currently broken when using the new complex polygons. It currently works as expected while mounted to an annotation's corners, and does not behave while editing a single segment of a polygon. If you try to edit a segment in such a way, visually nothing will happen. Then if you press ctrl + z to undo the un-rendered edit, it will delete one side of the polygon. From there using the edit_suggestion dialog, on one of the corners which used to belong to the deleted polygon segment, will cause the two segments which should not be connected to connect to each other. Pressing ctrl + z will not undo this action.

Good catch, I had totally forgot that was a feature. Fix was simple: 58c4d71

Code Suggestions: Throughout most of the modified code in this pr, whenever you declare a new variable in a Typescript file, it has not been typed. It is recommended to do so as that's the main benefit of using Typescript in the first place. I do realize that throughout the code base we have been quite sloppy in typing every variable appropriately, however we should try to uphold the standard whenever we can. Additionally I've noticed that you always use the == equality operator instead of the === equality operator. == will try to convert values before comparing them which can often result in weird bugs. For example: null == undefined -> true null === undefined -> false "5" == 5 -> true "5" === 5 -> false In short === will only ever return true if the two values it's comparing are of the same type so its recommended to use it instead.

Equality changes: 2153662

Better typing (i think?): ce48afd

[CarterSolberg]

BUGS:

1. Sometime between my previous review and this review demo/resume-from.html no longer loads any annotation when opening. I checked and demo/row-filtering-example.html does load annotations when opening.
2. It seems like its impossible to create a complex polygon with a "ring" of un-shaded area. By that I mean that its impossible to create a complex polygon three layers deep. As a user I would expect that if I made another polygon region inside of an un-shaded polygon region it would then become shaded again, but that does not appear to be the current behavior.

Code:
Thank you for typing all of you Typescript variables. In Javascript everything that is not a primitive is considered an Object. It's much more useful to type each object by what its properties are and what type each of those properties is. I provided a single example in a single line comment but it would be helpful to type each Object as best as you can. This way the Typescript compiler will tell you when you try to access a property that doesn't exist on an object, or when you try to set a property on an object to the wrong type.

[TrevorBurgoyne]

2. It seems like its impossible to create a complex polygon with a "ring" of un-shaded area. By that I mean that its impossible to create a complex polygon three layers deep. As a user I would expect that if I made another polygon region inside of an un-shaded polygon region it would then become shaded again, but that does not appear to be the current behavior.

Indeed, let me tell you this is not a trivial feature. I tried several different approaches to handle arbitrarily deep holes, but nothing adequately handled every edge case, especially when you try and draw very weird concave shapes. For now I'm going with this, which handles the general cases I expect to be used in the immediate term. At the very least, the polygon outlines will draw correctly even in the remaining edge cases where the fills don't work. Even with this approach, after nesting many layers on polygons with lots of points, it was getting pretty computationally expensive to recompute on every redraw, so I've added a cache to speed things up when not editing a given polygon.

UPDATE: I've reworked the way that fills are computed to be much more robust, and take less computation as well. The idea is to determine whether a complex layer is a hole or not right after it is draw or edited, and then track that in an spatial_payload_holes array. To do this, we still check for intersections, but once we find a valid intersection between layers, we merge the non-intersecting portions into one of the layers, while the intersection remains as a new layer. The result is a much more consistent visual representation of holes/fills, and also prevents really strange shapes by just merging them into the main body when possible.

[CarterSolberg]

Bugs:
I found a visual bug involving undo \ redo. To replicate do the following. Create a simple complex polygon. Press ctrl + z to undo the action of closing the polygon. Press ctrl + shift + z to redo the action of closing the polygon. The final segment of the polygon will end wherever the cursor was when the redo action was performed instead of its original location. This bug is only visual, however, as moving the polygon will cause it to be rendered properly again.

General Comments
We have a lot of overlap of types throughout this project and I think that it's getting pretty messy and confusing. I also recently learned that the way we handle custom types in this project is not considered very good practice. In general types should be declared in 3 different places. First is inside of index.d.ts. This file should be used for types that are useful for interfacing with the project when ulabel is being imported. And index.d.ts can also be imported from internally for each file that needs a given type. The second place is inside of a dedicated types.ts (or some other generic\preferred name) for types that are useful across multiple files in the project. They can then be imported as needed from that file. Finally types can should be declared at the top of a typescript file if they are useful inside of that file, but not needed in any other file in the project. This system can keep our type system more organized and reduce duplicated types.
For example we currently have two types that behave exactly the same.
type ULabelSpatialPayload = [number, number][] and
type Point2D = [number, number]
type ULabelSpatialPayload2D = Point2D[]

Conclusion
Very cool pr, I think that the code was well done and the feature is really cool. I'd like to see the visual bug I outlined above fixed before I approve, but everything I said in the general comments is just something that I think we should keep in mind going forward but I don't think is necessary for this pr.

[TrevorBurgoyne]

Conclusion Very cool pr, I think that the code was well done and the feature is really cool. I'd like to see the visual bug I outlined above fixed before I approve, but everything I said in the general comments is just something that I think we should keep in mind going forward but I don't think is necessary for this pr.

This visual bug seems to be something I resolved in #130. I also opened #131 to track the general type refactor that you've outlined here.

[CarterSolberg]

LGTM 👍