Closed Bug 634982 Opened 13 years ago Closed 11 years ago

[css3-images][Mac only] Gradient color stops ignored when at the same position and more than 2 total

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: lea, Assigned: roc)

References

()

Details

Attachments

(3 files, 2 obsolete files)

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
Also, if I add another color stop at the end, it works just like it should.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is essentially the same issue as bug 635222, right?
Depends on: 635222
No, it's not.
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
Attached file 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.
Unless I'm misunderstanding the description, I don't see this one on Linux (whereas I do see bug 635222 on Linux).
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.)
Summary: [css3-gradients] 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 position and more than 2 total
Component: Style System (CSS) → Graphics
QA Contact: style-system → thebes
And I do see it in on Mac, so I'm definitely looking at the right thing.
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.
It might be worth checking if this is fixed with the update in bug 562746 or upstream cairo.
Assignee: nobody → roc
I don't have Mac access right now.
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'...
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. :-(
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));
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.
(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.
> or anything below 100%, the rendering is correct.

I mean "the gray will show". The rendering is correct in both cases.
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.
(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 :)
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 :-).
Attached patch fix (obsolete) — Splinter Review
Attachment #530545 - Flags: review?(jmuizelaar)
Comment on attachment 530545 [details] [diff] [review]
fix

Needs more work to not break radial gradients
Attachment #530545 - Attachment is obsolete: true
Attachment #530545 - Flags: review?(jmuizelaar)
Attached patch fix (obsolete) — Splinter Review
This works for radial gradients too.
Attachment #531243 - Flags: review?(jmuizelaar)
It looks like the reftest for this is broken.
linear-gradient(left, white 0, orange 100%);
linear-gradient(left, 0 white, 100% orange);
(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.
(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%'
Attached patch updated patchSplinter Review
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.)
Attachment #531243 - Attachment is obsolete: true
Attachment #536204 - Flags: review?(jmuizelaar)
Attachment #531243 - Flags: review?(jmuizelaar)
Question: why does the gradient stop for orange end up close to 1.0 in the test case instead of at .5?
The orange should stop halfway across the page. Does it not?
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
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.
This is fixed now. You're welcome to commit the reftest.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #536204 - Flags: review?(jmuizelaar)
Resolution: FIXED → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: