feat: add superpixel-based marker

ChasNelson1990 commented 2 years ago

Description

The idea here is that when we toggle the Super Marker on we calculate a super-pixel image (should have region approximately the size of the brush but instead of circles they should snap to intensity boundaries).

This should make annotating images quicker in many cases as the details kind of get "auto caught".

At the moment you can only add one superpixel at a time (i.e. click to add). I have some experimental code for being able to drag and fill a series of superpixels but it's mega slow (https://github.com/gliff-ai/annotate/blob/7654e743277dadd129a23e7db40265a436c89010/src/toolboxes/paintbrush/Canvas.tsx#L514).

There is a lint error caused by https://github.com/gliff-ai/annotate/blob/7654e743277dadd129a23e7db40265a436c89010/src/toolboxes/paintbrush/slic.ts#L365 and for some reason the disable isn't working? @cooper667 ?

I need an icon for the button @joshuajames-smith - maybe with an alpha symbol as discussed.

@philhallbio - this will need documenting before the Expo.

Testing

No tests added yet - what do people think would be useful to add? (Will make issues for them)

Documentation

Needed.

Checklist:

[x] I have read the gliff.ai Contribution Guide.
[x] I have requested to pull a branch and not from main.
[x] I have checked all commit message styles match the requested structure.
[x] My code follows the style guidelines of this project.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have performed a self-review of my own code.
[ ] I have assigned 3 or less reviewers.
[x] New and existing unit tests pass locally with my changes.
[ ] I have added tests that prove my fix is effective or that my feature works.
[ ] My changes generate no new warnings.
[ ] I have made corresponding changes to the documentation.

ChasNelson1990 commented 2 years ago

@joshuajames-smith I labelled this "feature" and the robot added the "enhancement" label... which is a lie... any ideas why?

github-actions[bot] commented 2 years ago

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-wave-0f4ddd903-759.westeurope.azurestaticapps.net

github-actions[bot] commented 2 years ago

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-wave-0f4ddd903-759.westeurope.azurestaticapps.net

joshuajames-smith commented 2 years ago

@joshuajames-smith I labelled this "feature" and the robot added the "enhancement" label... which is a lie... any ideas why?

Mmm.. im not too sure? The templates for issues pre-defines a label but the template for PRs doesn't.

ChasNelson1990 commented 2 years ago

Mmm.. im not too sure? The templates for issues pre-defines a label but the template for PRs doesn't.

I presume this is the culprit: https://github.com/gliff-ai/.github/blob/ce7d8428c06a5c1fe0dadd9464e2d743c3fdf810/.github/labeler.yml#L7 ?

joshuajames-smith commented 2 years ago

I also wonder from a UX P.O.V. if this isn't a toggle but a separate tool from the brush tool?

ChasNelson1990 commented 2 years ago

I also wonder from a UX P.O.V. if this isn't a toggle but a separate tool from the brush tool?

But it's just a different shape brush?

ChasNelson1990 commented 2 years ago

Interestingly, the annotation doesn't visually appear on Safari?

Sorry, I'm not sure I understand this comment @joshuajames-smith?

If you also move the mouse to the left or right of the canvas, the annotation tool appears back on the canvas?

Yes, known bug - will create an issue for it.

joshuajames-smith commented 2 years ago

Mmm.. im not too sure? The templates for issues pre-defines a label but the template for PRs doesn't.

I presume this is the culprit: https://github.com/gliff-ai/.github/blob/ce7d8428c06a5c1fe0dadd9464e2d743c3fdf810/.github/labeler.yml#L7 ?

Good catch! The commit history is just you, should i swap out the label to enhancement or remove this apply label for feat: all together?

joshuajames-smith commented 2 years ago

Interestingly, the annotation doesn't visually appear on Safari?

Sorry, I'm not sure I understand this comment @joshuajames-smith?

So if I have the Super Marker active and click to annotate nothing appears - it shows on Google Chrome, but not on Safari.

