1276161 - HTML5 Canvas no longer renders dashed lines correctly in Firefox 46.0

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[jaun]

Attachment: Canvas_dashed_lines_problem.pdf

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/601.6.17 (KHTML, like Gecko) Version/9.1.1 Safari/601.6.17

Steps to reproduce:

I am the author of a Molecule Editor program written in JavaScript that uses HTML5 Canvas and is used by hundreds of students at our University. Users have reported problems after upgrading to Firefox 46.0 and I verified them. 


Actual results:

In Firefox 44.0 and erarlier, dashed lines as defined by setLineDash were rendered correctly, whatever their direction. With the upgrade to 46.0 dashed lines only appear dashed if they are exactly horizontal or vertical, but as full lines at any other angle.
This persisted for several tested line widths and different parameters in the dash array.


Expected results:

dashed lines as defined by context.setLineDash and drawn by context.beginPath() context.moveTo(), context.lineTo, context.stroke() should be correctly rendered on canvas in any direction, not just horizontal or vertical.

Comment 1

[Markus Stange [:mstange]]

Attachment: testcase with working dashed diagonal line

Comment 2

[Markus Stange [:mstange]]

I can't reproduce this bug with the information you provided. Could you try to find out more about the circumstances under which the bug occurs? The ideal result would be a standalone HTML file that demonstrates the bug.

Comment 3

[jaun]

I'm currently on a bike tour and don't have access to my computer. Will send you a standalone HTML file as soon as I'm back (9 June).

Comment 4

[jaun]

Attachment: HTML file demonstrating the problem (vanilla JavaScript)

In response to the request in Comment #2 from Markus Stange [:mstange] <[e-mail]> ---
..... The ideal
result would be a standalone HTML file that demonstrates the bug.

Comment 5

[jaun]

This problem seems to be Mac specific. I tried my demo file (https://bugzilla.mozilla.org/attachment.cgi?id=8761545) under Firefox 47 under windows 10 and the dashed lines appear correctly. With the Mac version of Firefox 47 under MacOS 10.11.5, the problem persists.
By the way: mstange's attachment (https://bugzilla.mozilla.org/attachment.cgi?id=8758032) does work under Mac OS 10.11.5 Firefox 47. However, I do not understand why this code works and mine does not - I can't find any relevant differences between them (the fact that my canvas has "style = background-color: transparent" makes no difference, I tried it without and the problem persists).

Comment 6

[Markus Stange [:mstange]]

Thank you very much!

I can reproduce the problem with your testcase in Firefox 47, but not on Nightly (which is at version 50 now), so it looks like we've fixed the problem in the meantime.

Lee, do you know what might have fixed this?

Comment 7

[Lee Salzman [:lsalzman]]

(In reply to Markus Stange [:mstange] from comment #6)
> Thank you very much!
> 
> I can reproduce the problem with your testcase in Firefox 47, but not on
> Nightly (which is at version 50 now), so it looks like we've fixed the
> problem in the meantime.
> 
> Lee, do you know what might have fixed this?

There have been quite substantial updates to Skia between 47 and nightly. Can you try running mozregression? That might be the easiest way to see what fixed it.

Comment 8

[Markus Stange [:mstange]]

I'll find the fix range.

Comment 9

[Markus Stange [:mstange]]

Regression range is https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=06ab805e14b5e6d8a8a99c7846c8fde163a9e5d7&tochange=35b211eaad1fa828064514c547057e4400e24459 , so this was probably caused by the Skia update in bug 1082598.

Fix range is https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8c3fd523d75bd30f691ca2d6cfdad18d576392a1&tochange=2b7c421063ad7e30b6491d62ed8480ca333b628a , so this was probably fixed by the Skia update in bug 1265131.

Comment 10

[Markus Stange [:mstange]]

We should add a test for this.

Comment 11

[Lee Salzman [:lsalzman]]

Attachment: backport fix for dashed paths with SkiaGL

This is just a backport of the following cherry-picked fix from the m51 Skia branch: https://skia.googlesource.com/skia/+/6266dca7a7a991634d144cbc3a2c6cffefd67454

It was trying to draw dashed paths as a batch type that did not support them, so this patch just makes it skip them as it should.

Comment 12

[Lee Salzman [:lsalzman]]

Comment on attachment 8761793 [details] [diff] [review]
backport fix for dashed paths with SkiaGL

Approval Request Comment
[Feature/regressing bug #]: bug 1082598 regressed this in 46 with a Skia update, fixed in 49 with bug 1265131 with a further Skia update, so affects 46-48
[User impact if declined]: Dashed lines/paths in canvas on Mac and Android will show up as solid lines
[Describe test coverage new/current, TreeHerder]: This particular odd case made it through our mochitest and reftests, although those should have cases for ensuring correctness of dashed lines under some scenarios
[Risks and why]: Basically none, we're already using this fix as of the last Skia update in 49, and this is just a backport of the fix
[String/UUID change made/needed]: None

Comment 13

[Markus Stange [:mstange]]

Attachment: test

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b56509fbed7
Reftests. r=lsalzman

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7b56509fbed7

Comment 16

[jaun]

Thank you all!
I'm impressed by the efficient way bugs are dealt with by Bugzilla.org and don't want to even think about how long it would take to get such a bug removed in an application of one of the big commercial vendors.

I tested my program against Nightly50.0a1 and everything worked as in Versions of Firefox ≤ 46.
Now, my students and I just have to wait for the official release of Firefox50+, then we are back to normal.

Comment 17

[Lee Salzman [:lsalzman]]

(In reply to jaun from comment #16)
> Thank you all!
> I'm impressed by the efficient way bugs are dealt with by Bugzilla.org and
> don't want to even think about how long it would take to get such a bug
> removed in an application of one of the big commercial vendors.
> 
> I tested my program against Nightly50.0a1 and everything worked as in
> Versions of Firefox ≤ 46.
> Now, my students and I just have to wait for the official release of
> Firefox50+, then we are back to normal.

We are trying to uplift the fix so that it can either be in a update of 47, or at the worst it should be in 48. It shouldn't require waiting till 50.

Comment 18

[Gerry Chang [:gchang]]

Comment on attachment 8761793 [details] [diff] [review]
backport fix for dashed paths with SkiaGL

This fixes a regression, take it in 48 beta 2. For release, it's Ritu's call.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/30e28d37807b

Comment 20

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

Given that this is not a new regression in 46, let's not add any additional risk to a dot release. I would like the fix to ride the 48 train to release.