Last Comment Bug 634982 - [css3-images][Mac only] Gradient color stops ignored when at the same position and more than 2 total
: [css3-images][Mac only] Gradient color stops ignored when at the same positio...
Status: RESOLVED WORKSFORME
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal with 4 votes (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
http://jsfiddle.net/leaverou/nJ4px/2/
: 651870 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-17 11:19 PST by Lea Verou
Modified: 2014-01-30 13:21 PST (History)
6 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
reporter's testcase (756 bytes, text/html)
2011-02-18 09:37 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details
fix (3.25 KB, patch)
2011-05-05 22:38 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
reftest (2.40 KB, patch)
2011-05-05 22:48 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
fix (9.31 KB, patch)
2011-05-09 21:50 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
updated patch (11.12 KB, patch)
2011-05-30 18:19 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review

Description Lea Verou 2011-02-17 11:19:05 PST
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre

See the testcase in Minefield and Webkit nightlies. Webkit gets this right, the area from 60% to 100% should be gray, but this only happens in Moz if the 2nd color stop gets a position of 59% instead 

Reproducible: Always
Comment 1 Lea Verou 2011-02-17 11:20:49 PST
Also, if I add another color stop at the end, it works just like it should.
Comment 2 Boris Zbarsky [:bz] 2011-02-18 09:00:43 PST
This is essentially the same issue as bug 635222, right?
Comment 3 Lea Verou 2011-02-18 09:01:56 PST
No, it's not.
Comment 4 Lea Verou 2011-02-18 09:04:59 PST
They 're probably related, but it's not the same bug. 

This is about any kind of gradients, the other about radial gradients.
This is about more than 2 color stops, the other can be reproduced with 2 as well.
This is about the last color stop being ignored, the other is about the first.
etc
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-02-18 09:37:07 PST
Created attachment 513496 [details]
reporter's testcase

The first place we do any manipulation of stop positions, as far as I can tell, is in http://hg.mozilla.org/mozilla-central/file/26c7b580d5a2/layout/base/nsCSSRendering.cpp#l1925 , and it doesn't look like it drops any stops, so I suspect the bug here is in cairo.
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-02-18 09:45:03 PST
Unless I'm misunderstanding the description, I don't see this one on Linux (whereas I do see bug 635222 on Linux).
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-02-18 09:47:34 PST
I don't see the problem on Windows either.  (On both Linux and Windows, I'm seeing the area to the right of 60% as gray.)
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-02-18 15:19:58 PST
And I do see it in on Mac, so I'm definitely looking at the right thing.
Comment 9 Lea Verou 2011-04-09 13:37:01 PDT
Still present on 4.2a1pre (09-04-2011).
It's really annoying when trying to do diagonal stripes (linear-gradient(-45deg, #222, #222 25%, #444 25%, #444 50%, #222 50%, #222 75%, #444 75%)), since you have to add a last color stop for Moz only.
Comment 10 Jeff Muizelaar [:jrmuizel] 2011-04-10 03:18:50 PDT
It might be worth checking if this is fixed with the update in bug 562746 or upstream cairo.
Comment 11 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-04-11 23:35:50 PDT
I don't have Mac access right now.
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-21 16:09:58 PDT
*** Bug 651870 has been marked as a duplicate of this bug. ***
Comment 13 Lea Verou 2011-04-29 08:03:34 PDT
By the way, IMO this bug is one of the worst CSS gradient bugs in the OSX platform. 

In my CSS gradients talk, I use Firefox for the slides and I run into it in most of the demos, so I have to explain to the audience how they need to add an extra color stop just because of this Gecko bug. 

