Bug 38108 – Pixel test failure(s) related to recent SVG optimizations

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jeremy Orlow]

Nicholas, if this seems like a Chromium specific problem, feel free to re-assign.  But given that none of the webkit.org builders run pixel tests, I'm suspecting this is a real regression.

It looks as though http://trac.webkit.org/changeset/58212/ changed the image output of contain-and-cover.html: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fbackgrounds%2Fsize%2Fcontain-and-cover.html&showExpectations=true&useWebKitCanary=true

At this link, you can see all the tests that were failing after the patch landed: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&useWebKitCanary=true&tests=svg%2FW3C-SVG-1.1%2Fanimate-elem-31-t.svg%2Csvg%2FW3C-SVG-1.1%2Fanimate-elem-78-t.svg%2Csvg%2FW3C-SVG-1.1%2Fcolor-prop-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fcoords-units-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-color-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-comptran-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpainting-render-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-02-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-03-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-04-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-06-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-07-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-09-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-11-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-12-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-13-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-14-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-15-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-16-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-17-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-18-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-pattern-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fstruct-group-03-t.svg%2Csvg%2FW3C-SVG-1.1%2Fstruct-use-05-b.svg%2Csvg%2FW3C-SVG-1.1%2Fstyling-inherit-01-b.svg%2Csvg%2Fbatik%2Ffilters%2FfeTile.svg%2Csvg%2Fbatik%2Fmasking%2FmaskRegions.svg%2Csvg%2Fbatik%2Fpaints%2FgradientLimit.svg%2Csvg%2Fbatik%2Fpaints%2FpatternRegionA.svg%2Csvg%2Fbatik%2Fpaints%2FpatternRegions.svg%2Csvg%2Fbatik%2Ftext%2FtextEffect.svg%2Csvg%2Fbatik%2Ftext%2FtextEffect2.svg%2Csvg%2Fbatik%2Ftext%2FtextEffect3.svg%2Csvg%2Fbatik%2Ftext%2FtextProperties.svg%2Csvg%2Fcarto.net%2Ftabgroup.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Discrete.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Gamma.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Linear.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Table.svg%2Csvg%2Fcustom%2Ffill-fallback.svg%2Csvg%2Fcustom%2Fgradient-cycle-detection.svg%2Csvg%2Fcustom%2Fgradient-deep-referencing.svg%2Csvg%2Fcustom%2Finline-svg-in-xhtml.xml%2Csvg%2Fcustom%2Finvalid-css.svg%2Csvg%2Fcustom%2Fjs-late-gradient-and-object-creation.svg%2Csvg%2Fcustom%2Fjs-late-gradient-creation.svg%2Csvg%2Fcustom%2Fjs-late-pattern-and-object-creation.svg%2Csvg%2Fcustom%2Fjs-late-pattern-creation.svg%2Csvg%2Fcustom%2Fpattern-cycle-detection.svg%2Csvg%2Fcustom%2Fpattern-deep-referencing.svg%2Csvg%2Fcustom%2Fpattern-with-transformation.svg%2Csvg%2Fcustom%2Fstroke-fallback.svg%2Csvg%2Fcustom%2Fstroked-pattern.svg%2Csvg%2Fcustom%2Fuse-on-symbol-inside-pattern.svg%2Csvg%2Fhixie%2Ferror%2F003.xml%2Csvg%2Ftext%2Fselection-background-color.xhtml%2Csvg%2Ftext%2Fselection-styles.xhtml%2Csvg%2Ftext%2Ftext-gradient-positioning.svg%2Cfast%2Fbackgrounds%2Fsvg-as-background-2.html%2Csvg%2Fcustom%2Fgradient-stroke-width.svg%2Csvg%2Fcustom%2Fjs-update-gradient.svg%2Csvg%2FW3C-SVG-1.1%2Fmasking-mask-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-05-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-19-b.svg%2Csvg%2Fcustom%2Fdominant-baseline-hanging.svg

As you can see, besides the expected changes, it seems as though all the gradients (especially on mac) are a bit lighter.  I wonder if this is some sort of gamma related effect or something?

[Nikolas Zimmermann]

Just answering on the specific question regarding gradients being lighter on mac?
I don't see this problem - if you check my commit, you'll see that I did not change every -expected.png, just a few needed updates (invisible changes though).

