Bug 78219 - Switch svg/dynamic-updates tests to repaint harness
Summary: Switch svg/dynamic-updates tests to repaint harness
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on: 78332 78422
Blocks: 77541
  Show dependency treegraph
 
Reported: 2012-02-09 03:50 PST by Nikolas Zimmermann
Modified: 2012-03-07 10:46 PST (History)
12 users (show)

See Also:


Attachments
Patch (535.90 KB, patch)
2012-02-09 07:55 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v2 (654.26 KB, patch)
2012-02-15 01:38 PST, Nikolas Zimmermann
morrita: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2012-02-09 03:50:01 PST
To finally fix bug 77541, we have to switch the last remaining folder to use the repaint.js harness: svg/dynamic-updates.
Lots of rebaselines needed, so I have to wait for the bots to catchup first from the last rebaseline marathon :-)
Comment 1 Nikolas Zimmermann 2012-02-09 07:55:37 PST
Created attachment 126305 [details]
Patch

Full patch excluding the actual png results, it's over 50mb, as all tests in that directory (300+) need a rebaseline.
Fortunately that's the final patch of the series, fixing all SVG tests.
Comment 2 WebKit Review Bot 2012-02-09 09:26:29 PST
Comment on attachment 126305 [details]
Patch

Attachment 126305 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11487329

New failing tests:
svg/dynamic-updates/SVGCursorElement-dom-y-attr.html
svg/dynamic-updates/SVGCircleElement-dom-cy-attr.html
compositing/direct-image-compositing.html
http/tests/inspector/inspect-element.html
svg/dynamic-updates/SVGCircleElement-svgdom-requiredFeatures.html
svg/dynamic-updates/SVGCircleElement-dom-requiredFeatures.html
svg/dynamic-updates/SVGClipPathElement-dom-clipPathUnits-attr.html
svg/dynamic-updates/SVGClipPathElement-css-transform-influences-hitTesting.html
svg/animations/animate-calcMode-spline-by.html
svg/dynamic-updates/SVGCursorElement-svgdom-x-prop.html
svg/dynamic-updates/SVGCircleElement-svgdom-cx-prop.html
svg/dynamic-updates/SVGCursorElement-dom-x-attr.html
svg/animations/animVal-basics.html
svg/dynamic-updates/SVGClipPathElement-transform-influences-hitTesting.html
svg/dynamic-updates/SVGCircleElement-dom-r-attr.html
svg/dynamic-updates/SVGCursorElement-svgdom-y-prop.html
svg/dynamic-updates/SVGCircleElement-dom-cx-attr.html
svg/dynamic-updates/SVGEllipseElement-dom-cy-attr.html
svg/dynamic-updates/SVGEllipseElement-dom-cx-attr.html
accessibility/aria-describedby-on-input.html
svg/dynamic-updates/SVGClipPath-influences-hitTesting.html
svg/dynamic-updates/SVGCircleElement-svgdom-r-prop.html
svg/dynamic-updates/SVGCircleElement-svgdom-cy-prop.html
svg/dynamic-updates/SVGClipPathElement-svgdom-clipPathUnits-prop.html
Comment 3 Nikolas Zimmermann 2012-02-09 11:40:13 PST
Hm, I tried to set svg/dynamic-updates = IMAGE in the expectations, doesn't that work for whole dirs?
Comment 4 Zoltan Herczeg 2012-02-10 00:17:51 PST
Comment on attachment 126305 [details]
Patch

Oh, big patch. I hope the scripts really cover everything. I hope it will work on all bots...

View in context: https://bugs.webkit.org/attachment.cgi?id=126305&action=review

> LayoutTests/svg/dynamic-updates/script-tests/SVGFEConvolveMatrixElement-dom-kernelUnitLength-attr.js:48
> -var successfullyParsed = true;
>  \ No newline at end of file
> +var successfullyParsed = true;

