Add Text Support for Box Annotation

[WilliamHYZhang]

Hi,

This is a simple addition to the Text structure that allows for the option of adding a bounding box (Bbox). Currently, the supported specifications include the Bbox border size, as well as the Bbox color. I modified the test annotation to use this feature, now the image should show the original "HELLO WORLD" text but now with a green bounding box around it.

Let me know what you all think!

William

EDIT: instances of bbox have been changed to boundingBox as it is more descriptive.

[WilliamHYZhang]

Also, would there be a possibility of more GCI tasks with SwiftPlot and other Tensorflow project contributions?

I would love to keep working on these, but the tasks left with Tensorflow seem to have dwindled down to be all tutorial/assignment tasks. While they are interesting (I've done a few notebooks and tutorials), I have come to really enjoy the tasks where we get to apply our knowledge and skills to create new features such as annotation support for SwiftPlot, various new datasets for Tensorflow Datasets, usage examples for Tensorflow documentation, etc. Additionally, because I'm in China at the moment, Google Colab seems to be inaccessible, so I can't do any of the tasks involving it :(

Thanks so much!

William

[KarthikRIyer]

This looks good. Instead of bbox, the parameter name can be drawBoundingBox. This is a bit more descriptive. I would have a separate example for the text annotation with the bounding box. It'll help us make sure in the future that each feature works well independently.

It would be nice to have a bounding box corner radius. You would need to make changes to the draw Rect functions in the renderers. It seems simple enough to do this in SVG, but you'd have to explore ways to do this in AGG or CoreGraphics. You might have to implement it on your own by doing some math. This isn't necessary for GCI, but I'm just writing it down here for future reference. If you u're welcome to work on it if you want.

[KarthikRIyer]

I can say only about SwiftPlot tasks. I'll be happy to create more tasks related to this project. There are some in GitHub issue that you could try, or if you have any ideas of your own, I'd be happy to accept them. If you don't find those tasks interesting let me know, we'll discuss and find something of your interest.

[WilliamHYZhang]

Awesome, thanks for the feedback! I'll work on the changes today.

[WilliamHYZhang]

Also, regarding the tasks, unfortunately because I already completed the tasks, I can't claim another instance of it.

[WilliamHYZhang]

Alright, pushed with update. Changed bbox variable to drawBoundingBox and updated tests for separate with and without bbox text annotations.

[WilliamHYZhang]

Alright, overhauled the Text structure to use the new Box as optional. However, please note that just like making the Annotation optional to the Arrow structure, now adding these are considerably more difficult than before (take a look at the test code this commit vs previous commit). Previously, the draw function was able to calculate exactly the coordinates given the width of the boundingBox. However, now with the Box as optional the boundingBox coordinates has to be "hard passed" when creating the Text annotation, which isn't ideal. Again, I still have the code to calculate these coordinates, but now I don't know where to put it (see the previous commit, code is there lines 59-64). Can anybody advise me on this?

Thanks!

[KarthikRIyer]

Why do you need to hard code it? Can't you calculate and set the size and location as you were doing before?

[WilliamHYZhang]

If we make Box optional, we construct it using the init function in Box when passing it in as an argument for boundingBox. The Box annotation itself doesn't know anything about the Text annotation. Previously, the code required to calculate the properties of the Rect passed to drawSolidRect needed access to the renderer using renderer.getTextLayoutSize. This is why it was in the draw function of Text, where we would create an instance of the Rect and draw it.

However, now they are in a sense "isolated", so I'm confused how I would calculate the properties as before.

[KarthikRIyer]

Before, you were calculating the size of the text and the location. You can do the same now and set the location and size for the box . You have location and size variable in Box annotation. Set those values.

[WilliamHYZhang]

Resolved merge conflicts.

Regarding the issue of location and size, here are some possible solutions I thought up of and their shortcomings.

• Make a separate init function in Box that would take in the Text annotation, border width, and color. Size and location would be calculated similar to before. There are two problems with this. The first problem is that the renderer also needs to be passed as described earlier. However, we only have access to the renderer during draw functions for the annotations. Secondly, passing the entire Text annotation is undesirable since in order to do this, the end user would have to set that Text annotation to a var and then pass it that way. This would get rid of the simplicity of just calling the constructor in one line with addAnnotation(annotation: Text(...))

• Make a seperate function in Text that returns a Box with the correct properties. Similar to before, with the same shortcomings. renderer still needs to be passed, and a var must be declared.

Again, the main issue is just that in order to calculate the correct properties of the Box annotation, we need all properties of the Text annotation, as well as access to the renderer. Currently, the best place to do that is in the draw function, however when using optionals, the properties are set beforehand, and we are just calling the draw function for the Box annotation now.

[WilliamHYZhang]

@KarthikRIyer maybe I am misunderstanding. Is the optional Box annotation not set by the user with its init? Where should I set those values in the code?

[KarthikRIyer]

It is set by the user. The user needs to just set the color of the box because the location and size have default parameters. You are using optional chaining and getting the box as a var. The Box itself has location and size as vars. Just use assignment to set those values. You can do this in draw if you need the renderer, as you've said above.

Something like location = Point(1,2)

[WilliamHYZhang]

Oh, got it! So sorry I didn't get you the first couple of times you explained it.

[KarthikRIyer]

No worries. I'm sorry if I wasn't clear the first few times. Try this and let me.know if you face any issues.

[WilliamHYZhang]

Assigning the values in draw is getting a bit problematic since now the function is mutating and I have to define it as such, consequently now it doesn't conform to the Annotation protocol.

EDIT: At the moment, I'm fixing this by changing the protocol and fixing the errors that I've caused by doing that.

[WilliamHYZhang]

Alright, pushed some updates!

• made draw a mutating function in Annotation protocol, and update all structures to reflect that
• add code to calculate size and location of Box optional in Text structure
• updated test to a lighter color boundingBox (pink) to make text more readable and appealing

If there's anything that needs to be changed, please let me know.

William

[KarthikRIyer]

This looks good for now. There seems to be a bug with SVG Rendering. The box width is much lesser than it was last time. I'll merge this for now. In the next PR, could you make the alpha of the text in the box example 1.0? I've approved your task as you've met the basic requirements of the task. It would be nice if you could see this to completion. Good job!

[WilliamHYZhang]

Sure! Thank you @KarthikRIyer and @odmir for the help on these annotations.

I would love to continue to work on these annotations. Could more tasks be created for the coordinate conversion, additional features to the two annotations, and possibly more annotation types? Thank you so much.

[KarthikRIyer]

@WilliamHYZhang coordinate conversion is in a way a part of this task although it wasn't a basic requirement. I'd prefer it if you completed it as a part of this task. If you want to work on any other features, I could create tasks for them.

[WilliamHYZhang]

Alright, I'll make a new PR for the coordinate conversion function first and once that's done work on tasks with additional features. Thanks again.

[KarthikRIyer]

@WilliamHYZhang you could extend your work on these annotations by implementing custom box style, with curved corners, arrow/custom shaped boxes, rotated boxes, and different styles of arrows, curved arrows like the matplotlib annotations. https://matplotlib.org/3.1.1/tutorials/text/annotations.html

[WilliamHYZhang]

Sweet! Could a task be made with implementing those advanced features once I'm done with the coordinate conversion?

[KarthikRIyer]

Sure!