Other people come to me and ask me what's the problem with their code and I have to explain to them that Gecko has this bug, all the time. I don't think it's very good for Firefox's reputation to have a bug that people run into so often. Just sayin'...
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 21:18:39 PDT
Here's what happens... First we normalize the gradient stops to a range 0.0 to 1.0. Then CoreGraphics asks us for the gradient values for a range of values including 0.97something, 0.986something, and 1.0. We return orange for 0.97something and 0.986something and return gray for 1.0. Coregraphics ignores the gray value and decides to paint orange all the way along past the end of the gradient line! The same is true even if I increase the domain of the gradient function to 1.001. :-(
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 21:43:11 PDT
This indeed appears to be a CoreGraphics bug, which we make show up more often due to our normalization of the gradient stop. I can reproduce the same bug in Safari using
  background: -webkit-gradient(linear, 0 0, 60% 0, color-stop(0.5, white), color-stop(1.0, orange), color-stop(1.0, gray));
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 21:45:05 PDT
Perhaps internally Quartz is hitting some kind of rounding error that causes the function value it wants to sample to be slightly less than 1.0, and then it uses a truncating division to find the array bucket containing its sampled function value.
Comment 17 Lea Verou 2011-05-05 21:49:49 PDT
(In reply to comment #15)
> This indeed appears to be a CoreGraphics bug, which we make show up more often
> due to our normalization of the gradient stop. I can reproduce the same bug in
> Safari using
>   background: -webkit-gradient(linear, 0 0, 60% 0, color-stop(0.5, white),
> color-stop(1.0, orange), color-stop(1.0, gray));

You're not reproducing any bug. That's the correct rendering, since the gray color stop is at 100% so it's not supposed to show gray anywhere. If you set them both at .6 for example, or anything below 100%, the rendering is correct.
Comment 18 Lea Verou 2011-05-05 21:51:55 PDT
> or anything below 100%, the rendering is correct.

I mean "the gray will show". The rendering is correct in both cases.
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 21:57:57 PDT
No, because I set the end of the gradient line to 60% of the width of the window. The area to the right of that should use the color of the last gradient stop, in theory. Of course -webkit-gradient isn't really defined anywhere so it's hard to be sure what it's supposed to do. Anyway, it's definitely a Quartz bug.
Comment 20 Lea Verou 2011-05-05 22:05:50 PDT
(In reply to comment #19)
> No, because I set the end of the gradient line to 60% of the width of the
> window. The area to the right of that should use the color of the last gradient
> stop, in theory. Of course -webkit-gradient isn't really defined anywhere so
> it's hard to be sure what it's supposed to do. Anyway, it's definitely a Quartz
> bug.

I see. You could try it with -webkit-linear-gradient, -o-linear-gradient or -ms-linear-gradient, since those are defined :)
Comment 21 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 22:16:12 PDT
Yeah, I just don't have Safari running with ToT Webkit, so I can't try -webkit-linear-gradient, and Chrome, Opera and IE don't use CoreGraphics :-).
Comment 22 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 22:38:17 PDT
Created attachment 530545 [details] [diff] [review]
fix
Comment 23 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 22:48:08 PDT
Created attachment 530546 [details] [diff] [review]
reftest
Comment 24 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-05 23:19:12 PDT
Comment on attachment 530545 [details] [diff] [review]
fix

Needs more work to not break radial gradients
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-09 21:50:31 PDT
Created attachment 531243 [details] [diff] [review]
fix

This works for radial gradients too.
Comment 26 Jeff Muizelaar [:jrmuizel] 2011-05-30 14:11:01 PDT
It looks like the reftest for this is broken.
linear-gradient(left, white 0, orange 100%);
linear-gradient(left, 0 white, 100% orange);
Comment 27 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-30 17:30:38 PDT
(In reply to comment #26)
> It looks like the reftest for this is broken.
> linear-gradient(left, white 0, orange 100%);
> linear-gradient(left, 0 white, 100% orange);

What do you mean? "orange 100%" doesn't occur in the patch.
Comment 28 Jeff Muizelaar [:jrmuizel] 2011-05-30 17:34:02 PDT
(In reply to comment #26)
> It looks like the reftest for this is broken.
> linear-gradient(left, white 0, orange 100%);
> linear-gradient(left, 0 white, 100% orange);

Sorry, I was really unclear.

the patch uses '100% orange', it should use 'orange 100%'
Comment 29 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-30 17:51:46 PDT
Sure, I'll fix that.
Comment 30 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-30 18:19:28 PDT
Created attachment 536204 [details] [diff] [review]
updated patch

Also marks some existing tests as failing. The differences are visually insignificant. It doesn't seem worth the effort to try to make them pass. (I imagine at some point we should switch to real Quartz gradients, which will obsolete all this code.)
Comment 31 Jeff Muizelaar [:jrmuizel] 2011-06-09 11:05:41 PDT
Question: why does the gradient stop for orange end up close to 1.0 in the test case instead of at .5?
Comment 32 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-09 15:16:41 PDT
The orange should stop halfway across the page. Does it not?
Comment 33 Jeff Muizelaar [:jrmuizel] 2011-06-09 15:33:51 PDT
The orange does stop halfway across the page. 

It seems like the gradient we're sending cairo assuming a 1000px wide window is:
(0, white), (1.0, orange), (1.0, white) from 0px to 500px

I'm wondering why we don't use:
(0, white), (0.5, orange), (0.5, white) (1.0, white) from 0px to 1000px
Comment 34 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-09 15:47:51 PDT
We normalize the stops to a 0 - 1.0 range, primarily to ensure that CSS stops at > 1.0 are handled correctly.

Setting up the stops as you suggest would require adding an extra stop in some cases, like this one, and I'd rather not do that just to work around a cairo bug.
Comment 35 Jeff Muizelaar [:jrmuizel] 2013-12-04 12:28:16 PST
This is fixed now. You're welcome to commit the reftest.
Comment 36 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2014-01-30 01:36:58 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cac96a0f55a
Comment 37 Ryan VanderMeulen [:RyanVM] 2014-01-30 13:21:10 PST
https://hg.mozilla.org/mozilla-central/rev/4cac96a0f55a

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