Closed Bug 1018527 Opened 6 years ago Closed 4 years ago

[Skia canvas] failed assertion "rect.fLeft <= rect.fRight && rect.fTop <= rect.fBottom" in SkPathRef::setBounds

Categories

(Core :: Canvas: 2D, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: jruderman, Assigned: pchang)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(4 files, 2 obsolete files)

Attached file testcase
With:
  user_pref("gfx.canvas.azure.backends", "skia");

gfx/skia/trunk/include/core/SkPathRef.h:300: failed assertion "rect.fLeft <= rect.fRight && rect.fTop <= rect.fBottom"
Attached file stack
The pref mentioned in comment 0 is now the default, and the testcase still trips the assertion.
Assignee: nobody → howareyou322
Current the clearRect in CanvasRenderingContext2D is using double precision as input for x/y/width/height, but our 2D API(clearRect) is using the float precision. Therefore, it causes this bug when skia tries to calculate the left/right/top/bottom of the rect.

In the attached test case, the x is -infinite and the width is infinite. Therefore, it returns top(x+width) as a NaN.  
ctx.clearRect(-1.7976931348623157e+308, 0.5405572262919874, 1.7976931348623157e+308, 8);

Maybe we need to support the RectDouble in our 2D API. Bas, how do you think?
Flags: needinfo?(bas)
In my wip, it doesn't have this assertion when I pass the rect with double precision to DrawTargetSkia.
Another option is to skip the API calls when the value of input are out of range(in float precision) and the changed scope is smaller. I will work on this approach, so clear previous needinfo.
Flags: needinfo?(bas)
(In reply to peter chang[:pchang][:peter] from comment #5)
> Another option is to skip the API calls when the value of input are out of
> range(in float precision) and the changed scope is smaller. I will work on
> this approach, so clear previous needinfo.

Just ad a nore, I agree. I'd say a saner approach would be to cap those to values that fit in single precision floats, I don't see value in supporting double precision float.
Attached wip, will add the test case soon
Attached patch bug-1018527-testing.patch (obsolete) — Splinter Review
Add the clearRect testing with input in double precision.
Without attachment 8691240 [details] [diff] [review], it will trigger assertion during mochitest testing.
Attachment #8691240 - Flags: review?(jmuizelaar)
Attachment #8696603 - Flags: review?(jmuizelaar)
Attachment #8691240 - Flags: review?(jmuizelaar) → review+
Attachment #8696603 - Flags: review?(jmuizelaar) → review+
Fix mochitest fail because original NormalizeRect uses pass by reference, make same change to ValidateRect.
Attachment #8691240 - Attachment is obsolete: true
Attachment #8696903 - Flags: review+
Add reviewer name
Attachment #8696603 - Attachment is obsolete: true
Attachment #8696904 - Flags: review+
Pass try in comment 11.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/23f42ea04dd1
https://hg.mozilla.org/mozilla-central/rev/65bc8bf4b432
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Duplicate of this bug: 1229973
Duplicate of this bug: 1245429
You need to log in before you can comment on or make changes to this bug.