Bug 23108

Summary: Poppler should provide default appearance streams for annotations without
Product: poppler Reporter: Alexander Hunziker <alex.hunziker>
Component: generalAssignee: poppler-bugs <poppler-bugs>
Status: RESOLVED MOVED QA Contact:
Severity: normal    
Priority: medium CC: gpoo, jonathan.verner, mozbugbox, nalimilan, nate, nsoranzo, tadej.j, tgpraveen89, vish
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: PDF file with annotations
Evince icons
Set of patches
poppler svg icons
Poppler icons with background color
Updated set of patches
Icons for tag and graph
Icons for mic and speaker
Another patchset

Description Alexander Hunziker 2009-08-03 05:49:11 UTC
Created attachment 28298 [details]
PDF file with annotations

The file

http://ftp.roedu.net/mirrors/ctan.org/macros/latex/contrib/pdfcomment/doc/pdfcomment.pdf

has annotations that do not show with Evince 2.27 and Poppler 0.11

KaL on #evince says: 

 ok, I know what the problem is
 and yes, it's a poppler issue
 that annot doesn't have an appearance stream, poppler should provide default AP streams for such cases
Comment 1 Carlos Garcia Campos 2009-10-07 00:38:25 UTC
*** Bug 24364 has been marked as a duplicate of this bug. ***
Comment 2 Vish 2009-12-02 06:33:06 UTC
Created attachment 31662 [details]
Evince icons

Attaching the 48px icons  , I had shown Kal on #evince. 
He mentioned few problems on the evince side and was not sure if the annotations would work , do let me know if it works and I can get the rest of the icons done too.

Also , attaching the 24px versions of a couple of icons  ,of the idea I had.
To use the square as the constant shape for the icon and symbols embedded in the square. That way the instant the user notices the shape , the user realizes it is an annotation. [This is just an idea , if the icons are desired similar to the acroread , that can be done too.]
Comment 3 Carlos Garcia Campos 2009-12-05 03:40:58 UTC
A couple of screenshots showing how pdf_commenting_new.pdf  looks with that icon:

http://people.freedesktop.org/~carlosgc/poppler-ap1.png
http://people.freedesktop.org/~carlosgc/poppler-ap2.png

Second screenshot doesn't look correct due to bug #25441 :-(
Comment 4 Carlos Garcia Campos 2009-12-06 08:22:39 UTC
Created attachment 31779 [details] [review]
Set of patches

This set of patches adds appaearance streams for Text, Line and Gemoetry annotations. In the case of Text annotations it uses the icon provided by mac_v in this bug (we might want to modify it, but the code is mostly the same)

It can be tested with this document:

http://www.pdfill.com/example/pdf_commenting_new.pdf
Comment 5 Albert Astals Cid 2009-12-06 13:05:33 UTC
From a quick look the patch seems mostly ok though there are some indentation issues (lines that had spaces and you add tabs) also notes in page 3 seem too small (not sure you are tackling this already)

Want me to run reg testing with your changes? Or not still?
Comment 6 Carlos Garcia Campos 2009-12-07 00:39:47 UTC
(In reply to comment #5)
> From a quick look the patch seems mostly ok though there are some indentation
> issues (lines that had spaces and you add tabs)

hmm, I'll review it, I hadn't noticed it. 

> also notes in page 3 seem too
> small (not sure you are tackling this already)

Yes, it's because of bug #25441, not sure how to fix it, though.
 
> Want me to run reg testing with your changes? Or not still?
> 

Not yet, I'm going to add more icons for Text annots and probably for file attachment annots too. 

Thanks for reviewing :-)
Comment 7 Carlos Garcia Campos 2009-12-07 05:44:34 UTC
Thanks to ulisse and mac_v we'll have more icons soon. I still don't know how to apply the color specified in the annotation dictionary, the spec only says it should be used to draw the annot background. We've done severals tests:

1.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note.png
2.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note2.png
3.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note3.png
4.- http://people.freedesktop.org/~carlosgc/poppler-paperclip2.png
5.- http://people.freedesktop.org/~carlosgc/poppler-paperclip3.png

The important thing here is not the icon itself, but how we apply the annot color (yellow in this case). 1) and 5) are indeed the same approach but with different icons. 

which one do you prefer?
Comment 8 Vish 2009-12-07 10:03:10 UTC
Created attachment 31819 [details]
poppler svg icons

Ulisse and myself have completed the set of icons as requested by Kal.

Attaching the svg's to the bug report
Comment 9 Praveen Thirukonda 2009-12-09 02:00:12 UTC
(In reply to comment #7)
> Thanks to ulisse and mac_v we'll have more icons soon. I still don't know how
> to apply the color specified in the annotation dictionary, the spec only says
> it should be used to draw the annot background. We've done severals tests:
> 
> 1.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note.png
> 2.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note2.png
> 3.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note3.png
> 4.- http://people.freedesktop.org/~carlosgc/poppler-paperclip2.png
> 5.- http://people.freedesktop.org/~carlosgc/poppler-paperclip3.png
> 
> The important thing here is not the icon itself, but how we apply the annot
> color (yellow in this case). 1) and 5) are indeed the same approach but with
> different icons. 
> 
> which one do you prefer?
> 

 2.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note2.png looks good to me.
