[css-shapes] Comprehensive tests for clip-path: shape() by noamr · Pull Request #33107 · web-platform-tests/wpt

noamr · 2022-03-08T09:22:30Z

No description provided.

birtles · 2022-03-09T02:33:22Z

Hi @BorisChiou -- sorry to throw so many tasks your way at the same time. I added you as a reviewer of this because I think you did the shape interpolation in Gecko. Is that right?

BorisChiou · 2022-03-09T18:50:56Z

I did the CSS path() interpolation and updated some code for other basic-shape functions in Gecko. Looks like this is a new function, shape() from https://drafts.csswg.org/css-shapes-2/#funcdef-shape. Sure, I can take a look at these tests. :)

noamr · 2022-03-09T18:52:46Z

I did the CSS path() interpolation and updated some code for other basic-shape functions in Gecko. Looks like this is a new function, shape() from https://drafts.csswg.org/css-shapes-2/#funcdef-shape. Sure, I can take a look at these tests. :)

I contributed the shape() definition and working on some draft (webkit) implementation which allowed me to write tests that work :)
Wanted to contribute the tests before working on upstreaming the implementation

BorisChiou

Overall looks good, but have some typos and nits. Would you mind updating them before merging? I request changes for now and will approve them after you update these nits. Thanks.

Basically, clip-path-shape-interpolation-*.html should be in the animation folder because these two files test the css animations. However, this is my bad because I added clip-path-path-interpolation-*.html in the same places. This patch looks good, and perhaps we should move them into animation folder in the future.

BorisChiou · 2022-03-09T22:35:59Z

+test_valid_value("clip-path", "shape(from 10px 10px, arc to 50px 1pt of 10px 10px)", "shape(from 10px 10px, arc to 50px 1pt of 10px ccw small)");
+test_valid_value("clip-path", "shape(from 10px 10px, arc to 50px 1pt of 10px 10px small rotate 0deg)", "shape(from 10px 10px, arc to 50px 1pt of 10px ccw small)");


Omitting rotate 0deg makes sense to me, but ccw and small are also default values, so I guess we don't have to serialize them, either?
So this should be something like:
test_valid_value("clip-path", "shape(from 10px 10px, arc to 50px 1pt of 10px 10px small rotate 0deg)", "shape(from 10px 10px, arc to 50px 1pt of 10px)");?

BorisChiou · 2022-03-09T22:42:16Z

+test_valid_value("clip-path", "shape(from 10px 10px, smooth to 50px 1pt)");
+test_valid_value("clip-path", "shape(from 10px 10px, arc to 50px 1pt of 10px 10px)", "shape(from 10px 10px, arc to 50px 1pt of 10px ccw small)");
+test_valid_value("clip-path", "shape(from 10px 10px, arc to 50px 1pt of 10px 10px small rotate 0deg)", "shape(from 10px 10px, arc to 50px 1pt of 10px ccw small)");
+test_valid_value("clip-path", "shape(from 10% 1rem, arc to 50px 1pt of 20% cw large rotate 25deg)", "shape(from 10% 1rem, arc to 50px 1pt of 20% rotate 25deg cw large)");


Perhaps we should follow the order when serializing these: [[<arc-sweep>] || [<arc-size>] || rotate [<angle>]]?
so this should be something like:
test_valid_value("clip-path", "shape(from 10% 1rem, arc to 50px 1pt of 20% rotate 25deg cw large)", "shape(from 10% 1rem, arc to 50px 1pt of 20% cw large rotate 25deg)");?

BorisChiou · 2022-03-09T22:59:01Z

+<!DOCTYPE html>
+<html>
+<head>
+  <title>CSS Masking: Test clip-path property and shape function with nonzero fill</title>


s/nonzero/evenodd/

BorisChiou · 2022-03-09T22:59:22Z

+  <link rel="help" href="https://drafts.csswg.org/css-shapes-2/#funcdef-shape">
+  <link rel="match" href="reference/clip-path-path-002-ref.html">
+  <meta name="assert" content="The clip-path property takes the basic shape
+	'shape()' for clipping. Test nonzero path function. On pass you should


s/nonzero/evenodd/

BorisChiou · 2022-03-09T23:03:30Z

+  <meta name="assert" content="The clip-path property takes the basic shape
+	'shape()' for clipping. Test curves.">


nit: we don't need to add meta "assert" for reference file

BorisChiou · 2022-03-09T23:07:29Z

+  <link rel="match" href="reference/clip-path-path-002-ref.html">
+  <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+  <meta name="assert" content="The clip-path property takes the basic shape
+	'shape()' for clipping. Test nonzero path function. On pass you should


s/nonzero/evenodd/

BorisChiou · 2022-03-09T23:09:38Z

+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>CSS Masking: Test clip-path nonzero path interpolation</title>


