1224761 - Gradient with two color-stops at 0% renders as if second color stop were at 100%

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:
 1. Load attached testcase.

ACTUAL RESULTS:
 Second box has a gradient going from yellow at the left to blue at the right.

EXPECTED RESULTS:
 Second box should be entirely yellow.


This box has "linear-gradient(to right, blue 0%, yellow 0%);", which means we should be at yellow by the time we hit the left edge of the div (0%) and there shouldn't be any color changing to the right of that.

The other boxes in this testcase show that we're consistent about this for other values (e.g. -5%, 30%, 100%, 150%), but not for 0%.  It's not shown in the testcase, but we also show EXPECTED RESULTS if I use "blue -1%, yellow 0%" (bumping down the blue percentage slightly).  We just get things wrong if both color-stops are at 0%.

Chrome 48 & latest Edge show EXPECTED RESULTS. Firefox Nightly 45 shows ACTUAL RESULTS.

Comment 1

[Daniel Holbert [:dholbert]]

(Firefox 42 shows ACTUAL RESULTS, too. So, not a regression, or not a recent one at least.)

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The place to start debugging this is probably nsCSSRendering::PaintGradient.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So I debugged what was going on in nsCSSRendering::PaintGradient, and everything there seemed reasonable, except for one interesting thing:  we hit the case that sets forceRepeatToCoverTiles to true.  We hit that only in the buggy case and not in the other 4 cases in dholbert's testcase.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Fix forceRepeatToCoverTiles optimization to not trigger when all stops at left edge of tile

I tested locally in the reftest harness that the reftest fails as
expected (the second box shows a gradient) without the patch, and passes
with the patch.

(The reftest test is dholbert's testcase from the bug, unmodified; I
wrote the reference.)

Comment 5

[Daniel Holbert [:dholbert]]

Thanks for fixing this, dbaron!

With this bug fixed, we should also be able to simplify the gradient expression in the following reference case to just "background: yellow" (which is what it should be equivalent to, anyway):
https://mxr.mozilla.org/mozilla-central/source/layout/reftests/webkit-gradient/webkit-gradient-linear-2-ref.html?mark=35-41&force=1#35

(Note the comment there pointing to this bug. The corresponding testcase has a gradient expression that approximately maps to the broken piece of this testcase, and the reference case does as well. Right now they each render as a smooth gradient, and once this is fixed, they'll both render as solid-yellow.  And at that point, there's no reason for us to use the complex gradient expression in the reference case anymore, & we can just simplify to "background:yellow".)

If you're up for making this reference-case tweak as part of this bug here, great -- otherwise, I can also do it separately.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> With this bug fixed, we should also be able to simplify the gradient
> expression in the following reference case to just "background: yellow"
> (which is what it should be equivalent to, anyway):
> https://mxr.mozilla.org/mozilla-central/source/layout/reftests/webkit-
> gradient/webkit-gradient-linear-2-ref.html?mark=35-41&force=1#35

Sure, I'll land that as patch by you with r=dbaron.

Comment 7

[Daniel Holbert [:dholbert]]

Ah, great - thanks!

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7661180c26a283fd9b3d02ca41d5d2295cdd0eba
Bug 1224761 - Fix forceRepeatToCoverTiles optimization to not trigger when all stops at left edge of tile.  r=roc

https://hg.mozilla.org/integration/mozilla-inbound/rev/d97bfd0955d1cf7ab61c2ab17e9b297dc6dd6772
Remove workaround for bug 1224761.  r=dbaron

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1425df0a968&group_state=expanded

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7661180c26a2
https://hg.mozilla.org/mozilla-central/rev/d97bfd0955d1