Comment 10 Vish 2009-12-09 02:40:22 UTC
(In reply to comment #9)
> 
>  2.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note2.png looks
> good to me.
> 

Ulisse and myself really dont like option 2,  for 2 reasons. 
- It uses too much color and distracts the user from the real content of the page.
- If we are to use option 2 , then all the annotation icons wouldnt look consistent.

The current icons [attached in comment #8] will all use the color in the "frame" the corners(when specified) while the icons themselves would be grey and will look consistent. > http://imagebin.ca/view/B0p_8c.html

If the icons are to be done as in option 2  , we'd end up with the same loud colored icons of acroread , which are very distracting from the page contents. 
Comment 11 Praveen Thirukonda 2009-12-09 07:31:56 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > 
> >  2.- http://people.freedesktop.org/~carlosgc/poppler-paperclip-note2.png looks
> > good to me.
> > 
> 
> Ulisse and myself really dont like option 2,  for 2 reasons. 
> - It uses too much color and distracts the user from the real content of the
> page.
> - If we are to use option 2 , then all the annotation icons wouldnt look
> consistent.
> 
> The current icons [attached in comment #8] will all use the color in the
> "frame" the corners(when specified) while the icons themselves would be grey
> and will look consistent. > http://imagebin.ca/view/B0p_8c.html
> 
> If the icons are to be done as in option 2  , we'd end up with the same loud
> colored icons of acroread , which are very distracting from the page contents. 
> 

yes,i hguess you are right.
comment #8 icons are better
Comment 12 Albert Astals Cid 2009-12-09 11:46:16 UTC
You may or may not like how Adobe Reader draws the icons but the specification says how they should be drawn and not doing it the way the specification says because you think it looks ugly is not an option, sorry.
Comment 13 Vish 2009-12-09 23:20:05 UTC
(In reply to comment #12)
> You may or may not like how Adobe Reader draws the icons but the specification
> says how they should be drawn and not doing it the way the specification says
> because you think it looks ugly is not an option, sorry.
> 

Are you sure that the icons are specified to be done the exact same way?
The specs seem to only mention >
"C  - array -  The background of the annotation’s icon when closed"

But , strictly speaking the acroread icon's background isnt really colored. 
Rather the color is applied within the icon. While the background is transparent.

It seems that the acroread icons were designed to only allow the use of color rather than sticking to the exact specs. IMO , we can take the same liberties too.

To me the specs are a little fuzzy , and too huge. 
I might have missed the part where the icon design was specified. 
Could you point the right page where the icon design is mentioned ? [we can correct the icons accordingly]
Comment 14 Carlos Garcia Campos 2009-12-10 01:21:02 UTC
Well, I also prefer option two, but the spec doesn't say proposed icons are wrong.
Comment 15 Albert Astals Cid 2009-12-10 14:46:21 UTC
Well, that's a discussion of terminology, but for me

http://people.freedesktop.org/~carlosgc/poppler-paperclip-note2.png
is the only one that is painting the background as said on the spec

http://people.freedesktop.org/~carlosgc/poppler-paperclip-note3.png
and
http://people.freedesktop.org/~carlosgc/poppler-paperclip3.png
are using the color in a frame

http://people.freedesktop.org/~carlosgc/poppler-paperclip-note.png
and
http://people.freedesktop.org/~carlosgc/poppler-paperclip2.png
are using the color in the pen
Comment 16 Vish 2009-12-13 10:22:22 UTC
Created attachment 32041 [details]
Poppler icons with background color

Attaching the latest set of icons.
Comment 17 Carlos Garcia Campos 2009-12-14 11:41:48 UTC
Created attachment 32073 [details] [review]
Updated set of patches

Here is the updated set of patches for current master using the new icons.
Comment 18 Vish 2009-12-16 05:45:24 UTC
Created attachment 32120 [details]
Icons for tag and graph

Annot Icons for tag and graph
Comment 19 Vish 2009-12-16 08:35:34 UTC
Created attachment 32124 [details]
Icons for mic and speaker

Annot Icons for mic and speaker
Comment 20 Carlos Garcia Campos 2009-12-17 09:34:03 UTC
Created attachment 32150 [details] [review]
Another patchset

This patchset applies on top of the previous one and includes:

 - Fix a memleak of previous patches
 - Fix hightlight annots (see commit message) and provide appearance streams for TextMarkup
 - Add Graph and Tag icons for FileAttachment
 - Add speaker and mic icons for Sound 
 - Fail if QuadPoints is not present in TextMarkup since it's required
Comment 21 Carlos Garcia Campos 2009-12-17 11:32:11 UTC
I've just pushed both patch sets. I leave the bug open becuase there are still annotations missing default appearance streams (Free Text annots, rubber stamp, ...)
Comment 22 Albert Astals Cid 2012-11-30 21:59:14 UTC
*** Bug 36660 has been marked as a duplicate of this bug. ***
Comment 23 Kevin Burton 2018-05-23 16:09:19 UTC
Was this ever merged? 

I just filed a bug in Okular related to this.

https://bugs.kde.org/show_bug.cgi?id=394620

This was 9 years ago but the ticket is still open and the patches still exist.
Comment 24 Albert Astals Cid 2018-05-23 19:23:53 UTC
(In reply to Kevin Burton from comment #23)
> Was this ever merged? 

Do you know how to read? comment #21 is *VERY* clear, isn't it?
Comment 25 Nate Graham 2018-05-23 19:26:59 UTC
Let's try to keep the comments constructive and technical so we can avoid arguments that will distract us from the common goal that we all share of improving annotations.
Comment 26 GitLab Migration User 2018-08-21 11:07:42 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/poppler/poppler/issues/522.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.