Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[motion-path] Make <ray-size> optional in ray()
https://bugs.webkit.org/show_bug.cgi?id=258110
rdar://110818689

Reviewed by Antti Koivisto and Darin Adler.

In the updated motion path spec, the size in the offset path ray() function is now optional. If not specified, the size
parameter defaults to `closest-side`.

Also implement logic to omit `closest-side` when serializing, since we should always serialize to the shortest form.

Spec: https://drafts.fxtf.org/motion-1/#ray-function

Fix pre-existing some unefficient idioms as well:
- Switch to consumeIdentRaw for parsing isContaining flag to avoid creating a CSSValue
- Make CSSRayValue store a CSSValueID for size instead of a CSSValue
- Make CSSRayValue use makeString for serialization

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/motion/offset-supports-calc-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/motion/parsing/offset-path-computed-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/motion/parsing/offset-path-parsing-valid-expected.txt:
* Source/WebCore/css/CSSRayValue.cpp:
(WebCore::CSSRayValue::customCSSText const):
(WebCore::CSSRayValue::equals const):
* Source/WebCore/css/CSSRayValue.h:
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::valueForPathOperation):
* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeRayShape):
* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertPathOperation):

Canonical link: https://commits.webkit.org/265200@main
  • Loading branch information
nt1m committed Jun 15, 2023
1 parent 9a055c1 commit 1a13191
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 38 deletions.
1 change: 0 additions & 1 deletion LayoutTests/TestExpectations
Expand Up @@ -5272,7 +5272,6 @@ webkit.org/b/233340 imported/w3c/web-platform-tests/css/motion/offset-anchor-tra
webkit.org/b/233344 imported/w3c/web-platform-tests/css/motion/offset-path-ray-007.html [ ImageOnlyFailure ]

# CSS motion path tests for missing <position> support in ray().
imported/w3c/web-platform-tests/css/motion/offset-path-ray-010.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/motion/offset-path-ray-011.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/motion/offset-path-ray-012.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/motion/offset-path-ray-013.html [ ImageOnlyFailure ]
Expand Down
@@ -1,6 +1,6 @@

PASS offset-position supports calc
FAIL offset-path supports calc assert_equals: expected "ray(270deg)" but got "ray(270deg closest-side)"
PASS offset-path supports calc
PASS offset-distance supports calc
PASS offset-rotate supports calc
PASS offset-anchor supports calc
Expand Down
@@ -1,7 +1,7 @@

PASS Property offset-path value 'none'
FAIL Property offset-path value 'ray(0deg)' assert_true: 'ray(0deg)' is a supported value for offset-path. expected true got false
FAIL Property offset-path value 'ray(0rad closest-side)' assert_equals: expected "ray(0deg)" but got "ray(0deg closest-side)"
PASS Property offset-path value 'ray(0deg)'
PASS Property offset-path value 'ray(0rad closest-side)'
PASS Property offset-path value 'ray(0.25turn closest-corner contain)'
PASS Property offset-path value 'ray(200grad farthest-side)'
PASS Property offset-path value 'ray(270deg farthest-corner contain)'
Expand Down
@@ -1,7 +1,7 @@

