Skip to content

Commit

Permalink
[CSS Math Functions] Correctly serialize round() root nodes
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259035
rdar://111984509

Reviewed by Darin Adler.

From the spec https://drafts.csswg.org/css-values-4/#calc-simplification:

> If root is an operator node that’s not one of the calc-operator nodes, and all of its calculation children are numeric values with enough information to compute the operation root represents, return the result of running root’s operation using its children, expressed in the result’s canonical unit.

We now always try to resolve the top-level round() function, e.g. round(up, 2px, 5px) now gives calc(5px) instead of round(up, 2px, 5px).
This is consistent with calc(round(up, 2px, 5px)) being serialized as calc(5px).

* LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-serialize-expected.txt:
* Source/WebCore/css/calc/CSSCalcOperationNode.cpp:
(WebCore::CSSCalcOperationNode::simplifyNode):
* Source/WebCore/css/calc/CSSCalcOperationNode.h:
* Source/WebCore/platform/calc/CalcOperator.h: Remove unnecessary include.

Canonical link: https://commits.webkit.org/265892@main
  • Loading branch information
nt1m committed Jul 10, 2023
1 parent c7b1f51 commit 9ff9626
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

FAIL 'round(1.1,1)' as a specified value should serialize as 'calc(1)'. assert_equals: 'round(1.1,1)' and 'calc(1)' should serialize the same in specified values. expected "calc(1)" but got "round(nearest, 1.1, 1)"
FAIL 'scale(round(1.1,1))' as a specified value should serialize as 'scale(calc(1))'. assert_equals: 'scale(round(1.1,1))' and 'scale(calc(1))' should serialize the same in specified values. expected "scale(calc(1))" but got "scale(round(nearest, 1.1, 1))"
PASS 'round(1.1,1)' as a specified value should serialize as 'calc(1)'.
PASS 'scale(round(1.1,1))' as a specified value should serialize as 'scale(calc(1))'.
PASS 'round(1.1,1)' as a computed value should serialize as '1'.
PASS 'scale(round(1.1,1))' as a computed value should serialize as 'matrix(1, 0, 0, 1, 0, 0)'.
PASS 'mod(1,1)' as a specified value should serialize as 'calc(0)'.
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/css/calc/CSSCalcOperationNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ Ref<CSSCalcExpressionNode> CSSCalcOperationNode::simplifyNode(Ref<CSSCalcExpress
calcOperationNode.hoistChildrenWithOperator(CalcOperator::Multiply);
}

if (calcOperationNode.shouldNotPreserveFunction() || calcOperationNode.isCalcProductNode() || calcOperationNode.isCalcSumNode() || (calcOperationNode.shouldPreserveFunction() && depth))
if (calcOperationNode.isNonCalcFunction() || calcOperationNode.isCalcProductNode() || calcOperationNode.isCalcSumNode())
calcOperationNode.combineChildren();

// If only one child remains, return the child (except at the root).
Expand All @@ -904,7 +904,7 @@ Ref<CSSCalcExpressionNode> CSSCalcOperationNode::simplifyNode(Ref<CSSCalcExpress
if (depth)
return true;

if (parent.shouldNotPreserveFunction() && is<CSSCalcPrimitiveValueNode>(parent.children()[0])) {
if (parent.isNonCalcFunction() && is<CSSCalcPrimitiveValueNode>(parent.children()[0])) {
parent.makeTopLevelCalc();
return false;
}
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/css/calc/CSSCalcOperationNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ class CSSCalcOperationNode final : public CSSCalcExpressionNode {
bool isHypotNode() const { return m_operator == CalcOperator::Hypot; }
bool isSqrtNode() const { return m_operator == CalcOperator::Sqrt; }
bool isPowOrSqrtNode() const { return m_operator == CalcOperator::Pow || isSqrtNode(); }
bool shouldPreserveFunction() const { return isRoundOperation(); }
bool isClampNode() const { return m_operator == CalcOperator::Clamp; }

void hoistChildrenWithOperator(CalcOperator);
Expand Down Expand Up @@ -148,7 +147,7 @@ class CSSCalcOperationNode final : public CSSCalcExpressionNode {
}

void makeTopLevelCalc();
bool shouldNotPreserveFunction() const { return isAbsOrSignNode() || isClampNode() || isMinOrMaxNode() || isExpNode() || isHypotNode() || isSteppedNode() || isPowOrSqrtNode() || isInverseTrigNode() || isAtan2Node() || isTrigNode(); }
bool isNonCalcFunction() const { return isAbsOrSignNode() || isClampNode() || isMinOrMaxNode() || isExpNode() || isHypotNode() || isRoundOperation() || isSteppedNode() || isPowOrSqrtNode() || isInverseTrigNode() || isAtan2Node() || isTrigNode(); }
static double evaluateOperator(CalcOperator, const Vector<double>&);
static Ref<CSSCalcExpressionNode> simplifyNode(Ref<CSSCalcExpressionNode>&&, int depth);
static Ref<CSSCalcExpressionNode> simplifyRecursive(Ref<CSSCalcExpressionNode>&&, int depth);
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/platform/calc/CalcOperator.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

#pragma once

#include <cmath>
#include <wtf/Forward.h>
#include <wtf/MathExtras.h>

Expand Down

0 comments on commit 9ff9626

Please sign in to comment.