Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Previously, PDFium only supported widget annotations to draw forms. As
we've implemented other annotations, the behavior of
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw changed. So, this CL
clearly defines what needs to be done in FPDF_RenderPageBitmap_Retail
and FPDF_FFLDraw.

This CL first assumes that PDFium users will always call
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw to render PDF pages,
because otherwise they are not able to support PDF forms.

FPDF_RenderPageBitmap_Retail should only deal with non-widget
annotations, such as highlight, underline, text, etc. If
FPDF_ANNOT flag is passed, non-widget annotations are drawn. Otherwise,
they are hidden.

FPDF_FFLDraw should only deal with annotations that requires
user-interaction, such as widget annotations and popup annotation. Since
popup annotation is associated with non-widget annotation, they should
not be drawn if the associated annotation is hidden. Thus, if FPDF_ANNOT
flag is passed, popup annotations are drawn. Otherwise, they are hidden.
Widget annotations should be always drawn regardless of FPDF_ANNOT flag
since they need to be always displayed for PDF forms.

Also, roll DEPS for testing/corpus to 8485b30.

BUG=pdfium:594
Committed: https://pdfium.googlesource.com/pdfium/+/75f84a56fed36111ece82d0ac96e87289622b093

[jaepark, 2016-09-09 02:37:40]

The CQ bit was checked by jaepark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 02:37:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jaepark, 2016-09-09 02:38:13]

Description was changed from

==========
Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Previously, PDFium only supported widget annotations to draw forms. As
we've implemented other annotations, the behavior of
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw changed. So, this CL
clearly defines what needs to be done in FPDF_RenderPageBitmap_Retail
and FPDF_FFLDraw.

This CL first assume that PDFium users will always call
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw to render PDF pages,
because otherwise they are not able to support PDF forms.

FPDF_RenderPageBitmap_Retail should only deal with non-widget
annotations, such as highlight, underline, text, etc. If
FPDF_ANNOT flag is passed, non-widget annotations are drawn. Otherwise,
they are hidden.

FPDF_FFLDraw should only deal with annotations that requires
user-interaction, such as widget annotations and popup annotation. Since
popup annotation is associated with non-widget annotation, they should
not be drawn if the associated annotation is hidden. Thus, if FPDF_ANNOT
flag is passed, popup annotations are drawn. Otherwise, they are hidden.
Widget annotations should be always drawn regardless of FPDF_ANNOT flag
since they need to be always displayed for PDF forms.

BUG=pdfium:594
==========

to

==========
Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Previously, PDFium only supported widget annotations to draw forms. As
we've implemented other annotations, the behavior of
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw changed. So, this CL
clearly defines what needs to be done in FPDF_RenderPageBitmap_Retail
and FPDF_FFLDraw.

This CL first assumes that PDFium users will always call
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw to render PDF pages,
because otherwise they are not able to support PDF forms.

FPDF_RenderPageBitmap_Retail should only deal with non-widget
annotations, such as highlight, underline, text, etc. If
FPDF_ANNOT flag is passed, non-widget annotations are drawn. Otherwise,
they are hidden.

FPDF_FFLDraw should only deal with annotations that requires
user-interaction, such as widget annotations and popup annotation. Since
popup annotation is associated with non-widget annotation, they should
not be drawn if the associated annotation is hidden. Thus, if FPDF_ANNOT
flag is passed, popup annotations are drawn. Otherwise, they are hidden.
Widget annotations should be always drawn regardless of FPDF_ANNOT flag
since they need to be always displayed for PDF forms.

BUG=pdfium:594
==========

[commit-bot: I haz the power, 2016-09-09 02:45:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 02:45:33]

Dry run: Try jobs failed on following builders:
  linux on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/linux/builds/2002)
  mac_xfa on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_xfa/builds/...)
  win on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win/builds/2007)

[jaepark, 2016-09-09 02:53:32]

jaepark@google.com changed reviewers:
+ dsinclair@chromium.org, thestig@chromium.org

[jaepark, 2016-09-09 02:53:32]

The dry run is failing for some corpus tests.
This is because there's a slight difference between drawing widget annotation
using FPDF_RenderPageBitmap, and FPDF_FFLDraw.

[jaepark, 2016-09-09 03:52:54]

