Add image overlays to annotation schema #742

naglepuff · 2021-12-17T14:45:21Z

This PR addresses server-side changes for #678

Changes include:

Modification of existing annotation schema to allow for specification of an image overlay

An image overlay is specified on the annotation itself, and not as an element. At a minimum, an image overlay requires a girder large image item ID. Opacity and location can also be specified. Location is specified with properties for position of the top-left corner of the image to overlay, as well as ta height and width. By default the overlaid image has opacity 1 and is aligned in the upper left in its native resolution.

The reason I went with adding the overlay as a direct child to the annotation schema, and not as a type of annotation element, is that I am trying to avoid a major refactor of what an annotation element is. It seems that a lot of downstream code assumes that annotation elements are made up of points/lines/polygons, and image overlays will need to be handled differently by client code (rendering on different layers, ensuring the linked tiled image exists).

I am open to discussing other possibilities regarding how best to model image overlays on the back end. Particularly I'm wondering if my reasoning above is indeed valid, or if there's another, better approach, like making a dedicated girder Model for image overlays.

manthey · 2021-12-17T14:50:18Z

girder_annotation/girder_large_image_annotation/models/annotation.py

+                    'upperLeftCorner': coordSchema,
+                    'width': {
+                        'type': 'number',
+                        'minimum': 0


We probably want these to be exclusiveMinimum rather than minimum.

We probably will eventually want to allow for an affine transform matrix instead. We end up with six values: xoffset, yoffset, plus a 2x2 scaling matrix [[s11, s12], [s21, s22]]. For the width/height condition, this ends up being [[specified width/image width, 0], [0, specified height/image height]]. In many instances, we don't want to apply a scale or skew, but just an offset and a rotation, but we can leave that to whoever specifies the matrix.

We could add an optional field of transform which would be an array of exactly four numbers (and document them). In this case, it would be nice to specify that you can't specify a transform AND a width or height.

See 95c8b45. I think if we eventually want to allow for an affine transform matrix, it makes sense to be able to specify that now.

manthey · 2021-12-17T14:50:55Z

girder_annotation/girder_large_image_annotation/models/annotation.py

+                        'minimum': 0
+                    }
+                },
+                'required': ['upperLeftCorner', 'width', 'height']


We shouldn't need to require these -- if they aren't specified they can default to 0, 0 for the upper left and the image's actual width and height.

See 95c8b45. Properties for the position of the overlay are no longer specified as required.

manthey · 2021-12-17T15:15:46Z

girder_annotation/girder_large_image_annotation/models/annotation.py

@@ -496,7 +536,8 @@ class AnnotationSchema:
                'title': 'Image Markup',
                'description': 'Subjective things that apply to a '
                               'spatial region.'
-            }
+            },
+            'overlay': overlaySchema


I'd rather this was just another possible annotation element. We can expect the viewer to do something similar to how we handle heatmaps -- in web_client/annotations/convertFeatures.js we create a layer per heatmap (which isn't actually strictly necessary, as two heatmaps could share a layer) and manage it there.

See d0aa882. Once I added image overlays as an option for elements, I was hitting errors on upload of a new annotation during the calculation of the bounding box.

My naive approach to bounding box calculation for image overlays was just to set it to the size of the entire base image. Perhaps a better approach would be to use the transform properties to calculate the exact coordinates of the overlay. Thoughts?

It would be better to calculate the actual bounding box, since, in principle, you could have several images and only need to fetch the ones that intersect some region. Similarly, the overlay could actually extend outside of the base image.

I just committed 9c44499, which should do a better job of calculating the bounding box. Let me know if you have any additional thoughts/suggestions.

manthey · 2021-12-17T15:23:57Z

girder_annotation/girder_large_image_annotation/models/annotation.py

