open the test.html. drag the number of shadowBlur to left, until shadowBlur=5; you will see some text get truncated when the value of shadowBlur becomes very small. drag the numbers inside the translate() until the "ABCEDF" move to left top coner, the lines shown, but not fully shown. click the save() to disable it, the lines shown. some problems in savestate? you may drag the rotate number to see more.

looks like a azure cairo bug. works fine on skia.

Before tracking, it would be good to understand an estimate of user impact and when this first regressed.

(In reply to Alex Keybl [:akeybl] from comment #2) > Before tracking, it would be good to understand an estimate of user impact > and when this first regressed. Regression window(m-c) Good: http://hg.mozilla.org/mozilla-central/rev/3d9424eb6eb4 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120901193714 Bad: http://hg.mozilla.org/mozilla-central/rev/6c66c3997381 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120902185812 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3d9424eb6eb4&tochange=6c66c3997381 Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/476d122c82a6 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120902155712 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/6c66c3997381 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20120902160711 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=476d122c82a6&tochange=6c66c3997381 Regressed by: Bug 781731 And bug 802658 did not fix in some case.

This looks to affect any use of strokeText combined with shadowBlur. I, sadly, have no insight as to how common that is. Surprisingly, though, this bug shows up on both Cairo and Quartz. It could even be an underlying canvas bug that the Cairo fixes simply exposed.

it's a regression, and assigned, so will track and hope we've got a safe forward fix or potential backout that can return former functionality.

It could be related to 786113.

I think there are 2 bugs. 1. the state + the path 2. transform + shadow + strokeText

more clear demo.

(In reply to Joe Drew (:JOEDREW! \o/) from comment #4) > This looks to affect any use of strokeText combined with shadowBlur. I, > sadly, have no insight as to how common that is. > > Surprisingly, though, this bug shows up on both Cairo and Quartz. It could > even be an underlying canvas bug that the Cairo fixes simply exposed. Hi Joe - when do you expect to have the time to investigate and fix? Before 4 is going to build tomorrow, and the holidays are coming up.

I'll investigate this and try to at least give an ETA tomorrow.

Bas, for some reason you made us only think we're drawing a shadow when we're filling, which isn't the case. Is there a reason for that? This fixes the bug.

Try: https://tbpl.mozilla.org/?tree=Try&rev=c0402d000183

Comment on attachment 692465 [details] [diff] [review] detect when we're drawing a shadow correctly Review of attachment 692465 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this bool is only used for determining whether to only Redraw the bounding box of the surface as far as I can see. If the operation is a stroke we always invalidate the whole thing using a Redraw() anyway. As such we don't need the bounding box computation so we avoid wasting that time. Something weird's going on if this fixes it, see the surrounding code, I'd like to know what it is before we r+ this.

Comment on attachment 692466 [details] [diff] [review] test Review of attachment 692466 [details] [diff] [review]: ----------------------------------------------------------------- Test name should be stroketext-shadow I think.

(In reply to Bas Schouten (:bas.schouten) from comment #14) > Yes, this bool is only used for determining whether to only Redraw the > bounding box of the surface as far as I can see. If the operation is a > stroke we always invalidate the whole thing using a Redraw() anyway. As such > we don't need the bounding box computation so we avoid wasting that time. It's also used, when shadows are involved, to find the size of the temporary surface we're going to draw into so we can then generate a shadow from that surface. That's where the problem comes in.

Comment on attachment 692465 [details] [diff] [review] detect when we're drawing a shadow correctly Because Bas is on vacation, I'mma go looking for review elsewheres.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4e2a208826d https://hg.mozilla.org/integration/mozilla-inbound/rev/be49bceac581

(In reply to Joe Drew (:JOEDREW! \o/) from comment #18) > https://hg.mozilla.org/integration/mozilla-inbound/rev/f4e2a208826d > https://hg.mozilla.org/integration/mozilla-inbound/rev/be49bceac581 Joe, are we comfortable taking this on FF18 beta 5(going to build today) based on the baketime/testing it has had ? Can you please help understand if the fix is revolving around taking a perf hit to fix the issue ?

This is pretty safe. It restores a codepath that we had prior to the removal of the old canvas implementation. It should have no perf hit other than that needed to have correct behaviour.

Comment on attachment 692465 [details] [diff] [review] detect when we're drawing a shadow correctly [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 651858 User impact if declined: Incorrect shadowed stroked text in canvas in certain case Testing completed (on m-c, etc.): on m-c for a short time Risk to taking this patch (and alternatives if risky): Not too risky. String or UUID changes made by this patch: none

(In reply to Joe Drew (:JOEDREW! \o/) from comment #20) > This is pretty safe. It restores a codepath that we had prior to the removal > of the old canvas implementation. It should have no perf hit other than that > needed to have correct behaviour. Thanks for the explanation . I was indeed in favor of taking it to restore the right behavior vs perf hit (if it had caused any). Please provide any pointers you have on testing this to QA(juanb).

QA: please focus on text canvas demos; this won't affect anything but text. https://hg.mozilla.org/releases/mozilla-aurora/rev/9009b97e34c2 https://hg.mozilla.org/releases/mozilla-beta/rev/b4c741ca7b67

https://hg.mozilla.org/mozilla-central/rev/f4e2a208826d https://hg.mozilla.org/mozilla-central/rev/be49bceac581