Bug 75552 – rebaselining some canvas images

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

new baselines to eliminate these lines from test_expectations.txt :

BUG_REED MAC : fast/canvas/canvas-text-baseline.html = IMAGE
BUG_REED MAC : fast/canvas/quadraticCurveTo.xml = IMAGE
BUG_REED MAC : fast/canvas/canvas-lineWidth.html = TEXT
BUG_REED WIN LINUX GPU : fast/canvas/canvas-text-baseline.html = IMAGE
BUG_REED WIN LINUX GPU : fast/canvas/quadraticCurveTo.xml = IMAGE
BUG_REED WIN LINUX GPU : fast/canvas/canvas-lineWidth.html = TEXT

Created attachment 121116 [details]
Patch

As much as anything, I want to figure out if I have done this rebaseline correctly... so here are the steps I took to generate it.

GENERATE NEW BASELINES:
# add REBASELINE lines to LayoutTests/platform/chromium/test_expectations.txt
#  (see examples in http://trac.webkit.org/wiki/Rebaseline )
Tools/Scripts/rebaseline-chromium-webkit-tests --use-deprecated-script \
  &> ../rebaseline-out.txt

MAKE DIFFS VIEWABLE BY OTHERS:
TMPDIR=/tmp/tmpTW2LKg/rebaseline_html  # found in result comparison window above
NOW=$(date +%F-%H-%M-%S)
cp -a $TMPDIR ~/www/rebaselines/$NOW
sed -e "s|file://$TMPDIR|.|g" $TMPDIR/rebaseline.html \
  >~/www/rebaselines/$NOW/rebaseline.html
