Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2246863006:
Switch svg/clip-path/clip-path-multiple-children to pixel results (Closed)
|
Created:
4 years, 4 months ago by Stephen Chennney Modified:
4 years, 4 months ago Reviewers:
fs CC:
chromium-reviews, blink-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix reference for svg/clip-path/clip-path-multiple-children.svg
The path data for the reference result was slightly different to that of the test, resulting in anti-aliasing differences. Matching up the paths exactly makes the reference match the test.
R=fs@opera.com
BUG=425113, 636475
Committed: https://crrev.com/e7c705f5137839b87318d633aa7f66afb811ef42
Cr-Commit-Position: refs/heads/master@{#412839}
Patch Set 1 #Patch Set 2 : Fix the reference image for svg/clip-path/clip-path-multiple-children.svg #Patch Set 3 : Rebased #Patch Set 4 : Rebased #
Messages
Total messages: 27 (16 generated)
For context on the expected result, here's the flakiness board: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg... We'll have to wait on the try servers to get new results but locally they look fine.
The CQ bit was checked by schenney@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/08/17 at 16:22:32, schenney wrote: > For context on the expected result, here's the flakiness board: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg... > > We'll have to wait on the try servers to get new results but locally they look fine. Could the test be rewritten to use more "predictable" shapes instead? (I.e rectangles/straight lines)
On 2016/08/17 16:42:36, fs wrote: > On 2016/08/17 at 16:22:32, schenney wrote: > > For context on the expected result, here's the flakiness board: > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg... > > > > We'll have to wait on the try servers to get new results but locally they look > fine. > > Could the test be rewritten to use more "predictable" shapes instead? (I.e > rectangles/straight lines) Might work. I'll check.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by schenney@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Turns out the problem was that the reference paths, while in theory were the same, were not exactly the same. Adjusting floating point resolution and re-ordering the pts on the path seems to make it work as expected.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
On 2016/08/17 at 18:01:01, schenney wrote: > Turns out the problem was that the reference paths, while in theory were the same, were not exactly the same. Adjusting floating point resolution and re-ordering the pts on the path seems to make it work as expected. LGTM (Rietveld still has the -expected.svg as deleted though... odd)
On 2016/08/17 18:09:54, fs wrote: > On 2016/08/17 at 18:01:01, schenney wrote: > > Turns out the problem was that the reference paths, while in theory were the > same, were not exactly the same. Adjusting floating point resolution and > re-ordering the pts on the path seems to make it work as expected. > > LGTM (Rietveld still has the -expected.svg as deleted though... odd) That is odd. I'll make sure it's not an issue with my checkout. And a rebase is necessary. All these changes are constantly conflicting. At least we're getting our test coverage back again.
The CQ bit was checked by schenney@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com Link to the patchset: https://codereview.chromium.org/2246863006/#ps40001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Switch svg/clip-path/clip-path-multiple-children to pixel results The test basically passes expect for the presence of very minor anti-aliasing differences. Due to the semantics of clip paths we can't just update the ref. We need to go with pixel results. R=fs@opera.com BUG=425113,636475 ========== to ========== Fix reference for svg/clip-path/clip-path-multiple-children.svg The path data for the reference result was slightly different to that of the test, resulting in anti-aliasing differences. Matching up the paths exactly makes the reference match the test. R=fs@opera.com BUG=425113,636475 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by schenney@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com Link to the patchset: https://codereview.chromium.org/2246863006/#ps60001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix reference for svg/clip-path/clip-path-multiple-children.svg The path data for the reference result was slightly different to that of the test, resulting in anti-aliasing differences. Matching up the paths exactly makes the reference match the test. R=fs@opera.com BUG=425113,636475 ========== to ========== Fix reference for svg/clip-path/clip-path-multiple-children.svg The path data for the reference result was slightly different to that of the test, resulting in anti-aliasing differences. Matching up the paths exactly makes the reference match the test. R=fs@opera.com BUG=425113,636475 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Fix reference for svg/clip-path/clip-path-multiple-children.svg The path data for the reference result was slightly different to that of the test, resulting in anti-aliasing differences. Matching up the paths exactly makes the reference match the test. R=fs@opera.com BUG=425113,636475 ========== to ========== Fix reference for svg/clip-path/clip-path-multiple-children.svg The path data for the reference result was slightly different to that of the test, resulting in anti-aliasing differences. Matching up the paths exactly makes the reference match the test. R=fs@opera.com BUG=425113,636475 Committed: https://crrev.com/e7c705f5137839b87318d633aa7f66afb811ef42 Cr-Commit-Position: refs/heads/master@{#412839} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e7c705f5137839b87318d633aa7f66afb811ef42 Cr-Commit-Position: refs/heads/master@{#412839} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