On 2016/09/09 02:53:32, jaepark wrote:
> The dry run is failing for some corpus tests.
> This is because there's a slight difference between drawing widget annotation
> using FPDF_RenderPageBitmap, and FPDF_FFLDraw.

Actually, this is not true. Since all Widget annotations are drawn through
FPDF_FFLDraw in before and after this CL, the output should be the same. I'll
look into why there's corpus test failures.

[Lei Zhang, 2016-09-09 07:09:01]

https://codereview.chromium.org/2323203002/diff/1/core/fpdfapi/fpdf_render/fp...
File core/fpdfapi/fpdf_render/fpdf_render.cpp (right):

https://codereview.chromium.org/2323203002/diff/1/core/fpdfapi/fpdf_render/fp...
core/fpdfapi/fpdf_render/fpdf_render.cpp:106:
CPDF_RenderOptions::CPDF_RenderOptions(const CPDF_RenderOptions& rhs)
Can this just be:

CPDF_RenderOptions::CPDF_RenderOptions(const CPDF_RenderOptions& rhs) = default;

https://codereview.chromium.org/2323203002/diff/1/core/fpdfdoc/cpdf_annot.cpp
File core/fpdfdoc/cpdf_annot.cpp (right):

https://codereview.chromium.org/2323203002/diff/1/core/fpdfdoc/cpdf_annot.cpp...
core/fpdfdoc/cpdf_annot.cpp:325: if (IsAnnotationHidden(m_pAnnotDict))
The newly added code is the same as the code in DrawAppearance() above -
combine?

https://codereview.chromium.org/2323203002/diff/1/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/2323203002/diff/1/fpdfsdk/fpdfview.cpp#newcod...
fpdfsdk/fpdfview.cpp:899: bPrinting, &matrix, FALSE, nullptr);
Can you explain why this gets flipped?

[Lei Zhang, 2016-09-09 07:12:12]

We should also update the documentation in public/ for the functions in
questions.

Also, the corpus test is failing. Is that because of the popup changes?

[jaepark, 2016-09-09 18:03:23]

On 2016/09/09 07:12:12, Lei Zhang wrote:
> We should also update the documentation in public/ for the functions in
> questions.
> 
> Also, the corpus test is failing. Is that because of the popup changes?

The corpus test is failing because of FreeText annotation with given AP stream.

Before the change, all annotations are drawn through FPDF_FFLDraw. Now, only
widget annotations are drawn through FPDF_FFLDraw, and other annotations are
drawn through FPDF_RenderPageBitmap_Retail.

FPDF_FFLDraw eventually calls CPDF_Annot::DrawAppearance, and
FPDF_RenderPageBitmap_Retail eventually calls CPDF_Annot::DrawInContext.
So there seems to be very slight difference in font rendering, but I'm not 100%
sure which result is correct. It looks the same to the naked eye.

[jaepark, 2016-09-09 18:44:33]

I also updated the documentation in public/

https://codereview.chromium.org/2323203002/diff/1/core/fpdfapi/fpdf_render/fp...
File core/fpdfapi/fpdf_render/fpdf_render.cpp (right):

https://codereview.chromium.org/2323203002/diff/1/core/fpdfapi/fpdf_render/fp...
core/fpdfapi/fpdf_render/fpdf_render.cpp:106:
CPDF_RenderOptions::CPDF_RenderOptions(const CPDF_RenderOptions& rhs)
On 2016/09/09 07:09:01, Lei Zhang wrote:
> Can this just be:
> 
> CPDF_RenderOptions::CPDF_RenderOptions(const CPDF_RenderOptions& rhs) =
default;

I get "error: [chromium-style] Complex constructor has an inlined body." if I do
as suggested.

https://codereview.chromium.org/2323203002/diff/1/core/fpdfdoc/cpdf_annot.cpp
File core/fpdfdoc/cpdf_annot.cpp (right):

https://codereview.chromium.org/2323203002/diff/1/core/fpdfdoc/cpdf_annot.cpp...
core/fpdfdoc/cpdf_annot.cpp:325: if (IsAnnotationHidden(m_pAnnotDict))
On 2016/09/09 07:09:01, Lei Zhang wrote:
> The newly added code is the same as the code in DrawAppearance() above -
> combine?

Done.