Why these lines are swapped?
Comment 5 Nikolas Zimmermann 2012-02-11 13:49:57 PST
(In reply to comment #4)
> (From update of attachment 126305 [details])
> Oh, big patch. I hope the scripts really cover everything. I hope it will work on all bots...
The svg/animation test cases will stop working with this patch, they depend on the scripts in dynamic-updates. I've fixed it all locally, and will split this up into smaller chunks now.

> Why these lines are swapped?
No idea.
Comment 6 Nikolas Zimmermann 2012-02-15 01:38:28 PST
The svg/animations patch is in, so I'm uploading a new version, with fixed chromium expectations.
I didn't include the pixel test changes, as this is 51mb total :(

No worries, the patch is mostly mechanics, so it's rubber-stampable. I explained the scripts I used in the ChangeLog.
Comment 7 Nikolas Zimmermann 2012-02-15 01:38:54 PST
Created attachment 127134 [details]
Patch v2
Comment 8 Nikolas Zimmermann 2012-02-15 02:50:40 PST
Committed r107797: <http://trac.webkit.org/changeset/107797>
Comment 9 Nikolas Zimmermann 2012-02-15 02:52:40 PST
There are three tests, that should have text changes, that I'm going to take care of for Gtk/Qt etc.
Ports that run pixel tests, need to update all svg/dynamic-updates baselines.
Comment 10 WebKit Review Bot 2012-02-15 04:05:00 PST
Attachment 127134 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
Index mismatch: 0dfd183742a71cb5de5dadc3ae177fc72b63a194 != 9cdcda984def14b8bf8a32b6da6784c8a6ef7b3a
rereading 8567f8d3c2539a28a496edaf1048483e973975c2
	M	LayoutTests/fast/forms/radio-nested-labels.html
	M	LayoutTests/ChangeLog
107798 = 3671b2d23de7ade4cb1d1e78a3f6f7673db6a6c9 already exists! Why are we refetching it?
 at /usr/lib/git-core/git-svn line 5210

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Csaba Osztrogonác 2012-02-15 04:59:52 PST
Reopen, because it made tests assert on Qt in debug mode:

ASSERTION FAILED: m_transparencyCount > 0
../../../../Source/WebCore/platform/graphics/GraphicsContext.cpp(346) : void WebCore::GraphicsContext::endTransparencyLayer()

http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Debug/r107797%20%2821142%29/results.html
Comment 12 Nikolas Zimmermann 2012-02-15 05:29:35 PST
(In reply to comment #11)
> ASSERTION FAILED: m_transparencyCount > 0

Hmpf, the problem is the m_transparencyCount generalization. Qt maintains a different count "layerCount" returned by isInTransparentLayer(), and it calls endTransparencyLayer too often, compared to all other ports - to support image clipping.

Cairo has similar needs (mask image operation during restore()) but doesn't suffer from the problem - can this be reused instead? I hope Zoltan can have a look.
Comment 13 noel gordon 2012-02-16 15:29:05 PST
These results look a bit wacky to me.  What's with all the "gray areas", is that a progression?

svg/dynamic-updates/SVGFEImageElement-dom-preserveAspectRatio-attr.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=SVGFEImageElement-dom-preserveAspectRatio-attr.html&showExpectations=true

svg/dynamic-updates/SVGFEImageElement-svgdom-preserveAspectRatio-prop.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg%2Fdynamic-updates%2FSVGFEImageElement-svgdom-preserveAspectRatio-prop.html&showExpectations=true
Comment 14 Ojan Vafai 2012-02-16 15:52:49 PST
(In reply to comment #3)
> Hm, I tried to set svg/dynamic-updates = IMAGE in the expectations, doesn't that work for whole dirs?

For the record, this should work. Not sure why it didn't above. I don't see how http://queues.webkit.org/results/11487329 matches the list of failing tests. Is that for a different run maybe. A bug in the commit queue? It's hard to diagnose what went wrong without seeing the actual output from that run.
Comment 15 Nikolas Zimmermann 2012-02-17 00:04:42 PST
(In reply to comment #13)
> These results look a bit wacky to me.  What's with all the "gray areas", is that a progression?
It's intended, we're forcing the paint and start tracking repaint rects on the WebView. After the initial painting, it will paint a half-opaque gray rect over the view, and then each following repaint, is highlighted with a white area.

> svg/dynamic-updates/SVGFEImageElement-dom-preserveAspectRatio-attr.html 
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=SVGFEImageElement-dom-preserveAspectRatio-attr.html&showExpectations=true
Looks just fine on first sight - it's like the mac expected.png, no?
Comment 16 noel gordon 2012-02-17 00:37:38 PST
gray it shall be then, I will rebaseline these two tests on bug 78454
 svg/dynamic-updates/SVGFEImageElement-svgdom-preserveAspectRatio-prop.html
 svg/dynamic-updates/SVGFEImageElement-dom-preserveAspectRatio-attr.html
Comment 17 noel gordon 2012-02-17 02:01:54 PST
(In reply to comment #16)
> gray it shall be then, I will rebaseline these two tests on bug 78454
>  svg/dynamic-updates/SVGFEImageElement-svgdom-preserveAspectRatio-prop.html
>  svg/dynamic-updates/SVGFEImageElement-dom-preserveAspectRatio-attr.html

http://trac.webkit.org/changeset/108052
Comment 18 Nikolas Zimmermann 2012-02-17 03:14:24 PST
(In reply to comment #17)
> http://trac.webkit.org/changeset/108052
Thanks!

(In reply to comment #11)
> Reopen, because it made tests assert on Qt in debug mode:
> ASSERTION FAILED: m_transparencyCount > 0
Closing this bug again, it's not related to my cleanup - but a general problem with Qt & transparency layers - induced by bug 65643. Tracking the Qt fix happens in bug 78332 now.
Comment 19 Adam Klein 2012-02-29 14:09:33 PST
There's still a huge section of "needs rebaseline" expectations in Chromium. Do these still just need rebaselining?  Here's a snippet:


3806: // Needs a rebaseline, as svg/dynamic-updates tests are now using the repaint.js harness.
3807: BUGWK78219 : svg/dynamic-updates/SVGAElement-dom-target-attr.html = IMAGE
3808: BUGWK78219 : svg/dynamic-updates/SVGAElement-svgdom-href-prop.html = IMAGE
3809: BUGWK78219 : svg/dynamic-updates/SVGAElement-svgdom-target-prop.html = IMAGE
3810: BUGWK78219 : svg/dynamic-updates/SVGCircleElement-dom-cx-attr.html = IMAGE
3811: BUGWK78219 : svg/dynamic-updates/SVGCircleElement-dom-cy-attr.html = IMAGE
3812: BUGWK78219 : svg/dynamic-updates/SVGCircleElement-dom-r-attr.html = IMAGE
3813: BUGWK78219 : svg/dynamic-updates/SVGCircleElement-dom-requiredFeatures.html = IMAGE
Comment 20 Stephen Chenney 2012-03-07 08:39:00 PST
svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetX-attr.html
svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetY-attr.html
svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetX-prop.html
svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetY-prop.html

These tests are showing that the filter result has moved when comparing the former expectations and the new expectations. This may be an issue with the test or may be an issue with the underlying code, as the difference in image position seems to be 2 pixels, which is the same amount that the animation moves the target.

See, for example, http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6/results/layout-test-results/svg/dynamic-updates/ for the old and new expectations.
Comment 21 Stephen Chenney 2012-03-07 09:16:00 PST
The turbulence filter content has also changed, but only for these two tests. I find this unexpected, because any change I would expect to see in every image, not just these.

svg/dynamic-updates/SVGFETurbulenceElement-dom-baseFrequency-attr.html = IMAGE
svg/dynamic-updates/SVGFETurbulenceElement-svgdom-baseFrequency-prop.html = IMAGE
Comment 22 Stephen Chenney 2012-03-07 09:31:27 PST
The filter changes are Chromium only, or at least not Mac. So I'm closing this after opening https://bugs.webkit.org/show_bug.cgi?id=80517
Comment 23 Stephen Chenney 2012-03-07 10:46:54 PST
Committed r110069: <http://trac.webkit.org/changeset/110069>