Add initial support for annotations

[dcaliste]

I open an issue here, so the discussion that has been started in the comments of another PR (see #58) may be continue here and easily found back.

The current status of the implementation is the following:

• annotations are created by selecting a text and tapping on the selection, via a context menu like for URL ;
• support for highlight annotation (hightlight, under-line…) and text annotation (only linked text) ;
• annotations can be modified by tapping on them (making a dialog appears in) or deleted by choosing the pull up menu in the annotation dialog ;
• annotations are automatically saved on-the-fly to a file with a tailing « -annotated » suffix, or the same if the suffix already exists.

There are still pending issues in the current implementation:

• page is fully rerendered each time an annotation is created or modified, blanking the view ;
• page is not cropped when the annotation dialog is moved away (accepted or rejected) ;
• saving to the same file (i.e. when modifying toto-annotated.pdf) creates a broken PDF file that cannot be opened anymore ;
• there is no support for multi-page annotation (selecting text spanning over two pages and highlight it results in having text on the first page being annotated). This restriction exists in Evince also.
• the bounding box of a text annotation is wrong. It is set to the initial user selection, and not to the size of the icon used by Poppler to indicate a linked-annotation.

@pvuorela when you have time, you may ask the design team their opinion on this, as you suggested in some previous comments, to polish (or rework) the UI part. What's your opinion on this and specifically the mentionned issues ? This is currently not fast moving because of the previously mentionned issues that are tricky (broken saved file, or non-crop page) or not so easy to implement (partial page rendering), but hopefully I can address all in the coming weeks.

[pvuorela]

I'll try to bug design more on these. There was some initial plan how annotations could be done UI-wise but still not finished due to tons of things proceeding.

page is not cropped when the annotation dialog is moved away (accepted or rejected)

Same issue maybe as backstepping to front page doesn't show it until pdf page is fully away? I think something on scene graph level.

saving to the same file (i.e. when modifying toto-annotated.pdf) creates a broken PDF file that cannot be opened anymore

Hmh, any idea what's wrong with resulting file? Poppler bug somehow?

there is no support for multi-page annotation (selecting text spanning over two pages and highlight it results in having text on the first page being annotated). This restriction exists in Evince also.

IMO this should be ok.

[dcaliste]

Hmh, any idea what's wrong with resulting file? Poppler bug somehow?

Looking at Poppler code, the exportation is done by function saveIncrementalUpdate() at PDFDoc.cc#853. And this function starts by copying the source file into the destination file before appending the actual modifications. The output file is opened by poppler-base-converter.cpp#49 with a truncation of the file. So when the output file is the same than the source file, the file is first erased, then copied, then appended with modifications. Thus the result file is broken because it contains only the modifications.

Thinking about this, it's not wise to change the source file while it's still used. Indeed, at each new modification, the source file is copied into the output. Thus we will have the modifications several times.

I need to think this better, but I like the idea of having something like:

• all modifications are stored in a unique QtemporaryFile.
• in the PDFDocumentPage.qml, PDF.Document has an attribute exportDestination: <string>, which is the same if already annotated or appended with -annoted.pdf otherwise.
• when the PDFDocument object is killed, the temporary file is renamed to the value of exportDestination attribute, so the source and the destination are always separated.

There is special attention to give when implementing this to be sure that the temporary file which lives with the render thread (which owns the Poppler::Document) is actually saved to disk and the file moved before the object is destroyed.

[rburchell]

You may want to look at QSaveFile.

[dcaliste]

Thanks @rburchell, that's exactly what I had in mind, like g_file_set_contents() I'm accustomed to.

[dcaliste]

saving to the same file (i.e. when modifying toto-annotated.pdf) creates a broken PDF file that cannot be opened anymore ;

Ok, issue solved. Any modified document is saved to disk by the render thread before returning without issue anymore when saving to the same file thanks to QSaveFile.

page is not cropped when the annotation dialog is moved away (accepted or rejected)

Ok, issue solved also. After looking at the code of PageStack.qml, I understood that I misinterpreted the Inactivating and Activating status, thinking that a peek gesture in the dialog would trigger the Activating status. In fact, the solution is simple : the page should be croped whenever not active.

I'll continue to work on the remaining issues and report here. The current code can still be tested from my annotate branch.

[dcaliste]

page is fully rerendered each time an annotation is created or modified, blanking the view ;

This is not happening anymore, I've added a commit that allows to update a subpart of a page, locally on the annotation bounding box. The generated texture is added to the node graph like a patch on the big page texture.

So, beside the UI revamp, the only blocking issue that remains is the wrong bounding box for linked text annotations.

[dcaliste]

the bounding box of a text annotation is wrong.

Looking into Poppler code, the rendering size of the icon to represent a linked text annotation is hard-coded to 24×24 in 1/72th of a inch. So, I do the same for the declared bounding box of text annotation. Now, we can tap on the icon safely !

Feel free to test the code. I don't see any blocking issues from my side anymore. Thanks for the comments and the tips.

[pvuorela]

Now finally some design available for annotations. Includes also a way of getting rid of split view. That is still subject to change, but luckily should be independent of annotation.

Would this make sense to you?

[dcaliste]

Wahou, great. Yes, it's crystal clear. Very nicely well thought (as always) in every detail (e.g. the filename spanning full width in detail page). I think, the way the toolbar interacts with the view (tap the icon, have a small banner explaining what to do, and when accustomed to, do it directly) is very intuitive and fast and consistent between the various actions.

I may have some remarks anyway, that may or may not be relevant:

• for annotations, especially text annotations, there is no mention of the author. That's from my point of view, an over simplification of it. Annotations can be used to comment on a document, and possibly by several commenters. In that use-case, having the author is a relevant information.
• for highlight annotations, there is no possibility to select the kind of highlight (strike through, underline…). Once again, when commenting on a text, it's convenient to choose the type of highlighting.
• for highlight annotations, there is no mention of associated text. Still my use case of commenting, when there is a strike through, a text explaining the reason would be nice for the next reader. Besides, it seems to me that there is no tap action associated to the highlighted area (just a long tap). So it could be associated to a text page as for the text annotation ? There may be an additional entry in the context menu of an highlight annotation, like « add note ».
• there is no mention of the table of content (except maybe in the ToDos while mentioning the go to page capability), should it still be has an attached page ?

Besides these small remarks, it looks really great. It requires to rework the layout of the toolbar. I'll begin with this for the search capability. Then, when the toolbar is up to date and we agree on its implementation, I'll try to propose something that implements this design for the annotation PR, to avoid too many conflicts if toolbar is heavily modified later on. Do you agree ?

[pvuorela]

for annotations, especially text annotations, there is no mention of the author.

Checking Adobe Acrobat on Android, there's a settings page including author name. Maybe we could have something like that. Don't think author name needs to change for every annotation?

for highlight annotations, there is no possibility to select the kind of highlight (strike through, underline…).

Good point. How I see it now, only coloring parts of text is possible. Need to consider e.g. separate actions for underline and strike through.

One problem is how many items the toolbar can contain. But maybe highlight type of annotations could be shown only when selection exist, dropping then search and note actions. Starting highlight with action first and then selection can be problematic anyway if lifting finger applies the highlight already.

for highlight annotations, there is no mention of associated text. Still my use case of commenting, when there is a strike through, a text explaining the reason would be nice for the next reader.

FWIW, Acrobat doesn't allow associated text either. Would adding a note or then annotation text item suffice?

Besides, it seems to me that there is no tap action associated to the highlighted area (just a long tap). So it could be associated to a text page as for the text annotation ? There may be an additional entry in the context menu of an highlight annotation, like « add note ».

"after tap and long tap the context sensitive menu appears". But context menu is an option to consider.

there is no mention of the table of content (except maybe in the ToDos while mentioning the go to page capability), should it still be has an attached page ?

At the moment, yes.

I'll begin with this for the search capability. Then, when the toolbar is up to date and we agree on its implementation, I'll try to propose something that implements this design for the annotation PR, to avoid too many conflicts if toolbar is heavily modified later on. Do you agree ?

Yes, iterative approach makes sense.

[dcaliste]

Don't think author name needs to change for every annotation?

Yes, I think so. My use case is this one: Person A receive the PDF document and create annotations a1, a2, a3. Then sends the annotated document to person B, who creates some new annotations a4, a5 and modify a1. Finally person C receives the document and need to make a synthesis of all annotation. It's necessary for person C to know that a1 was done by person A and a4 by person B. It's different from the document author. You can see this as the "save modification" feature of Libre Office.

Need to consider e.g. separate actions for underline and strike through.

As an after thought, it may be proposed as a styling modification like the colour is proposed in the context menu. The context menu would have an additional line with horizontally aligned square buttons for highlight, underline, strike through…

Would adding a note or then annotation text item suffice?

Well, implementation allows to set text with every kind of annotation, including video or highlight annotations for instance. That would be strange to have a text annotation beside an highlight annotation. I've sent you by mail an example of highlighted document as I've received from an editor before publication.

[pvuorela]

Yes, I think so. My use case is this one: Person A receive the PDF document and create annotations a1, a2, a3. Then sends the annotated document to person B, who creates some new annotations a4, a5 and modify a1. Finally person C receives the document and need to make a synthesis of all annotation. It's necessary for person C to know that a1 was done by person A and a4 by person B. It's different from the document author. You can see this as the "save modification" feature of Libre Office.

Is this something expected to be done on a mobile phone? Sounds tedious job to manually combine annotations from two sources. Would be possible with a single setting if persons A and B did their modifications on Sailfish devices and person C combined results with some better tool. Also as user it would feel tedious if I had to write my name separately for each annotation, so global value could be a starting point?

As an after thought, it may be proposed as a styling modification like the colour is proposed in the context menu. The context menu would have an additional line with horizontally aligned square buttons for highlight, underline, strike through…

Yea, another option too.

Well, implementation allows to set text with every kind of annotation, including video or highlight annotations for instance. That would be strange to have a text annotation beside an highlight annotation. I've sent you by mail an example of highlighted document as I've received from an editor before publication.

Thanks for the file, helps to understand what's expected. It's a balancing act on what features a mobile device application should provide and how complex the UI can become. Do understand your use case, but then again even Acrobat doesn't allow that. But maybe context menu could also include an action to show and edit text related to annotations.

[dcaliste]

Is this something expected to be done on a mobile phone?

No, I agree that the real job is done on a desktop computer, with mouse and big screen ! But I think that the mobile can be usefull to consult such documents while being in transportations or in a waiting room…

Also as user it would feel tedious if I had to write my name separately for each annotation, so global value could be a starting point?

In my current implementation, you enter your name once per document and it's reused for all other annotations. I agree that it's not optimal.

Well, let say that this author information is not as important as that ! But it would be nice to have the various highlight styles, I think, and maybe the text data associated to highlight also.

Anyway, let's implement what design has nicely suggested first, and then propose some additions to them depending on the feedback we can have or various tries we can do.