https://codereview.chromium.org/2323203002/diff/1/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/2323203002/diff/1/fpdfsdk/fpdfview.cpp#newcod...
fpdfsdk/fpdfview.cpp:899: bPrinting, &matrix, FALSE, nullptr);
On 2016/09/09 07:09:01, Lei Zhang wrote:
> Can you explain why this gets flipped?

The name of this parameter is bShowWidget, which means whether to render widget
annotations. Since this path is taken by FPDF_RenderPageBitmap_Retail, we don't
want to render widget annotations. So I flipped it to FALSE.

[Lei Zhang, 2016-09-09 21:20:25]

lgtm

[Lei Zhang, 2016-09-09 21:28:33]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-09-09 21:28:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 21:33:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 21:33:52]

Try jobs failed on following builders:
  mac_xfa on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_xfa/builds/...)
  win on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win/builds/2012)
  win_no_v8 on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_no_v8/build...)

[Lei Zhang, 2016-09-09 21:57:28]

On 2016/09/09 18:03:23, jaepark wrote:
> On 2016/09/09 07:12:12, Lei Zhang wrote:
> > We should also update the documentation in public/ for the functions in
> > questions.
> > 
> > Also, the corpus test is failing. Is that because of the popup changes?
> 
> The corpus test is failing because of FreeText annotation with given AP
stream.
> 
> Before the change, all annotations are drawn through FPDF_FFLDraw. Now, only
> widget annotations are drawn through FPDF_FFLDraw, and other annotations are
> drawn through FPDF_RenderPageBitmap_Retail.
> 
> FPDF_FFLDraw eventually calls CPDF_Annot::DrawAppearance, and
> FPDF_RenderPageBitmap_Retail eventually calls CPDF_Annot::DrawInContext.
> So there seems to be very slight difference in font rendering, but I'm not
100%
> sure which result is correct. It looks the same to the naked eye.

It sounds like FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw draw text slightly
differently. Do you know what they are doing differently?

It is a very minor difference especially at low DPI. I'll take a look at all the
diffs from the corpus tests. Maybe we can do another round of corpus expectation
updates if the difference is barely noticable.

[Lei Zhang, 2016-09-09 22:05:46]

On 2016/09/09 21:57:28, Lei Zhang wrote:
> It is a very minor difference especially at low DPI. I'll take a look at all
the
> diffs from the corpus tests. Maybe we can do another round of corpus
expectation
> updates if the difference is barely noticable.

Let's update the corpus.

[jaepark, 2016-09-09 22:20:01]

On 2016/09/09 22:05:46, Lei Zhang wrote:
> On 2016/09/09 21:57:28, Lei Zhang wrote:
> > It is a very minor difference especially at low DPI. I'll take a look at all
> the
> > diffs from the corpus tests. Maybe we can do another round of corpus
> expectation
> > updates if the difference is barely noticable.
> 
> Let's update the corpus.

I checked that the matrix used for rendering free text annotations were the
same.
The only difference is that CPDF_Annot::DrawAppearance creates
CPDF_RenderContext inside the method, and CPDF_Annot::DrawInContext uses
CPDF_RenderContext passed as argument. Right now, I'm not 100% sure how this can
affect the rendering of text annotation.

I have updated the corpus in https://codereview.chromium.org/2325273002/ .

[jaepark, 2016-09-09 22:23:22]

Description was changed from

==========
Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Previously, PDFium only supported widget annotations to draw forms. As
we've implemented other annotations, the behavior of
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw changed. So, this CL
clearly defines what needs to be done in FPDF_RenderPageBitmap_Retail
and FPDF_FFLDraw.

This CL first assumes that PDFium users will always call
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw to render PDF pages,
because otherwise they are not able to support PDF forms.

FPDF_RenderPageBitmap_Retail should only deal with non-widget
annotations, such as highlight, underline, text, etc. If
FPDF_ANNOT flag is passed, non-widget annotations are drawn. Otherwise,
they are hidden.

FPDF_FFLDraw should only deal with annotations that requires
user-interaction, such as widget annotations and popup annotation. Since
popup annotation is associated with non-widget annotation, they should
not be drawn if the associated annotation is hidden. Thus, if FPDF_ANNOT
flag is passed, popup annotations are drawn. Otherwise, they are hidden.
Widget annotations should be always drawn regardless of FPDF_ANNOT flag
since they need to be always displayed for PDF forms.