I always run webkit tests with --pixel-tests and --tolerance 0, as this is the only true way to spot SVG regressions. So to summarize, for me gradients stayed the same, besides some marginal changes.

Will iook at the dashboard links soon. FYI some of the changes _may_ have influenced Skia, which contained a hack in the code I changed. I tried to preserve the hack, but couldn't test it on my own.

[Jeremy Orlow]

(In reply to comment #1)
> Just answering on the specific question regarding gradients being lighter on
> mac?
> I don't see this problem - if you check my commit, you'll see that I did not
> change every -expected.png, just a few needed updates (invisible changes
> though).
> 
> I always run webkit tests with --pixel-tests and --tolerance 0, as this is the
> only true way to spot SVG regressions. So to summarize, for me gradients stayed
> the same, besides some marginal changes.

Hm...interesting...Dimitri, thoughts?

> Will iook at the dashboard links soon. FYI some of the changes _may_ have
> influenced Skia, which contained a hack in the code I changed. I tried to
> preserve the hack, but couldn't test it on my own.

I believe all the issues are on Mac, and Mac uses CG (not Skia like Chromium Linux + Win).  So I'm guessing your hack-preservation worked.  (Thanks for doing that, btw!)

[Nikolas Zimmermann]

Oh good catch I guess, that file uses SVG + gradients, and I did not update the pixel tests.
I usually only run pixel tests with --tolerance 0 in the LayoutTests/svg directory. But nowadays SVG files are sprinkled all over LayoutTests/. I'll do a full run to with webkit trunk on mac, to see wheter I missed other files as well. Going to report...

[Nikolas Zimmermann]

I will update the Mac baseline for contain-and-cover.htm tomorrow, it's okay to rebaseline.
The svg-background-as-image2 tests etc, seem to be broken since a while! Need to run some bisect builds to track down, but it was definately broken before my patch. Just a quick FYI, to let you know I'm still on it.

[Jeremy Orlow]

(In reply to comment #4)
> I will update the Mac baseline for contain-and-cover.htm tomorrow, it's okay to
> rebaseline.
> The svg-background-as-image2 tests etc, seem to be broken since a while! Need
> to run some bisect builds to track down, but it was definately broken before my
> patch. Just a quick FYI, to let you know I'm still on it.

Thanks a lot Nikolas!

Note that svg-background-as-image2 did at least work on Chromium mac until we rolled in your revision though.  So I think at least something changed there.

Hopefully it won't be toooooo much longer before we can get Chromium pixel tests working upstream (which is blocked on getting a Chromium DRT implementation working unfortunately...but people are working on that).

[Nikolas Zimmermann]

(In reply to comment #5)
> (In reply to comment #4)
> > I will update the Mac baseline for contain-and-cover.htm tomorrow, it's okay to
> > rebaseline.
> > The svg-background-as-image2 tests etc, seem to be broken since a while! Need
> > to run some bisect builds to track down, but it was definately broken before my
> > patch. Just a quick FYI, to let you know I'm still on it.
> 
> Thanks a lot Nikolas!
> 
> Note that svg-background-as-image2 did at least work on Chromium mac until we
> rolled in your revision though.  So I think at least something changed there.
> 
> Hopefully it won't be toooooo much longer before we can get Chromium pixel
> tests working upstream (which is blocked on getting a Chromium DRT
> implementation working unfortunately...but people are working on that).

You were right that my last patch caused the regression. But only because we're not rendering gradients anymore, if they are contained in an unknown subtree. In our case, balloon.svg (referenced by svg-as-background-2.html) defined a <linearGradient> in a <def> section, instead of <defs>. That caused the problem, fixed by a simple s/def/defs. As a side effect, the background is not red anymore, but shows the real gradient, as it's supposed to be. This is because of the painting refactoring :-)

See patch on bug 38307. Hopefully this restored world peace again.

[Nikolas Zimmermann]

Patch landed, please update whether you still see problems with certain tests.

[Jeremy Orlow]

Looks like it's working!

Yaar, please confirm and remove the test expectation for svg-as-background-2 and rebaseline contain-and-cover

[Pavel Podivilov]

*** Bug 42168 has been marked as a duplicate of this bug. ***

[Pavel Podivilov]

*** Bug 42168 has been marked as a duplicate of this bug. ***