ChasNelson1990 commented 2 years ago

Good catch! The commit history is just you, should i swap out the label to enhancement or remove this apply label for feat: all together?

That's because I copy and pasted this file into the .github repo but I think you created it originally so I am happy for you to do what you need.

Re: enhancement vs feature... I think conventional commits would say both are feat: in the commit message, so maybe just use feature for PRs? And keep enhancement for issues?

ChasNelson1990 commented 2 years ago

So if I have the Super Marker active and click to annotate nothing appears - it shows on Google Chrome, but not on Safari.

Can you make a bug issue please?

joshuajames-smith commented 2 years ago

I also wonder from a UX P.O.V. if this isn't a toggle but a separate tool from the brush tool?

But it's just a different shape brush?

I could see a justification for a circle vs square brush but it's function is slightly disconnected - while yes its a brush annotation, it's less manual drawing/annotating and more clicking on auto defined areas?

I can see a user case where upon they use the super marker tool to click upon several defined areas but switch to the brush or eraser tool to tidy up edges. An insitive UX would likely be to switch tools rather than turn a toggle off. This is possibly because the tool if called super marker, a toggle might work if renamed something like "turn on automatic selection" / "turn on area selection".

ChasNelson1990 commented 2 years ago

I could see a justification for a circle vs square brush but it's function is slightly disconnected - while yes its a brush annotation, it's less manual drawing/annotating and more clicking on auto defined areas?

But it is manual annotation? I don't see the difference between this and a circular paintbrush lol?

I can see a user case where upon they use the super marker tool to click upon several defined areas but switch to the brush or eraser tool to tidy up edges.

This is exactly the idea... the smart marker, just like the eraser, is a way of getting the manual paintbrush annotations that you need?

An insitive UX would likely be to switch tools rather than turn a toggle off. This is possibly because the tool if called super marker, a toggle might work if renamed something like "turn on automatic selection" / "turn on area selection".

"insitive"?... see above. In my mind this is 100% a paintbrush activity... both in terms of the backend but also in terms of the output - a manually created pixel-level labelling of an image that you can combine with non-smart brush strokes and eraser strokes completely interchangeably?

joshuajames-smith commented 2 years ago

In my mind this is 100% a paintbrush activity... both in terms of the backend but also in terms of the output - a manually created pixel-level labelling of an image that you can combine with non-smart brush strokes and eraser strokes completely interchangeably?

Happy to go with the toggle for now and if not UX issues/comments arise then it's all good!

joshuajames-smith commented 2 years ago

Tooltip to use the beta-status icon (or alpha-status icon in it's place) as above. This will provide some good user feedback!

github-actions[bot] commented 2 years ago

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-wave-0f4ddd903-759.westeurope.azurestaticapps.net

ChasNelson1990 commented 2 years ago

Despite not having done a major version change for STYLE, STYLE seems to broken and I can no longer npm run serve... @ChrisBaidoo @cooper667 we ideally need this fixed on Monday so that this feature can be prod deployed and the sales team can use it at the expo - any ideas what's gone wrong given there shouldn't have been any breaking changes?

github-actions[bot] commented 2 years ago

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-wave-0f4ddd903-759.westeurope.azurestaticapps.net

cooper667 commented 2 years ago

Despite not having done a major version change for STYLE, STYLE seems to broken and I can no longer npm run serve... @ChrisBaidoo @cooper667 we ideally need this fixed on Monday so that this feature can be prod deployed and the sales team can use it at the expo - any ideas what's gone wrong given there shouldn't have been any breaking changes?

https://github.com/gliff-ai/style/pull/215

github-actions[bot] commented 2 years ago

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-wave-0f4ddd903-759.westeurope.azurestaticapps.net

cooper667 commented 2 years ago

@joshuajames-smith needs a change in Style but think that can come in another PR?

gliff-ai / annotate