truncation of path and text with shadow

RESOLVED FIXED in Firefox 18

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: kcleung88, Assigned: joe)

Tracking

({regression})

18 Branch
mozilla20
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox18+ fixed, firefox19+ fixed, firefox20+ fixed, b2g18 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Reporter

Description

7 years ago
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.
Reporter

Updated

7 years ago
Attachment #685645 - Attachment mime type: text/plain → text/html

Updated

7 years ago
Blocks: 781731, 802658
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
looks like a azure cairo bug. works fine on skia.
Assignee

Updated

7 years ago
Assignee: nobody → joe
Before tracking, it would be good to understand an estimate of user impact and when this first regressed.

Updated

7 years ago
QA Contact: ioana.budnar

Comment 3

7 years ago
(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.

Updated

7 years ago
QA Contact: ioana.budnar
Assignee

Comment 4

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

Updated

7 years ago
Keywords: qawanted
Reporter

Comment 7

7 years ago
I think there are 2 bugs.

1. the state + the path
2. transform + shadow + strokeText
Reporter

Comment 8

7 years ago
Posted file test.html (obsolete) —
more clear demo.
Attachment #685645 - Attachment is obsolete: true
Reporter

Updated

7 years ago
Attachment #688265 - Attachment mime type: text/plain → text/html
(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.
Assignee

Comment 10

7 years ago
I'll investigate this and try to at least give an ETA tomorrow.
Assignee

Comment 11

7 years ago
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.
Attachment #692465 - Flags: review?(bas)
Assignee

Comment 12

7 years ago
Posted patch testSplinter Review
Attachment #688265 - Attachment is obsolete: true
Attachment #692466 - Flags: review?(bas)
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.
Attachment #692466 - Flags: review?(bas) → review+
Assignee

Comment 16

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

Comment 17

7 years ago
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.
Attachment #692465 - Flags: review?(bas) → review?(roc)
(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 ?
Assignee

Comment 20

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

Comment 21

7 years ago
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
Attachment #692465 - Flags: approval-mozilla-beta?
Attachment #692465 - Flags: approval-mozilla-aurora?
(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).
Attachment #692465 - Flags: approval-mozilla-beta?
Attachment #692465 - Flags: approval-mozilla-beta+
Attachment #692465 - Flags: approval-mozilla-aurora?
Attachment #692465 - Flags: approval-mozilla-aurora+
QA Contact: jbecerra
https://hg.mozilla.org/mozilla-central/rev/f4e2a208826d
https://hg.mozilla.org/mozilla-central/rev/be49bceac581
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.