s/nonzero/evenodd/

BorisChiou · 2022-03-09T23:09:49Z

+  <link rel="help" href="https://drafts.csswg.org/css-shapes-2/#funcdef-shape">
+  <link rel="match" href="reference/clip-path-path-interpolation-002-ref.html">
+  <meta name="assert" content="The clip-path property takes the basic shape
+	'shape()' for clipping. Test the interpolation of nonzero


s/nonzero/evenodd/

BorisChiou · 2022-03-09T23:18:38Z

+test_interpolation({
+  property: 'clip-path',
+  from: 'shape(from 5% 5px, smooth by 10% 10px via 0% 80px, smooth by 30% 20px)',
+  to: 'shape(from 15% 15px, smooth by 20% 0px via 10% 60px, smooth by 20% 30px)',
+}, [
+  {at: -0.3, expect: 'shape(from 2% 2px, smooth by 7% 13px via -3% 86px, smooth by 33% 17px)'},
+  {at: 0, expect: 'shape(from 5% 5px, smooth by 10% 10px via 0% 80px, smooth by 30% 20px)'},
+  {at: 0.5, expect: 'shape(from 10% 10px, smooth by 15% 5px via 5% 70px, smooth by 25% 25px)'},
+  {at: 1.5, expect: 'shape(from 20% 20px, smooth by 25% -5px via 15% 50px, smooth by 15% 35px)'},
+]);


Is this duplicated? Looks like this is the same as the previous one.

by instead of to

BorisChiou · 2022-03-09T23:19:19Z

+test_interpolation({
+  property: 'clip-path',
+  from: 'shape(from 5% 5px, curve by 10% 10px via 0% 80px, curve by 30% 20px via 20% 50px 25% 70px)',
+  to: 'shape(from 15% 15px, curve by 20% 0px via 10% 60px, curve by 20% 30px via 30% 40px -5% 100px)',
+}, [
+  {at: -0.3, expect: 'shape(from 2% 2px, curve by 7% 13px via -3% 86px, curve by 33% 17px via 17% 53px 34% 61px)'},
+  {at: 0, expect: 'shape(from 5% 5px, curve by 10% 10px via 0% 80px, curve by 30% 20px via 20% 50px 25% 70px)'},
+  {at: 0.5, expect: 'shape(from 10% 10px, curve by 15% 5px via 5% 70px, curve by 25% 25px via 25% 45px 10% 85px)'},
+  {at: 1.5, expect: 'shape(from 20% 20px, curve by 25% -5px via 15% 50px, curve by 15% 35px via 35% 35px -20% 115px)'},
+]);


Is this duplicated? Looks like this is the same as the previous one.

It uses by instead of to

BorisChiou

Thanks!

Comprehensive tests for CSS shape()

6d13ebe

wpt-pr-bot added css-masking wg-css labels Mar 8, 2022

wpt-pr-bot assigned dirkschulze Mar 8, 2022

wpt-pr-bot requested review from birtles, dirkschulze and tabatkins March 8, 2022 09:22

noamr changed the title ~~[css-shapes] Comprehensive tests for shape()~~ [css-shapes] Comprehensive tests for clip-path: shape() Mar 8, 2022

birtles requested a review from BorisChiou March 9, 2022 02:32

BorisChiou requested changes Mar 9, 2022

View reviewed changes

CR comments

291c584

BorisChiou approved these changes Mar 10, 2022

View reviewed changes

noamr merged commit d152d79 into master Mar 10, 2022

noamr deleted the shape branch March 10, 2022 20:28

This was referenced Mar 21, 2022

Resource Timing: encoded/decoded body size for SW responses #33283

Closed

Iframes with missing redirect location should fire load event and queue RT entry #33317

Merged

noamr mentioned this pull request Mar 29, 2022

Report iframe resource timing also with 204/205 iframes, and don't report for download #33402

Merged

		test_valid_value("clip-path", "shape(from 10px 10px, arc to 50px 1pt of 10px 10px)", "shape(from 10px 10px, arc to 50px 1pt of 10px ccw small)");
		test_valid_value("clip-path", "shape(from 10px 10px, arc to 50px 1pt of 10px 10px small rotate 0deg)", "shape(from 10px 10px, arc to 50px 1pt of 10px ccw small)");

		<meta name="assert" content="The clip-path property takes the basic shape
		'shape()' for clipping. Test curves.">

Conversation

noamr commented Mar 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

birtles commented Mar 9, 2022

Uh oh!

BorisChiou commented Mar 9, 2022

Uh oh!

noamr commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BorisChiou left a comment • edited by tabatkins Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisChiou left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

noamr commented Mar 8, 2022 •

edited

Loading

noamr commented Mar 9, 2022 •

edited

Loading

BorisChiou left a comment •

edited by tabatkins

Loading