Adding Additional Annotation Support #742

johncharris · 2021-06-30T19:22:06Z

I'm working on more complete annotation support. I ran into an issue I'm unsure of.

Where is the best place to add the appearance? I added a method to do this, but I think there is a better way.

Once I get feedback on this I'll add more types of annotations.

…tations.

DavBfr

Please run make format to reformat all the code.
or dart format --fix . if you can't run the Makefile.

DavBfr · 2021-06-30T19:43:35Z

pdf/lib/src/widgets/annotations.dart

      ),
    );
  }
 }

 class AnnotationUrl extends AnnotationBuilder {
-  AnnotationUrl(this.destination);
+  AnnotationUrl(this.destination, {this.date, this.subject, this.author});


Can you add a comma after this.author?

DavBfr · 2021-06-30T19:49:46Z

pdf/lib/src/pdf/annotation.dart

+
+  final PdfColor? interiorColor;
+
+  void addAppearance() {


I think you should not have this here, see the other comment on AnnotationPolygon

Were is the best place to run the code that will make the appearance stream? I agree this isn't it.

You already have /Vertices, so the annotation seems fine. Then the appearance would be used to display user graphics on top of that, but still part of the annotation.

You can remove the appearance code from here, and add it to the Widget AnnotationPolygon as a child widget.

Same as PdfButtonField => FlatButton. Basically everything in lib/src/pdf is low-level dumb PDF, and lib/src/widgets is what developers expect for an easy-to-use API.

DavBfr · 2021-06-30T19:56:52Z

pdf/lib/src/widgets/annotations.dart

+}
+
+class AnnotationPolygon extends AnnotationBuilder {
+  AnnotationPolygon(this.points, {this.color, this.interiorColor, this.border, this.date, this.subject, this.author});


To keep the Flutter spirit of Widgets, the polygon drawing should be a child widget.
A Polygon widget will get the same List on its constructor and will just draw. It can be used outside the AnnotationPolygon with more settings, maybe a ClipPolygon widget can be interesting to clip its child, this child can be used to draw anything else within the defined annotation.
What do you think?

For proper PDF compatibility it should be use the different annotation subtypes Polygon, Polyline, Ink, etc.

That said, I had issues rendering the polygon annotation without the stream. Notably, PDF.js has issues with this 6810.

My preference would be to do it by the book for the most compatibility. I like the idea of having some drawing widgets the convert into streams. I haven't looked too see what this library has for that.

Look at the Signature Widget: it draws its child into an appearance stream.

DavBfr · 2021-07-01T09:54:42Z

pdf/lib/src/pdf/annotation.dart

+
+  final PdfColor? interiorColor;
+
+  void addAppearance() {


You already have /Vertices, so the annotation seems fine. Then the appearance would be used to display user graphics on top of that, but still part of the annotation.

DavBfr · 2021-07-01T10:02:41Z

pdf/lib/src/pdf/annotation.dart

+
+  final PdfColor? interiorColor;
+
+  void addAppearance() {


You can remove the appearance code from here, and add it to the Widget AnnotationPolygon as a child widget.

Same as PdfButtonField => FlatButton. Basically everything in lib/src/pdf is low-level dumb PDF, and lib/src/widgets is what developers expect for an easy-to-use API.

DavBfr · 2021-07-01T10:04:59Z

pdf/lib/src/widgets/annotations.dart

+}
+
+class AnnotationPolygon extends AnnotationBuilder {
+  AnnotationPolygon(this.points, {this.color, this.interiorColor, this.border, this.date, this.subject, this.author});


Look at the Signature Widget: it draws its child into an appearance stream.

codecov-commenter · 2021-07-01T10:11:25Z

Codecov Report

Merging #742 (2d53d45) into master (2e17c62) will decrease coverage by 0.24%.
The diff coverage is 8.25%.

@@            Coverage Diff             @@
##           master     #742      +/-   ##
==========================================
- Coverage   42.54%   42.30%   -0.25%     
==========================================
  Files         123      123              
  Lines       16022    16126     +104     
==========================================
+ Hits         6817     6822       +5     
- Misses       9205     9304      +99

Impacted Files	Coverage Δ
pdf/lib/src/widgets/widget.dart	`80.57% <0.00%> (-2.39%)`	⬇️
pdf/lib/src/pdf/annotation.dart	`63.97% <4.54%> (-18.42%)`	⬇️
pdf/lib/src/widgets/annotations.dart	`48.36% <11.47%> (-24.83%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e17c62...2d53d45. Read the comment docs.

DavBfr · 2021-07-03T11:37:54Z

pdf/lib/src/pdf/annotation.dart

+      String? author,
+      String? content,})
+      : super(
+          subtype: '/InkList',


I don't see this annotation type in the PDF documentation. Only Ink which has an InkList key.

DavBfr · 2021-07-03T11:40:30Z

pdf/lib/src/widgets/annotations.dart

+    String? subject,
+    String? content,
+  }) : super(
+            child: Rectangle(


What about having this:

SquareAnnotation({ ... Widget? child, }) : super( child: child ?? Rectangle(...), builder: ...

DavBfr · 2021-07-03T17:51:17Z

pdf/lib/src/widgets/annotations.dart

+    String? content,
+    String? subject,
+  }) : super(
+            child: InkList(


Could be replaced by a list of Polygons

DavBfr · 2021-07-03T17:52:36Z

pdf/lib/src/widgets/annotations.dart

+            ));
+}
+
+class PolyLineAnnotation extends Annotation {


Why not reuse Polygon with a closed parameter?

…losed parameter.

DavBfr · 2021-07-06T10:44:12Z

pdf/lib/src/widgets/annotations.dart

+    final maxX = allPoints.map((point) => point.x).reduce(max);
+    final maxY = allPoints.map((point) => point.y).reduce(max);
+    final rect =
+        context.localToGlobal(PdfRect(minX, minY, maxX - minX, maxY - maxX));


maxY - maxX?

DavBfr · 2021-07-06T10:44:58Z

pdf/lib/src/pdf/annotation.dart

+            date: date,
+            color: color,
+            subject: subject,
+            author: author);


missing comma

DavBfr · 2021-07-06T10:45:04Z

pdf/lib/src/pdf/annotation.dart

+            date: date,
+            color: color,
+            subject: subject,
+            author: author);


missing comma

DavBfr · 2021-07-06T10:45:10Z

pdf/lib/src/pdf/annotation.dart

+            date: date,
+            color: color,
+            subject: subject,
+            author: author);


missing comma

DavBfr · 2021-07-06T10:45:21Z

pdf/lib/src/pdf/annotation.dart

+            date: date,
+            color: color,
+            subject: subject,
+            author: author);


missing comma

DavBfr · 2021-07-06T10:49:47Z

pdf/lib/src/pdf/annotation.dart

+      String? author,
+      bool closed = true})
+      : super(
+            subtype: closed ? "/PolyLine" : '/Polygon',


Please use single quotes.

DavBfr · 2021-07-06T10:50:48Z

pdf/lib/src/widgets/annotations.dart

+          url: destination,
+          date: date,
+          author: author,
+          subject: subject),


missing comma

DavBfr · 2021-07-06T10:51:11Z

pdf/lib/src/widgets/annotations.dart

+          interiorColor: interiorColor,
+          date: date,
+          author: author,
+          subject: subject),


missing comma

DavBfr · 2021-07-06T10:51:19Z

pdf/lib/src/widgets/annotations.dart

+        interiorColor: interiorColor,
+        date: date,
+        author: author,
+        subject: subject);


missing comma

DavBfr · 2021-07-06T10:51:35Z

pdf/lib/src/widgets/annotations.dart

+          interiorColor: interiorColor,
+          date: date,
+          author: author,
+          subject: subject),


missing comma

DavBfr · 2021-07-22T16:13:19Z

Merged, thanks.

John Harris added 2 commits June 29, 2021 11:20

Added the Square Annotation and date, author, and subject to all anno…

7c393c3

…tations.

Added Circle and Polygon Annotations.

2d53d45

DavBfr requested changes Jun 30, 2021

View reviewed changes

DavBfr requested changes Jul 1, 2021

View reviewed changes

John Harris added 2 commits July 2, 2021 14:07

Added a circle shape widget.

4d86bd2

Completed Annotations for Circle, Square, Polygon, PolyLine and InkList.

6f8e0fb

DavBfr reviewed Jul 3, 2021

View reviewed changes

DavBfr requested changes Jul 3, 2021

View reviewed changes

Changed to PolyLine annotation to use the Polygon annotation with a c…

da6fd5a

…losed parameter.

DavBfr requested changes Jul 6, 2021

View reviewed changes

Flipped the polygon and ink list on y axis.

06801c1

johncharris marked this pull request as ready for review July 13, 2021 18:41

DavBfr closed this Jul 22, 2021

Uh oh!

Adding Additional Annotation Support #742

Adding Additional Annotation Support #742

Uh oh!

Conversation

johncharris commented Jun 30, 2021

Uh oh!

DavBfr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 1, 2021

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DavBfr commented Jul 22, 2021

Uh oh!

Uh oh!