+        'properties': {
+            'girderId': {
+                'type': 'string',
+                'description': 'Girder item ID containing the image to '


We should enforce that this is a 24 digit hexadecimal number 'pattern': '^[0-9a-f]{24}$'.

Fixed in 95c8b45

- Use affine transform matrix + offset - Specify pattern for girderId

- Generate bounding boxes for image overlays based on the large image item

manthey · 2021-12-20T19:54:41Z

girder_annotation/girder_large_image_annotation/models/annotationelement.py

+                lowx, highx, lowy, highy = Annotationelement()._overlayBounds(element)
+                baseImageItem = ImageItem().load(annotation['itemId'], force=True)
+                baseImageMetadata = ImageItem().getMetadata(baseImageItem)
+                highz = baseImageMetadata.get('levels', 0)


In thinking about this, I don't think we want to store levels in the z bounding box -- the levels aren't conceptually heights (whereas there ARE multi-frame images that have a z-stack of frames). I think for now, the z will always just be zero. If we do that, then we don't have to pass the parent annotation into the _boundingBox method.

manthey · 2021-12-20T19:55:53Z

girder_annotation/girder_large_image_annotation/models/annotationelement.py

+                baseImageMetadata = ImageItem().getMetadata(baseImageItem)
+                highz = baseImageMetadata.get('levels', 0)
+            except Exception as e:
+                logger.error('Error generating bounding box for image overlay annotation: %s' % e)


I think it would be better to have this guard in the _overlayBounds method rather than here.

See 95cf345

Also move some try/except code to the _overlayBounds method from _boundingBox.

manthey · 2021-12-21T14:01:46Z

Great. It would be nice to have some basic tests of this. For instance, you could add a test in the girder_annotation/test_annotation/test_annotations.py file, in the TestLargeImageAnnotationElement class, adding something like:

    def testOverlayAnnotation(self, server, admin, fsAssetstore):
        file = utilities.uploadExternalFile('sample_image.ptif', admin, fsAssetstore)
        itemId = str(file['itemId'])
        bbox = Annotationelement()._boundingBox(
            {'type': 'imageoverlay', 'girderId': itemId})
        assert bbox == {
            'lowx': 0, 'lowy': 0, 'lowz': 0,
            'highx': 58368, 'highy': 12288, 'highz': 0,
            'details': 1,
            'size': (58368**2 + 12288**2)**0.5}

would test some of the bounding box code. You'll want some additional conditions with offset and matrix values set.

manthey · 2021-12-21T16:51:51Z

girder_annotation/test_annotation/test_annotations.py

+        assert lowy == 1000
+        assert highx == (58368 / 2) + 500
+        assert highy == (12288 / 2) + 1000
+


Great. Can you also add a test that invokes the _boundingBox method to test that it invokes the _overlayBounds method appropriately? It doesn't have to test the various transform options, just the 'type': 'imageoverlay' pass-through.

manthey · 2021-12-21T16:53:23Z

girder_annotation/girder_large_image_annotation/models/annotationelement.py

+            lowy = min([corner[1] for corner in corners]).item()
+            highy = max([corner[1] for corner in corners]).item()
+        except Exception as e:
+            logger.error('Error generating bounding box for image overlay annotation: %s' % e)


You could just do logger.exception(''Error generating bounding box for image overlay annotation')

Also use `logger.exception` over `logger.error`

manthey

You might have to convert this to a non-draft PR in order to merge. It looks good.

Add image overlays to annotation schema

07a3932

naglepuff requested a review from manthey December 17, 2021 14:45

manthey reviewed Dec 17, 2021

View reviewed changes

naglepuff added 3 commits December 17, 2021 13:08

Modify image overlay schema

95c8b45

- Use affine transform matrix + offset - Specify pattern for girderId

Make image overlays an element type

d0aa882

- Generate bounding boxes for image overlays based on the large image item

Use overlay transform to compute bounding box

9c44499

manthey reviewed Dec 20, 2021

View reviewed changes

naglepuff added 2 commits December 20, 2021 15:54

Use 0 for the z bounding box for image overlays

95cf345

Also move some try/except code to the _overlayBounds method from _boundingBox.

Remove finally block in _overlayBounds

6fe5fb9

Add tests for Annotationelement::_overlayBounds

d6774aa

manthey reviewed Dec 21, 2021

View reviewed changes

naglepuff added 3 commits December 21, 2021 12:03

Update annotation docs for image overlays

00fcf91

Add additional test for _boundingBox method

f38b43c

Also use `logger.exception` over `logger.error`

Fix lint issue

b9b3c2d

manthey approved these changes Dec 21, 2021

View reviewed changes

naglepuff marked this pull request as ready for review December 21, 2021 17:55

naglepuff self-assigned this Dec 21, 2021

naglepuff merged commit 9164634 into master Dec 21, 2021

naglepuff deleted the overlay-schema branch December 21, 2021 19:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add image overlays to annotation schema #742

Add image overlays to annotation schema #742

naglepuff commented Dec 17, 2021

manthey Dec 17, 2021

manthey Dec 17, 2021

manthey Dec 17, 2021

naglepuff Dec 20, 2021

manthey Dec 17, 2021

naglepuff Dec 20, 2021

manthey Dec 17, 2021

naglepuff Dec 20, 2021

manthey Dec 20, 2021

naglepuff Dec 20, 2021

manthey Dec 17, 2021

naglepuff Dec 20, 2021

manthey Dec 20, 2021

manthey Dec 20, 2021

naglepuff Dec 20, 2021

manthey commented Dec 21, 2021

manthey Dec 21, 2021

manthey Dec 21, 2021

manthey left a comment

Add image overlays to annotation schema #742

Add image overlays to annotation schema #742

Conversation

naglepuff commented Dec 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manthey commented Dec 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manthey left a comment

Choose a reason for hiding this comment