Last Comment Bug 752341 - canvas fillText stopped supporting gradient fill
: canvas fillText stopped supporting gradient fill
Status: RESOLVED FIXED
[qa+]
: regression
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: 12 Branch
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
Depends on:
Blocks: 692879
  Show dependency treegraph
 
Reported: 2012-05-06 10:50 PDT by gmc
Modified: 2013-12-27 20:31 PST (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified
+
verified


Attachments
Avoid using CGContextSetTextMatrix (1.11 KB, patch)
2012-05-18 08:05 PDT, Jeff Muizelaar [:jrmuizel]
bgirard: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description gmc 2012-05-06 10:50:18 PDT
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

Loaded the following HTML:

<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-type" content="text/html;charset=UTF-8">
<title>bug demo</title>
</head>
<body>
<canvas id="mycanvas" width="300" height="300"></canvas>
<script type="text/javascript">
var canvas = document.getElementById('mycanvas');
var ctx = canvas.getContext('2d');
ctx.font = "bold 36px sans-serif";
var gradient = ctx.createLinearGradient(0, 0, 150, 100);
gradient.addColorStop(0, "rgb(255, 0, 0)");
gradient.addColorStop(1, "rgb(255, 255, 0)");
ctx.fillStyle = gradient;
ctx.fillText("Hello, world..", 10, 100);
</script>
</body>
</html>


Actual results:

Nothing was shown.


Expected results:

The text "Hello, world.." should have been shown with a gradient fill going from red in the top left to yellow in the bottom right.
Comment 1 Boris Zbarsky [:bz] 2012-05-14 10:04:45 PDT
Regression range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=34572943a3e4&tochange=f4049f65efc6

Turning off Azure canvas on Mac makes the bug go away, so this is a bug in either azure canvas or the coregraphics azure backend.
Comment 2 Alex Keybl [:akeybl] 2012-05-14 15:14:01 PDT
Sending over to JP to find an assignee on the gfx team. If for whatever reason this goes unfixed in FF13, we don't expect major fallout given the lack of dupes here.
Comment 3 Cameron Kaiser [:spectre] 2012-05-14 16:09:45 PDT
If this helps, we use different gradient and different fill code in TenFourFox's 10.4 version of CG Azure. On my internal build of 12, I get Hello, w and part of the o.

        for (unsigned int i = 0; i < aBuffer.mNumGlyphs; i++) {
                CGContextSaveGState(cg); // push the "non-clip" on the stack
                CGContextSetTextDrawingMode(cg, kCGTextClip);
                it[0] = aBuffer.mGlyphs[i].mIndex;
                CGContextShowGlyphsAtPoint(cg, 
                        aBuffer.mGlyphs[i].mPosition.x,
                        aBuffer.mGlyphs[i].mPosition.y,
                        it, 1);
                DrawGradient(cg, aPattern);
                CGContextRestoreGState(cg); // pop the clip off
        }
Comment 4 Jeff Muizelaar [:jrmuizel] 2012-05-14 23:37:42 PDT
It looks like we're hitting the problem mentioned in this commit:

http://cgit.freedesktop.org/cairo/commit/?id=9c0d761bfcdd28d52c83d74f46dd3c709ae0fa69

I'm not sure how best to fix it yet.
Comment 5 Jeff Muizelaar [:jrmuizel] 2012-05-18 08:05:08 PDT
Created attachment 625109 [details] [diff] [review]
Avoid using CGContextSetTextMatrix
Comment 6 Jeff Muizelaar [:jrmuizel] 2012-05-21 07:29:50 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/05f23d27c37a
Comment 7 Alex Keybl [:akeybl] 2012-05-21 15:58:37 PDT
(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/05f23d27c37a

If this patch is low risk, please nominate for aurora/beta approval prior to tomorrow's beta 5 go to build.
Comment 8 Jeff Muizelaar [:jrmuizel] 2012-05-21 16:05:38 PDT
Comment on attachment 625109 [details] [diff] [review]
Avoid using CGContextSetTextMatrix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 692879
User impact if declined: Gradient fills for text won't work on OS X
Testing completed (on m-c, etc.): limited (I haven't had a chance to test this much) It does pass the our reftests and mochitests so it's unlikely to break anything too badly.
Risk to taking this patch (and alternatives if risky): Canvas text could paint in the wrong place or not at all.
String or UUID changes made by this patch: None
Comment 9 Alex Keybl [:akeybl] 2012-05-21 16:42:27 PDT
(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> Risk to taking this patch (and alternatives if risky): Canvas text could
> paint in the wrong place or not at all.

Would you consider this as having a very low probability of regression? If not, we'd probably have this only land in FF14 and up given where we are in the cycle.
Comment 10 Jeff Muizelaar [:jrmuizel] 2012-05-21 17:40:38 PDT
(In reply to Alex Keybl [:akeybl] from comment #9)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> > Risk to taking this patch (and alternatives if risky): Canvas text could
> > paint in the wrong place or not at all.
> 
> Would you consider this as having a very low probability of regression? If
> not, we'd probably have this only land in FF14 and up given where we are in
> the cycle.

I can do some testing tomorrow that should give an idea if this is likely to cause a regression. I should have a pretty good feeling after that.
Comment 11 Jeff Muizelaar [:jrmuizel] 2012-05-22 05:33:25 PDT
(In reply to Jeff Muizelaar [:jrmuizel] from comment #10)
> (In reply to Alex Keybl [:akeybl] from comment #9)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> > > Risk to taking this patch (and alternatives if risky): Canvas text could
> > > paint in the wrong place or not at all.
> > 
> > Would you consider this as having a very low probability of regression? If
> > not, we'd probably have this only land in FF14 and up given where we are in
> > the cycle.
> 
> I can do some testing tomorrow that should give an idea if this is likely to
> cause a regression. I should have a pretty good feeling after that.

I did some testing and this appears to be quite safe.
Comment 12 Ed Morley [:emorley] 2012-05-22 06:48:02 PDT
https://hg.mozilla.org/mozilla-central/rev/05f23d27c37a
Comment 13 Alex Keybl [:akeybl] 2012-05-22 11:21:05 PDT
Comment on attachment 625109 [details] [diff] [review]
Avoid using CGContextSetTextMatrix

[Triage Comment]
Low risk fix for a regression in FF12, approved for Aurora 14 and Beta 13.
Comment 14 Benoit Girard (:BenWa) 2012-05-22 17:37:53 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/491fd5c11f42

Landed on beta because of deadline. Jeff can you take care of landing on aurora.
Comment 15 Benoit Girard (:BenWa) 2012-05-23 16:11:33 PDT
(In reply to Benoit Girard (:BenWa) from comment #14)
> https://hg.mozilla.org/releases/mozilla-beta/rev/491fd5c11f42
> 
> Landed on beta because of deadline. Jeff can you take care of landing on
> aurora.

I missed that the patch I imported didn't have an author field. It was written by Jeff, I did not review my own patch as the commit message says.
Comment 16 Jeff Muizelaar [:jrmuizel] 2012-05-24 14:30:11 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/17bdb597ba39
Comment 17 Virgil Dicu [:virgil] [QA] 2012-05-30 05:11:17 PDT
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0

Verified on Mac OS 10.6 with test case in comment 0 on F13 beta6. Could previously reproduce the issue in F12.
Comment 18 Vlad [QA] 2012-06-12 06:40:35 PDT
Verified on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:14.0) Gecko/20100101 Firefox/14.0 beta 6

The text appears as described as above when the html file is loaded in browser.
Setting the flag to Verified on Firefox 14.
Comment 19 Virgil Dicu [:virgil] [QA] 2012-07-18 07:22:56 PDT
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:15.0) Gecko/20100101 Firefox/15.0

Verified on Mac OS 10.6 with test case in comment 0 on F15 beta1.
Comment 20 Ken Tozier 2013-12-27 17:46:21 PST
12/27/2013 This bug appears to be back in FF 26 for Mac. I filled text with a gradient in Safari, Opera and Chrome, all worked. In Firefox, the first color of a gradient is used to fill text, not the full gradient.
Comment 21 Boris Zbarsky [:bz] 2013-12-27 20:31:34 PST
Ken, the testcase in this bug works fine for me in Firefox 26 on Mac.

Please file a new bug with your testcase that shows a problem and cc me on it?

Note You need to log in before you can comment on or make changes to this bug.