BUG=pdfium:594
==========

to

==========
Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Previously, PDFium only supported widget annotations to draw forms. As
we've implemented other annotations, the behavior of
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw changed. So, this CL
clearly defines what needs to be done in FPDF_RenderPageBitmap_Retail
and FPDF_FFLDraw.

This CL first assumes that PDFium users will always call
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw to render PDF pages,
because otherwise they are not able to support PDF forms.

FPDF_RenderPageBitmap_Retail should only deal with non-widget
annotations, such as highlight, underline, text, etc. If
FPDF_ANNOT flag is passed, non-widget annotations are drawn. Otherwise,
they are hidden.

FPDF_FFLDraw should only deal with annotations that requires
user-interaction, such as widget annotations and popup annotation. Since
popup annotation is associated with non-widget annotation, they should
not be drawn if the associated annotation is hidden. Thus, if FPDF_ANNOT
flag is passed, popup annotations are drawn. Otherwise, they are hidden.
Widget annotations should be always drawn regardless of FPDF_ANNOT flag
since they need to be always displayed for PDF forms.

Also, roll DEPS for testing/corpus to 8485b30.

BUG=pdfium:594
==========

[jaepark, 2016-09-09 22:24:02]

The CQ bit was checked by jaepark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 22:24:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 22:36:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 22:37:00]

Dry run: This issue passed the CQ dry run.

[jaepark, 2016-09-09 22:38:41]

The CQ bit was checked by jaepark@google.com

[jaepark, 2016-09-09 22:38:42]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2323203002/#ps40001
(title: "roll DEPS")

[commit-bot: I haz the power, 2016-09-09 22:38:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 22:39:11]

Description was changed from

==========
Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Previously, PDFium only supported widget annotations to draw forms. As
we've implemented other annotations, the behavior of
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw changed. So, this CL
clearly defines what needs to be done in FPDF_RenderPageBitmap_Retail
and FPDF_FFLDraw.

This CL first assumes that PDFium users will always call
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw to render PDF pages,
because otherwise they are not able to support PDF forms.

FPDF_RenderPageBitmap_Retail should only deal with non-widget
annotations, such as highlight, underline, text, etc. If
FPDF_ANNOT flag is passed, non-widget annotations are drawn. Otherwise,
they are hidden.

FPDF_FFLDraw should only deal with annotations that requires
user-interaction, such as widget annotations and popup annotation. Since
popup annotation is associated with non-widget annotation, they should
not be drawn if the associated annotation is hidden. Thus, if FPDF_ANNOT
flag is passed, popup annotations are drawn. Otherwise, they are hidden.
Widget annotations should be always drawn regardless of FPDF_ANNOT flag
since they need to be always displayed for PDF forms.

Also, roll DEPS for testing/corpus to 8485b30.

BUG=pdfium:594
==========

to

==========
Define behaviors of FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw.

Previously, PDFium only supported widget annotations to draw forms. As
we've implemented other annotations, the behavior of
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw changed. So, this CL
clearly defines what needs to be done in FPDF_RenderPageBitmap_Retail
and FPDF_FFLDraw.

This CL first assumes that PDFium users will always call
FPDF_RenderPageBitmap_Retail and FPDF_FFLDraw to render PDF pages,
because otherwise they are not able to support PDF forms.

FPDF_RenderPageBitmap_Retail should only deal with non-widget
annotations, such as highlight, underline, text, etc. If
FPDF_ANNOT flag is passed, non-widget annotations are drawn. Otherwise,
they are hidden.

FPDF_FFLDraw should only deal with annotations that requires
user-interaction, such as widget annotations and popup annotation. Since
popup annotation is associated with non-widget annotation, they should
not be drawn if the associated annotation is hidden. Thus, if FPDF_ANNOT
flag is passed, popup annotations are drawn. Otherwise, they are hidden.
Widget annotations should be always drawn regardless of FPDF_ANNOT flag
since they need to be always displayed for PDF forms.

Also, roll DEPS for testing/corpus to 8485b30.

BUG=pdfium:594

Committed:
https://pdfium.googlesource.com/pdfium/+/75f84a56fed36111ece82d0ac96e87289622...
==========

[commit-bot: I haz the power, 2016-09-09 22:39:13]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/75f84a56fed36111ece82d0ac96e87289622...