[api-minor] Re-factor the *internal* renderingIntent, and change the default intent value in the PDFPageProxy.getAnnotations method
#13867
Conversation
Snuffleupagus
force-pushed
the
RenderingIntentFlag
branch
from
b61c751 to
445d857
Compare
Snuffleupagus
force-pushed
the
RenderingIntentFlag
branch
6 times, most recently
from
bea737a to
b780270
Compare
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/efb9299a492e756/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/71a44485f1be365/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/71a44485f1be365/output.txt Total script time: 33.46 mins
Image differences available at: http://54.67.70.0:8877/71a44485f1be365/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/efb9299a492e756/output.txt Total script time: 39.21 mins
Image differences available at: http://3.101.106.178:8877/efb9299a492e756/reftest-analyzer.html#web=eq.log |
There was a problem hiding this comment.
I like the changes proposed here, but now that we're changing default API behavior it might be a good time to tag this as [api-major] instead and let the next release be version 3.0. This is not only consistent with semantic versioning, but also allows us to get rid of other code that we previously deprecated (in separate patches) since we're doing a major bump. What do you think?
…default `intent` value in the `PDFPageProxy.getAnnotations` method With the changes made in PR 13746 the *internal* renderingIntent handling became somewhat "messy", since we're now having to do string-matching in various spots in order to handle the "oplist"-intent correctly. Hence this patch, which implements the idea from PR 13746 to convert the `intent`-strings, used in various API-methods, into an *internal* renderingIntent that's implemented using a bit-field instead. *Please note:* This part of the patch, in itself, does *not* change the public API (but see below). This patch is tagged `api-minor` for the following reasons: 1. It changes the *default* value for the `intent` parameter, in the `PDFPageProxy.getAnnotations` method, to "display" in order to be consistent across the API. 2. In order to get *all* annotations, with the `PDFPageProxy.getAnnotations` method, you now need to explicitly set "any" as the `intent` parameter. 3. The `PDFPageProxy.getOperatorList` method will now also support the new "any" intent, to allow accessing the operatorList of all annotations (limited to those types that have one). 4. Finally, for consistency across the API, the `PDFPageProxy.render` method also support the new "any" intent (although I'm not sure how useful that'll be). Points 1 and 2 above are the significant, and thus breaking, changes in *default* behaviour here. However, unfortunately I cannot see a good way to improve the overall API while also keeping `PDFPageProxy.getAnnotations` unchanged.
…ng, since it's currently subtly wrong The value of the `renderInteractiveForms` parameter, as passed to the `PDFPageProxy.render` method, will (potentially) affect the size/content of the operatorList that's returned from the worker (for documents with forms). Given that operatorLists will generally, unless they contain huge images, be cached in the API, repeated `PDFPageProxy.render` calls that *only* change the `renderInteractiveForms` parameter can thus return an incorrect operatorList. As far as I can tell, this *subtle* bug has existed ever since `renderInteractiveForms`-support was first added in PR 7633 (which is almost five years ago). With the previous patch, fixing this is now really simple by "encoding" the `renderInteractiveForms` parameter in the *internal* renderingIntent handling.
Snuffleupagus
force-pushed
the
RenderingIntentFlag
branch
from
b780270 to
107efdb
Compare
Well, this wouldn't exactly be the first time that we've made backwards incompatible changes in The All-in-all, if we're thinking about increasing the major version then (in my opinion) it feels like there should be some substantial new feature(s) included and not just incremental changes. E.g. XFA-support might be one such thing, however that's still under very active development and I'd really like to not hold back the changes in this PR (since they might facilitate things such as issue 7282, assuming we want to fix that). |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/a627e1b19bd451a/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/a627e1b19bd451a/output.txt Total script time: 5.10 mins Published |
|
Thank you for doing this! I can't really argue with #13867 (comment) and the default value is now also what most users are interested in anyway, so let's do this. |
With the changes made in PR #13746 the internal renderingIntent handling became somewhat "messy", since we're now having to do string-matching in various spots in order to handle the "oplist"-intent correctly.
Hence this patch, which implements the idea from PR #13746 to convert the
intent-strings, used in various API-methods, into an internal renderingIntent that's implemented using a bit-field instead. Please note: This part of the patch, in itself, does not change the public API (but see below).This patch is tagged
api-minorfor the following reasons:intentparameter, in thePDFPageProxy.getAnnotationsmethod, to "display" in order to be consistent across the API.PDFPageProxy.getAnnotationsmethod, you now need to explicitly set "any" as theintentparameter.PDFPageProxy.getOperatorListmethod will now also support the new "any" intent, to allow accessing the operatorList of all annotations (limited to those types that have one).PDFPageProxy.rendermethod also support the new "any" intent (although I'm not sure how useful that'll be).Points 1 and 2 above are the significant, and thus breaking, changes in default behaviour here. However, unfortunately I cannot see a good way to improve the overall API while also keeping
PDFPageProxy.getAnnotationsunchanged.