PDF rendering regression, areas with content being rendered blank

VERIFIED FIXED in Firefox 50

Status

()

defect
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: ke5trel, Assigned: nical)

Tracking

({regression})

50 Branch
Firefox 51
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 unaffected, firefox49 unaffected, firefox50 verified, firefox51 verified)

Details

(Whiteboard: [regression])

Attachments

(3 attachments)

Reporter

Updated

3 years ago
Whiteboard: [regression]
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?
Blocks: 1167235
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(nical.bugzilla)
Keywords: regression
Assignee

Comment 2

3 years ago
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.
Flags: needinfo?(nical.bugzilla)
Assignee

Comment 3

3 years ago
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.
Assignee

Comment 4

3 years ago
This fixes the issue.
Assignee: nobody → nical.bugzilla
Attachment #8778856 - Flags: review?(bas)
Comment on attachment 8778856 [details] [diff] [review]
Record transforms in the style stack.

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

This is gross :(
Attachment #8778856 - Flags: review?(bas) → review+

Comment 6

3 years ago
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5584f816e28
Record/replay clips and transforms properly in CanvasRenderingContext2D. r=Bas

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a5584f816e28
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
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
QA Whiteboard: [bugday-20160810]
Will this be uplifted?
Flags: needinfo?(nical.bugzilla)
Assignee

Comment 10

3 years ago
(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.
Flags: needinfo?(nical.bugzilla)
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]
(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?
Flags: needinfo?(nical.bugzilla)
Assignee

Comment 13

3 years ago
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]:
Flags: needinfo?(nical.bugzilla)
Attachment #8778856 - Flags: approval-mozilla-aurora?
Hello Kestrel, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(kestrel)
Comment on attachment 8778856 [details] [diff] [review]
Record transforms in the style stack.

new regression in 50, Aurora50+
Attachment #8778856 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
Flags: needinfo?(jmaher)
Needs rebasing for Aurora.
Flags: needinfo?(nical.bugzilla)
Reporter

Comment 18

3 years ago
Verified as fixed for Nightly 51.0a1 (2016-09-01) on Ubuntu 16.04.
Status: RESOLVED → VERIFIED
Flags: needinfo?(kestrel)
oh this is great to know ahead of time!
Flags: needinfo?(jmaher)
Assignee

Comment 20

3 years ago
Flags: needinfo?(nical.bugzilla)
Hi Ryan, there's a rebased patch in comment 20.
Flags: needinfo?(ryanvm)
Flags: needinfo?(ryanvm)
Flags: qe-verify+

Comment 23

3 years ago
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.
You need to log in before you can comment on or make changes to this bug.