Canvas rotate & fillText: SkFixed.h: failed assertion "n64 == n32"

RESOLVED FIXED in Firefox 43

Status

()

Core
Canvas: 2D
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Jesse Ruderman, Assigned: lsalzman)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla43
All
Unspecified
assertion, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox42 affected, firefox43 fixed)

Details

Attachments

(4 attachments)

(Reporter)

Description

3 years ago
Created attachment 8642838 [details]
testcase

gfx/skia/skia/include/core/SkFixed.h:46: failed assertion "n64 == n32"
(Reporter)

Comment 1

3 years ago
Created attachment 8642857 [details]
stack
(Assignee)

Updated

3 years ago
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Hardware: Unspecified → All
(Assignee)

Comment 2

3 years ago
Created attachment 8644073 [details] [diff] [review]
part 1 - ensure that canvas 2d matrix transforms are finite

The webidl for the Canvas functions currently handles if explicit Infs or NaNs are passed in to the transform functions, but it does not handle the case where a large double number becomes an Inf through truncation to float.

Further, in certain cases like the rotate function, the Inf angles passed in to trig functions give out NaNs.

Skia canvas' Mac scaled font backend and also apparently the cross-platform Cairo scaled font backend both can't handle any non-finite numbers in the font transform, otherwise they can trigger assertions like above or various downwind failures like null-derefs (for the Cairo backend).

To mitigate this, this patch tries to verify that the resulting draw target transform has all finite values, especially after it has been composed with the existing transform. This weed out both cases where truncation directly produces non-finite values and cases where valid inputs through repeated composition may still yield a non-finite transform.
Attachment #8644073 - Flags: review?(bas)
(Assignee)

Comment 3

3 years ago
Created attachment 8644074 [details] [diff] [review]
part 2 - add crashtest for canvas 2d bug 1190705
Attachment #8644074 - Flags: review?(bas)
Attachment #8644073 - Flags: review?(bas) → review+
Comment on attachment 8644073 [details] [diff] [review]
part 1 - ensure that canvas 2d matrix transforms are finite

Jeff, I feel okay with this, do you agree?
Attachment #8644073 - Flags: review?(jmuizelaar)
Attachment #8644074 - Flags: review?(bas) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
How do other browsers handle these cases?
Keywords: checkin-needed
(Assignee)

Comment 7

3 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> How do other browsers handle these cases?

Chrome ignores it in this case.
(In reply to Lee Salzman [:eihrul] from comment #7)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> > How do other browsers handle these cases?
> 
> Chrome ignores it in this case.

So this makes us match them?
(Assignee)

Comment 9

3 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> (In reply to Lee Salzman [:eihrul] from comment #7)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> > > How do other browsers handle these cases?
> > 
> > Chrome ignores it in this case.
> 
> So this makes us match them?

More or less, the transform seems to not take effect for them. In any event, this is much better than bombing out.
(Assignee)

Comment 10

3 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> (In reply to Lee Salzman [:eihrul] from comment #7)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> > > How do other browsers handle these cases?
> > 
> > Chrome ignores it in this case.
> 
> So this makes us match them?

Also verified against Internet Explorer. This patch would make us match both Chrome and IE (and not crash).
Attachment #8644073 - Flags: review?(jmuizelaar) → review+
It would be nice to add a test for case this to the web-platform-tests
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/97de68014b7a
https://hg.mozilla.org/mozilla-central/rev/da1c20faa1d3
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.