1292870 - PDF rendering regression, areas with content being rendered blank

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Kestrel]

Attachment: Incomplete rendering comparison

Example PDF with incomplete rendering:

https://downloads.dell.com/manuals/all-products/esuprt_laptop/esuprt_alienware_laptops/alienware-m11x-r3_service%20manual_en-us.pdf

Comment 1

[Tooru Fujisawa [:arai]]

confirmed on linux and OSX.

here's regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=57cf7cae92f1b234b763240a33f030fd68fc2166&tochange=56715a33b016d05afc2541e19fd7e0c907258971

looks like yet another regression from bug 1167235.
nical, do you have any idea?

Comment 2

[Nicolas Silva [:nical]]

I am looking into this. Interestingly it reproduces both with and without the SharedBufferProvider, so not a regression from the big architectural change in bug 1167235 like all of the other regressions so far, although it could have been introduced by a related patch.

Comment 3

[Nicolas Silva [:nical]]

This bug is caused by the order in which we restore clips and transforms. Currently for each state in the style stack we restore the transform and then all clips associated with that state. Clips are affected by the transform and can be applied before and/or after the transform so we have to record restore clips and transforms in a way that preserve the ordering.
It would be so much easier if we could just swap the destination surface and preserve all of the drawing state encapsulated in the DrawTarget, instead of duplicating this information externally.

Comment 4

[Nicolas Silva [:nical]]

Attachment: Record transforms in the style stack.

This fixes the issue.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 8778856 [details] [diff] [review]
Record transforms in the style stack.

Review of attachment 8778856 [details] [diff] [review]:
-----------------------------------------------------------------

This is gross :(

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5584f816e28
Record/replay clips and transforms properly in CanvasRenderingContext2D. r=Bas

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a5584f816e28

Comment 8

[Mohammad Maruf Islam]

I have reproduced this bug with Firefox Nightly 51.0a1 (Build ID:20160806030806) on 
Windows 8.1, 64-bit.

Verified as fixed with Latest Firefox Nightly 51.0a1 (Build ID: 20160811030201)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0

Comment 9

[Brendan Dahl [:bdahl]]

Will this be uplifted?

Comment 10

[Nicolas Silva [:nical]]

(In reply to Brendan Dahl [:bdahl] from comment #9)
> Will this be uplifted?

Yes, but first I want to hold off for a few days while I investigate a talos regression that may have been caused by this patch.

Comment 11

[Farhadur Reja Fahim]

I have reproduced this bug with Nightly 51.0a1 (2016-08-06) on Ubuntu 14.04, 64 bit!

The bug's fix is now verified on latest Nightly 51.0a1.

Nightly 51.0a1:
Build ID 	20160812030200
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0

[testday-20160812]

Comment 12

[Jonas Jenwald [:Snuffleupagus]]

(In reply to Nicolas Silva [:nical] from comment #10)
> (In reply to Brendan Dahl [:bdahl] from comment #9)
> > Will this be uplifted?
> 
> Yes, but first I want to hold off for a few days while I investigate a talos
> regression that may have been caused by this patch.

Is it possible to uplift this now?

Comment 13

[Nicolas Silva [:nical]]

Comment on attachment 8778856 [details] [diff] [review]
Record transforms in the style stack.

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: canvas correctness issue that affects pdfjs badly
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: low, may cause with a perf regression, which is fixed by another patch that is upliftable with low risks as well.
[String/UUID change made/needed]:

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Kestrel, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8778856 [details] [diff] [review]
Record transforms in the style stack.

new regression in 50, Aurora50+

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Joel, in the uplift request (comment 13), Nical mentioned a potential perf impact. So this is just an FYI for when you guys do perf testing on 50 in a week or so.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for Aurora.

Comment 18

[Kestrel]

Verified as fixed for Nightly 51.0a1 (2016-09-01) on Ubuntu 16.04.

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

oh this is great to know ahead of time!

Comment 20

[Nicolas Silva [:nical]]

Attachment: Aurora version

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Ryan, there's a rebased patch in comment 20.

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9d78ac91dc38

Comment 23

[Carmen]

I managed to reproduce the issue using an old build of Firefox Nightly 50.0a1 (2016-07-09) on Ubuntu 16.04 LTS x64.

The fix is now verified on Firefox Beta 50.0b4 on the same platform.

Comment 24

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Thank you, Carmen!