Bug 102513

Summary:	[PATCH] Allow to control whether renderToImage shows annotations
Product:	poppler	Reporter:	oliver.sander
Component:	qt frontend	Assignee:	poppler-bugs <poppler-bugs>
Status:	RESOLVED FIXED	QA Contact:
Severity:	enhancement
Priority:	medium
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Patch that allows to control whether annotations are shown Patch that allows to control whether annotations are shown

Description oliver.sander 2017-09-01 20:30:57 UTC

Created attachment 133932 [details] [review]
Patch that allows to control whether annotations are shown

I'd like to control whether the renderToImage and renderToPainter methods show annotations or not. To this end, this patch introduces a new value 'HideAnnotations' to the Document::RenderHint enum, and makes the said methods mind it.

I have no idea whether this is a good way to get the new option into the interface, but it seemed the least invasive one to me.  renderToPainter has a dedicated PainterFlags parameter, but renderToImage doesn't see that one. Is the new RenderHint value okay?

Comment 1 Albert Astals Cid 2017-09-01 22:36:29 UTC

I'd prefer if in the showAnnotations case you pass null as the callback, that way we save a function for nothing.

Comment 2 oliver.sander 2017-09-02 18:35:35 UTC

That makes sense.  However, I've been wondering: if we touch the interface for this, shouldn't we make it more future-proof?  Maybe somebody will want more fine-grained control in the future.  We could add more options, but we could also simply have the call-back as a new parameter for renderToImage and renderToPainter.  What do you think?

Comment 3 Albert Astals Cid 2017-09-02 20:20:06 UTC

Before adding such a weird API, i'd like to listen why anyone would need it :D

Comment 4 oliver.sander 2017-09-03 04:40:53 UTC

I don't personally need more than the HideAnnotations flag.  But I could imagine that somebody would want to selectively hide certain annotation types, for example.

Comment 5 Albert Astals Cid 2017-09-03 19:55:08 UTC

Writing a library API without a use case usually ends up in broken API, let's not write something noone has asked to have :)

Comment 6 oliver.sander 2017-09-04 14:49:17 UTC

Alright.  New patch is attached.

Comment 7 oliver.sander 2017-09-04 14:49:39 UTC

Created attachment 133961 [details] [review]
Patch that allows to control whether annotations are shown

Comment 8 Albert Astals Cid 2017-09-04 19:52:21 UTC

Pushed

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.