PASS e.style['offset-path'] = "none" should set the property value
FAIL e.style['offset-path'] = "ray(0deg)" should set the property value assert_not_equals: property should be set got disallowed value ""
FAIL e.style['offset-path'] = "ray(0rad closest-side)" should set the property value assert_equals: serialization should be canonical expected "ray(0rad)" but got "ray(0rad closest-side)"
PASS e.style['offset-path'] = "ray(0deg)" should set the property value
PASS e.style['offset-path'] = "ray(0rad closest-side)" should set the property value
PASS e.style['offset-path'] = "ray(0.25turn closest-corner contain)" should set the property value
PASS e.style['offset-path'] = "ray(200grad farthest-side)" should set the property value
PASS e.style['offset-path'] = "ray(270deg farthest-corner contain)" should set the property value
Expand Down
24 changes: 10 additions & 14 deletions Source/WebCore/css/CSSRayValue.cpp
Expand Up @@ -33,25 +33,21 @@ namespace WebCore {

String CSSRayValue::customCSSText() const
{
StringBuilder builder;

builder.append("ray(");
builder.append(m_angle->cssText());
builder.append(" ");
builder.append(m_size->cssText());

if (m_isContaining)
builder.append(" contain");

builder.append(")");

return builder.toString();
bool isNonDefaultSize = m_size != CSSValueClosestSide;

return makeString(
"ray("_s, m_angle->cssText(),
isNonDefaultSize ? " "_s : ""_s,
isNonDefaultSize ? nameLiteral(m_size) : ""_s,
m_isContaining ? " contain"_s : ""_s,
')'
);
}

bool CSSRayValue::equals(const CSSRayValue& other) const
{
return compareCSSValue(m_angle, other.m_angle)
&& compareCSSValue(m_size, other.m_size)
&& m_size == other.m_size
&& m_isContaining == other.m_isContaining;
}

Expand Down
13 changes: 6 additions & 7 deletions Source/WebCore/css/CSSRayValue.h
Expand Up @@ -36,32 +36,31 @@ namespace WebCore {
// https://drafts.fxtf.org/motion-1/#funcdef-offset-path-ray.
class CSSRayValue final : public CSSValue {
public:
static Ref<CSSRayValue> create(Ref<CSSPrimitiveValue>&& angle, Ref<CSSPrimitiveValue>&& size, bool isContaining)
static Ref<CSSRayValue> create(Ref<CSSPrimitiveValue>&& angle, CSSValueID size, bool isContaining)
{
return adoptRef(*new CSSRayValue(WTFMove(angle), WTFMove(size), isContaining));
return adoptRef(*new CSSRayValue(WTFMove(angle), size, isContaining));
}

String customCSSText() const;

Ref<CSSPrimitiveValue> angle() const { return m_angle; }
Ref<CSSPrimitiveValue> size() const { return m_size; }
CSSValueID size() const { return m_size; }
bool isContaining() const { return m_isContaining; }

bool equals(const CSSRayValue&) const;

private:
CSSRayValue(Ref<CSSPrimitiveValue>&& angle, Ref<CSSPrimitiveValue>&& size, bool isContaining)
CSSRayValue(Ref<CSSPrimitiveValue>&& angle, CSSValueID size, bool isContaining)
: CSSValue(RayClass)
, m_angle(WTFMove(angle))
, m_size(WTFMove(size))
, m_size(size)
, m_isContaining(isContaining)
{
ASSERT(m_angle->isAngle());
ASSERT(m_size->isValueID());
}

Ref<CSSPrimitiveValue> m_angle;
Ref<CSSPrimitiveValue> m_size;
CSSValueID m_size;
bool m_isContaining;
};

Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/css/ComputedStyleExtractor.cpp
Expand Up @@ -1704,9 +1704,8 @@ static Ref<CSSValue> valueForPathOperation(const RenderStyle& style, const PathO
auto& ray = downcast<RayPathOperation>(*operation);

auto angle = CSSPrimitiveValue::create(ray.angle(), CSSUnitType::CSS_DEG);
auto size = CSSPrimitiveValue::create(valueIDForRaySize(ray.size()));

return CSSRayValue::create(WTFMove(angle), WTFMove(size), ray.isContaining());
return CSSRayValue::create(WTFMove(angle), valueIDForRaySize(ray.size()), ray.isContaining());
}
}

Expand Down
17 changes: 9 additions & 8 deletions Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp
Expand Up @@ -6930,8 +6930,9 @@ static RefPtr<CSSValue> consumeBasicShapeOrBox(CSSParserTokenRange& range, const
return CSSValueList::createSpaceSeparated(WTFMove(list));
}

// Parses the ray() definition as defined in https://drafts.fxtf.org/motion-1/#funcdef-offset-path-ray
// ray( [ <angle> && <size> && contain? ] )
// Parses the ray() definition as defined in https://drafts.fxtf.org/motion-1/#ray-function
// ray( <angle> && <ray-size>? && contain? && [at <position>]? )
// FIXME: Implement `at <position>`.
static RefPtr<CSSRayValue> consumeRayShape(CSSParserTokenRange& range, const CSSParserContext& context)
{
if (range.peek().type() != FunctionToken || range.peek().functionId() != CSSValueRay)
Expand All @@ -6940,23 +6941,23 @@ static RefPtr<CSSRayValue> consumeRayShape(CSSParserTokenRange& range, const CSS
CSSParserTokenRange args = consumeFunction(range);

RefPtr<CSSPrimitiveValue> angle;
RefPtr<CSSPrimitiveValue> size;
std::optional<CSSValueID> size;
bool isContaining = false;
while (!args.atEnd()) {
if (!angle && (angle = consumeAngle(args, context.mode, UnitlessQuirk::Forbid, UnitlessZeroQuirk::Forbid)))
continue;
if (!size && (size = consumeIdent<CSSValueClosestSide, CSSValueClosestCorner, CSSValueFarthestSide, CSSValueFarthestCorner, CSSValueSides>(args)))
if (!size && (size = consumeIdentRaw<CSSValueClosestSide, CSSValueClosestCorner, CSSValueFarthestSide, CSSValueFarthestCorner, CSSValueSides>(args)))
continue;
if (!isContaining && (isContaining = consumeIdent<CSSValueContain>(args)))
if (!isContaining && (isContaining = consumeIdentRaw<CSSValueContain>(args).has_value()))
continue;
return nullptr;
}

// <angle> and <size> must be present.
if (!angle || !size)
// <angle> must be present.
if (!angle)
return nullptr;

return CSSRayValue::create(angle.releaseNonNull(), size.releaseNonNull(), isContaining);
return CSSRayValue::create(angle.releaseNonNull(), size.value_or(CSSValueClosestSide), isContaining);
}

// Consumes shapes accepted by clip-path and offset-path.
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/style/StyleBuilderConverter.h
Expand Up @@ -723,7 +723,7 @@ inline RefPtr<PathOperation> BuilderConverter::convertPathOperation(BuilderState
auto& rayValue = downcast<CSSRayValue>(value);

RayPathOperation::Size size = RayPathOperation::Size::ClosestCorner;
switch (rayValue.size()->valueID()) {
switch (rayValue.size()) {
case CSSValueClosestCorner:
size = RayPathOperation::Size::ClosestCorner;
break;
Expand Down

0 comments on commit 1a13191

Please sign in to comment.