chmod a+rx ~/www/rebaselines/$NOW
chmod a+r ~/www/rebaselines/$NOW/*

VIEW DIFFS IN http://www.corp.google.com/~epoger/rebaselines/2012-01-04-08-53-11/rebaseline.html

CREATE CL AND UPLOAD FOR REVIEW:
# create a new bug at bugs.webkit.org
EMAIL_ADDRESS=$[e-mail] Tools/Scripts/webkit-patch upload

Mike- Please review the changes to the baseline images and see if they look reasonable.  http://www.corp.google.com/~epoger/rebaselines/2012-01-04-08-53-11/rebaseline.html will probably help quite a bit with this task.

Ryosuke- Please let me know if this CL looks reasonable, and if the process I used to create it (see above) looks right.  If it looks good, please r+.

[adding Tom in CC]

[Mike Reed]

the new images look good

(In reply to comment #3)
> Ryosuke- Please let me know if this CL looks reasonable, and if the process I used to create it (see above) looks right.  If it looks good, please r+.

P.S. Here's the output I got from rebaseline-chromium-webkit-tests .  Should I be disturbed by the errors and warnings?


This script is depreciated.  Please use use "webkit-patch rebaseline-expectations" instead.

If that command does not work properly, please file a bug and CC abarth. If you
really need to use the old script, run this command again with
--use-deprecated-script as the first argument.

Using /usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests
  and /usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium/test_expectations.txt

chromium-mac-lion: Rebaselining 3 tests:

ERROR: Cannot find platform key chromium-mac-lion in archive directory name dictionary
ERROR: No archive found.

chromium-mac-snowleopard: Rebaselining 3 tests:
  Using http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6/116319/layout-test-results.zip
  Adding platform/chromium-mac-snowleopard/fast/canvas/canvas-text-baseline-expected.png

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-mac-snowleopard/fast/canvas/canv..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-mac-snowleopard/fast/canvas/canvas-text-baseline-expected.png"

  Adding platform/chromium-mac-snowleopard/fast/canvas/quadraticCurveTo-expected.png

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-mac-snowleopard/fast/canvas/quad..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-mac-snowleopard/fast/canvas/quadraticCurveTo-expected.png"

  Adding platform/chromium-mac-snowleopard/fast/canvas/canvas-lineWidth-expected.txt

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-mac-snowleopard/fast/canvas/canv..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-mac-snowleopard/fast/canvas/canvas-lineWidth-expected.txt"


chromium-mac-leopard: Rebaselining 3 tests:
  Using http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_5/116316/layout-test-results.zip
  Skipping fast/canvas/canvas-text-baseline.html (matches platform/chromium-mac-snowleopard)
  Adding platform/chromium-mac-leopard/fast/canvas/quadraticCurveTo-expected.png

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-mac-leopard/fast/canvas/quadrati..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-mac-leopard/fast/canvas/quadraticCurveTo-expected.png"

  Skipping fast/canvas/canvas-lineWidth.html (matches platform/chromium-mac-snowleopard)

chromium-cg-mac-lion: Rebaselining 3 tests:

ERROR: Cannot find platform key chromium-cg-mac-lion in archive directory name dictionary
ERROR: No archive found.

chromium-cg-mac-snowleopard: Rebaselining 3 tests:
  Using http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6__CG_/116318/layout-test-results.zip

WARNING: No results in archive for fast/canvas/canvas-text-baseline.html
WARNING: No results in archive for fast/canvas/quadraticCurveTo.xml
WARNING: No results in archive for fast/canvas/canvas-lineWidth.html


chromium-cg-mac-leopard: Rebaselining 3 tests:
  Using http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_5__CG_/116317/layout-test-results.zip

WARNING: No results in archive for fast/canvas/canvas-text-baseline.html
WARNING: No results in archive for fast/canvas/quadraticCurveTo.xml
WARNING: No results in archive for fast/canvas/canvas-lineWidth.html


chromium-win-win7: No tests to rebaseline.
chromium-win-vista: No tests to rebaseline.
chromium-win-xp: No tests to rebaseline.
chromium-linux-x86_64: No tests to rebaseline.
chromium-linux-x86: No tests to rebaseline.

chromium-gpu-mac-snowleopard: Rebaselining 3 tests:
  Using http://build.chromium.org/f/chromium/layout_test_results/Webkit_Mac10_6_-_GPU/116319/layout-test-results.zip
  Adding platform/chromium-gpu-mac/fast/canvas/canvas-text-baseline-expected.png

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-gpu-mac/fast/canvas/canvas-text-..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-gpu-mac/fast/canvas/canvas-text-baseline-expected.png"

  Adding platform/chromium-gpu-mac/fast/canvas/quadraticCurveTo-expected.png

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-gpu-mac/fast/canvas/quadraticCur..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-gpu-mac/fast/canvas/quadraticCurveTo-expected.png"

  Adding platform/chromium-gpu-mac/fast/canvas/canvas-lineWidth-expected.txt

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-gpu-mac/fast/canvas/canvas-lineW..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-gpu-mac/fast/canvas/canvas-lineWidth-expected.txt"


chromium-gpu-win-win7: Rebaselining 3 tests:
  Using http://build.chromium.org/f/chromium/layout_test_results/Webkit_Win7_-_GPU/116318/layout-test-results.zip
  Updating platform/chromium-gpu-win/fast/canvas/canvas-text-baseline-expected.png
  Updating platform/chromium-gpu-win/fast/canvas/quadraticCurveTo-expected.png
  Adding platform/chromium-gpu-win/fast/canvas/canvas-lineWidth-expected.txt

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-gpu-win/fast/canvas/canvas-lineW..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-gpu-win/fast/canvas/canvas-lineWidth-expected.txt"


chromium-gpu-linux-x86_64: Rebaselining 3 tests:
  Using http://build.chromium.org/f/chromium/layout_test_results/Webkit_Linux_-_GPU/116319/layout-test-results.zip
  Updating platform/chromium-gpu-linux/fast/canvas/canvas-text-baseline-expected.png
  Updating platform/chromium-gpu-linux/fast/canvas/quadraticCurveTo-expected.png
  Adding platform/chromium-gpu-linux/fast/canvas/canvas-lineWidth-expected.txt

WARNING: Failed to run "[u'svn', u'cat', u'-r', u'BASE', u'LayoutTests/platform/chromium-gpu-linux/fast/canvas/canvas-lin..." exit_code: 1
WARNING:   No base file: "/usr/local/google/home/epoger/src/webkit/white/WebKit/LayoutTests/platform/chromium-gpu-linux/fast/canvas/canvas-lineWidth-expected.txt"



Rebaselining failed.

[Ryosuke Niwa]

Comment on attachment 121116 [details]
Patch

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

> LayoutTests/platform/chromium-gpu-win/fast/canvas/canvas-lineWidth-expected.txt:13
> +FAIL Pixel at 80387 should be 47 but was 48
> +FAIL Pixel at 97175 should be 47 but was 48

These are failing. Why is it okay to rebaseline these tests?

[Tom Hudson]

(In reply to comment #7)
> (From update of attachment 121116 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121116&action=review
> 
> > LayoutTests/platform/chromium-gpu-win/fast/canvas/canvas-lineWidth-expected.txt:13
> > +FAIL Pixel at 80387 should be 47 but was 48
> > +FAIL Pixel at 97175 should be 47 but was 48
> 
> These are failing. Why is it okay to rebaseline these tests?

Changes to the rendering implementation in Skia regularly change the least-significant-bit or few bits of the output. In general, the WebKit tests are way too finicky: it's useful to know when something changes, but it's only likely to be the kind of bug that should stop us if something changes *perceptibly*. We waste hours maintaining baselines / worrying about the right way to maintain baselines for changes like these.

We've got another huge raft of rebaselines of this nature likely to come through in the near term: supersampled antialiasing currently uses a relatively poor approximation for one term that we now have a performant exact solution for, but that's going to change alpha values by 5-10% in some cases.

[Ryosuke Niwa]

Comment on attachment 121116 [details]
Patch

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

>>> LayoutTests/platform/chromium-gpu-win/fast/canvas/canvas-lineWidth-expected.txt:13
>>> +FAIL Pixel at 97175 should be 47 but was 48
>> 
>> These are failing. Why is it okay to rebaseline these tests?
> 
> Changes to the rendering implementation in Skia regularly change the least-significant-bit or few bits of the output. In general, the WebKit tests are way too finicky: it's useful to know when something changes, but it's only likely to be the kind of bug that should stop us if something changes *perceptibly*. We waste hours maintaining baselines / worrying about the right way to maintain baselines for changes like these.
> 
> We've got another huge raft of rebaselines of this nature likely to come through in the near term: supersampled antialiasing currently uses a relatively poor approximation for one term that we now have a performant exact solution for, but that's going to change alpha values by 5-10% in some cases.

You haven't answered my question. Why is it okay to check in these "FAIL"? Is 48 an acceptable value here? If so, then we should modify the test so that it'll print PASS for 48 as well.

[Ryosuke Niwa]

Comment on attachment 121116 [details]
Patch

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

>>> LayoutTests/platform/chromium-gpu-win/fast/canvas/canvas-lineWidth-expected.txt:13
>>> +FAIL Pixel at 97175 should be 47 but was 48
>> 
>> These are failing. Why is it okay to rebaseline these tests?
> 
> Changes to the rendering implementation in Skia regularly change the least-significant-bit or few bits of the output. In general, the WebKit tests are way too finicky: it's useful to know when something changes, but it's only likely to be the kind of bug that should stop us if something changes *perceptibly*. We waste hours maintaining baselines / worrying about the right way to maintain baselines for changes like these.
> 
> We've got another huge raft of rebaselines of this nature likely to come through in the near term: supersampled antialiasing currently uses a relatively poor approximation for one term that we now have a performant exact solution for, but that's going to change alpha values by 5-10% in some cases.

You haven't answered my question. Why is it okay to check in these "FAIL"? Is 48 an acceptable value here? If so, then we should modify the test so that it'll print PASS for 48 as well.

(In reply to comment #10)
> You haven't answered my question. Why is it okay to check in these "FAIL"? Is 48 an acceptable value here? If so, then we should modify the test so that it'll print PASS for 48 as well.

So, from a procedural standpoint... I guess I should modify the test to return PASS first, and *then* take another stab at the rebaseline.  Right?

I'll assume so and get to work on it.  Let me know if that's wrong.

https://bugs.webkit.org/show_bug.cgi?id=75627 ('make canvas-lineWidth test pass even if pixel values vary a tiny bit') should make the text baseline better.

While we wait for that to be resolved... Ryosuke, do you have any thoughts on the errors and warnings I got from rebaseline-chromium-webkit-tests ?  Should I be disturbed, or ignore them?

[Ryosuke Niwa]

(In reply to comment #12)
> https://bugs.webkit.org/show_bug.cgi?id=75627 ('make canvas-lineWidth test pass even if pixel values vary a tiny bit') should make the text baseline better.
> 
> While we wait for that to be resolved... Ryosuke, do you have any thoughts on the errors and warnings I got from rebaseline-chromium-webkit-tests ?  Should I be disturbed, or ignore them?

I ignore those warnings and errors.

Created attachment 121710 [details]
Patch

(In reply to comment #14)
> Created an attachment (id=121710) [details]
> Patch

Uploaded new patch without the offending ("FAIL") text expectations.

Ryosuke, please review.  Diffs are at http://www.corp.google.com/~epoger/rebaselines/2012-01-09-12-09-06/rebaseline.html

[WebKit Review Bot]

Comment on attachment 121710 [details]
Patch

Clearing flags on attachment: 121710

Committed r104603: <http://trac.webkit.org/changeset/104603>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.