Skip to content

Commit

Permalink
Implement calculation dependency for css math nodes
Browse files Browse the repository at this point in the history
By design doc:
https://docs.google.com/document/d/1rikPXstHFK3b8N95QzfDar6dDDrEsuVgw1YF-JgvKQM
introduce runtime flag and implement calculation dependency to split up
the calculation result and the calculation dependency, as some functions
(e.g. sign()) can have different input and output types.

Change-Id: I96b65d62fa17aeef8524d39a8d37c4804625efa4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4607454
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Daniil Sakhapov <sakhapov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1158868}
  • Loading branch information
danielsakhapov authored and Chromium LUCI CQ committed Jun 16, 2023
1 parent add4d33 commit 5e2670d
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 89 deletions.
201 changes: 133 additions & 68 deletions third_party/blink/renderer/core/css/css_math_expression_node.cc

Large diffs are not rendered by default.

26 changes: 19 additions & 7 deletions third_party/blink/renderer/core/css/css_math_expression_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class CSSParserContext;

// The order of this enum should not change since its elements are used as
// indices in the addSubtractResult matrix.
enum CalculationCategory {
enum CalculationResultCategory {
kCalcNumber,
kCalcLength,
kCalcPercent,
Expand All @@ -79,6 +79,7 @@ class CORE_EXPORT CSSMathExpressionNode
CSSValueID function_id,
CSSParserTokenRange tokens,
const CSSParserContext&,
const bool is_percentage_allowed,
CSSAnchorQueryTypes allowed_anchor_queries);

virtual bool IsNumericLiteral() const { return false; }
Expand Down Expand Up @@ -127,10 +128,13 @@ class CORE_EXPORT CSSMathExpressionNode

virtual bool IsComputationallyIndependent() const = 0;

CalculationCategory Category() const { return category_; }
CalculationResultCategory Category() const { return category_; }
bool HasPercentage() const {
return category_ == kCalcPercent || category_ == kCalcPercentLength;
}
bool CanBeResolvedWithConversionData() const {
return can_be_resolved_with_conversion_data_;
}

// Returns the unit type of the math expression *without doing any type
// conversion* (e.g., 1px + 1em needs type conversion to resolve).
Expand Down Expand Up @@ -164,16 +168,20 @@ class CORE_EXPORT CSSMathExpressionNode
virtual void Trace(Visitor* visitor) const {}

protected:
CSSMathExpressionNode(CalculationCategory category,
CSSMathExpressionNode(CalculationResultCategory category,
const bool can_be_resolved_with_conversion_data,
bool has_comparisons,
bool needs_tree_scope_population)
: category_(category),
can_be_resolved_with_conversion_data_(
can_be_resolved_with_conversion_data),
has_comparisons_(has_comparisons),
needs_tree_scope_population_(needs_tree_scope_population) {
DCHECK_NE(category, kCalcOther);
}

CalculationCategory category_;
CalculationResultCategory category_;
bool can_be_resolved_with_conversion_data_;
bool is_nested_calc_ = false;
bool has_comparisons_;
bool needs_tree_scope_population_;
Expand Down Expand Up @@ -272,13 +280,17 @@ class CORE_EXPORT CSSMathExpressionOperation final
CSSMathExpressionOperation(const CSSMathExpressionNode* left_side,
const CSSMathExpressionNode* right_side,
CSSMathOperator op,
CalculationCategory category);
CalculationResultCategory category,
const bool can_be_resolved_with_conversion_data);

CSSMathExpressionOperation(CalculationCategory category,
CSSMathExpressionOperation(CalculationResultCategory category,
const bool can_be_resolved_with_conversion_data,
Operands&& operands,
CSSMathOperator op);

CSSMathExpressionOperation(CalculationCategory category, CSSMathOperator op);
CSSMathExpressionOperation(CalculationResultCategory category,
const bool can_be_resolved_with_conversion_data,
CSSMathOperator op);

const Operands& GetOperands() const { return operands_; }
CSSMathOperator OperatorType() const { return operator_; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,11 @@ TEST(CSSMathExpressionNode, TestParseDeeplyNestedExpression) {
const CSSParserContext* context = MakeGarbageCollected<CSSParserContext>(
kHTMLStandardMode, SecureContextMode::kInsecureContext);
const CSSMathExpressionNode* res = CSSMathExpressionNode::ParseMathFunction(
CSSValueID::kCalc, range, *context, kCSSAnchorQueryTypesNone);
CSSValueID::kCalc, range, *context, true, kCSSAnchorQueryTypesNone);

if (test_case.expected) {
EXPECT_TRUE(res);
EXPECT_TRUE(res->CanBeResolvedWithConversionData());
} else {
EXPECT_FALSE(res);
}
Expand All @@ -365,12 +366,13 @@ TEST(CSSMathExpressionNode, TestSteppedValueFunctions) {
const CSSParserContext* context = MakeGarbageCollected<CSSParserContext>(
kHTMLStandardMode, SecureContextMode::kInsecureContext);
const CSSMathExpressionNode* res = CSSMathExpressionNode::ParseMathFunction(
CSSValueID::kCalc, range, *context, kCSSAnchorQueryTypesNone);
CSSValueID::kCalc, range, *context, true, kCSSAnchorQueryTypesNone);
EXPECT_EQ(res->DoubleValue(), test_case.output);
CSSToLengthConversionData resolver{};
scoped_refptr<const CalculationExpressionNode> node =
res->ToCalculationExpression(resolver);
EXPECT_EQ(node->Evaluate(FLT_MAX, nullptr), test_case.output);
EXPECT_TRUE(res->CanBeResolvedWithConversionData());
}
}

Expand All @@ -390,7 +392,7 @@ TEST(CSSMathExpressionNode, TestSteppedValueFunctionsToCalculationExpression) {
CSSMathExpressionNumericLiteral::Create(
10, CSSPrimitiveValue::UnitType::kNumber)};
const auto* operation = MakeGarbageCollected<CSSMathExpressionOperation>(
kCalcNumber, std::move(operands), test_case.op);
kCalcNumber, true, std::move(operands), test_case.op);
CSSToLengthConversionData resolver{};
scoped_refptr<const CalculationExpressionNode> node =
operation->ToCalculationExpression(resolver);
Expand All @@ -417,7 +419,7 @@ TEST(CSSMathExpressionNode, TestSteppedValueFunctionsSerialization) {
const CSSParserContext* context = MakeGarbageCollected<CSSParserContext>(
kHTMLStandardMode, SecureContextMode::kInsecureContext);
const CSSMathExpressionNode* res = CSSMathExpressionNode::ParseMathFunction(
CSSValueID::kCalc, range, *context, kCSSAnchorQueryTypesNone);
CSSValueID::kCalc, range, *context, true, kCSSAnchorQueryTypesNone);
EXPECT_EQ(res->CustomCSSText(), test_case.input);
}
}
Expand All @@ -438,20 +440,24 @@ TEST(CSSMathExpressionNode, TestExponentialFunctions) {
const CSSParserContext* context = MakeGarbageCollected<CSSParserContext>(
kHTMLStandardMode, SecureContextMode::kInsecureContext);
const CSSMathExpressionNode* res = CSSMathExpressionNode::ParseMathFunction(
CSSValueID::kCalc, range, *context, kCSSAnchorQueryTypesNone);
CSSValueID::kCalc, range, *context, true, kCSSAnchorQueryTypesNone);
EXPECT_EQ(res->DoubleValue(), test_case.output);
CSSToLengthConversionData resolver;
scoped_refptr<const CalculationExpressionNode> node =
res->ToCalculationExpression(resolver);
EXPECT_EQ(node->Evaluate(FLT_MAX, nullptr), test_case.output);
EXPECT_TRUE(res->CanBeResolvedWithConversionData());
}
}

TEST(CSSMathExpressionNode, TestExponentialFunctionsSerialization) {
const struct TestCase {
const String input;
const bool can_be_simplified_with_conversion_data;
} test_cases[] = {
{"hypot(3%, 4%)"},
{"hypot(3em, 4rem)", true},
{"hypot(3%, 4%)", false},
{"hypot(hypot(3%, 4%), 5em)", false},
};

for (const auto& test_case : test_cases) {
Expand All @@ -461,8 +467,10 @@ TEST(CSSMathExpressionNode, TestExponentialFunctionsSerialization) {
const CSSParserContext* context = MakeGarbageCollected<CSSParserContext>(
kHTMLStandardMode, SecureContextMode::kInsecureContext);
const CSSMathExpressionNode* res = CSSMathExpressionNode::ParseMathFunction(
CSSValueID::kCalc, range, *context, kCSSAnchorQueryTypesNone);
CSSValueID::kCalc, range, *context, true, kCSSAnchorQueryTypesNone);
EXPECT_EQ(res->CustomCSSText(), test_case.input);
EXPECT_EQ(res->CanBeResolvedWithConversionData(),
test_case.can_be_simplified_with_conversion_data);
}
}

Expand All @@ -479,7 +487,7 @@ TEST(CSSMathExpressionNode, TestExponentialFunctionsToCalculationExpression) {
CSSMathExpressionNumericLiteral::Create(
4.0f, CSSPrimitiveValue::UnitType::kNumber)};
const auto* operation = MakeGarbageCollected<CSSMathExpressionOperation>(
kCalcNumber, std::move(operands), test_case.op);
kCalcNumber, true, std::move(operands), test_case.op);
CSSToLengthConversionData resolver{};
scoped_refptr<const CalculationExpressionNode> node =
operation->ToCalculationExpression(resolver);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ class CORE_EXPORT CSSMathFunctionValue : public CSSPrimitiveValue {

bool MayHaveRelativeUnit() const;

CalculationCategory Category() const { return expression_->Category(); }
CalculationResultCategory Category() const { return expression_->Category(); }
bool CanBeResolvedWithConversionData() const {
return expression_->CanBeResolvedWithConversionData();
}

bool IsAngle() const { return Category() == kCalcAngle; }
bool IsLength() const { return Category() == kCalcLength; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ CSSNumericValue* CSSNumericValue::parse(
CSSMathExpressionNode::ParseMathFunction(
CSSValueID::kCalc, range,
*MakeGarbageCollected<CSSParserContext>(*execution_context),
kCSSAnchorQueryTypesNone);
true /* is_percentage_allowed */, kCSSAnchorQueryTypesNone);
if (expression) {
return CalcToNumericValue(*expression);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -944,14 +944,15 @@ class MathFunctionParser {
CSSParserTokenRange& range,
const CSSParserContext& context,
CSSPrimitiveValue::ValueRange value_range,
const bool is_percentage_allowed = true,
CSSAnchorQueryTypes allowed_anchor_queries = kCSSAnchorQueryTypesNone)
: source_range_(range), range_(range) {
const CSSParserToken& token = range.Peek();
if (token.GetType() == kFunctionToken) {
calc_value_ = CSSMathFunctionValue::Create(
CSSMathExpressionNode::ParseMathFunction(
token.FunctionId(), ConsumeFunction(range_), context,
allowed_anchor_queries),
is_percentage_allowed, allowed_anchor_queries),
value_range);
}
if (calc_value_ && calc_value_->HasComparisons()) {
Expand Down Expand Up @@ -1259,7 +1260,7 @@ CSSPrimitiveValue* ConsumeAlphaValue(CSSParserTokenRange& range,
CSSPrimitiveValue::ValueRange::kAll);
}

bool CanConsumeCalcValue(CalculationCategory category,
bool CanConsumeCalcValue(CalculationResultCategory category,
CSSParserMode css_parser_mode) {
return category == kCalcLength || category == kCalcPercent ||
category == kCalcPercentLength ||
Expand All @@ -1280,6 +1281,7 @@ CSSPrimitiveValue* ConsumeLengthOrPercent(
return ConsumePercent(range, context, value_range);
}
MathFunctionParser math_parser(range, context, value_range,
true /* is_percentage_allowed */,
allowed_anchor_queries);
if (const CSSMathFunctionValue* calculation = math_parser.Value()) {
if (CanConsumeCalcValue(calculation->Category(), context.Mode())) {
Expand Down Expand Up @@ -2953,7 +2955,7 @@ static CSSPrimitiveValue* ConsumeGradientAngleOrPercent(
}
MathFunctionParser math_parser(range, context, value_range);
if (const CSSMathFunctionValue* calculation = math_parser.Value()) {
CalculationCategory category = calculation->Category();
CalculationResultCategory category = calculation->Category();
// TODO(fs): Add and support kCalcPercentAngle?
if (category == kCalcAngle || category == kCalcPercent) {
return math_parser.ConsumeValue();
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/svg/svg_length.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ static bool IsSupportedCSSUnitType(CSSPrimitiveValue::UnitType type) {
type != CSSPrimitiveValue::UnitType::kQuirkyEms;
}

static bool IsSupportedCalculationCategory(CalculationCategory category) {
static bool IsSupportedCalculationCategory(CalculationResultCategory category) {
switch (category) {
case kCalcLength:
case kCalcNumber:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,12 @@
status: "test",
base_feature: "none",
},
// Support relative numbers in non-length properties.
// Design doc: https://docs.google.com/document/d/1rikPXstHFK3b8N95QzfDar6dDDrEsuVgw1YF-JgvKQM
{
name: "CSSRelativeNumbersForNonLengthProperties",
status: "experimental",
},
{
// https://drafts.csswg.org/css-cascade-6/#scoped-styles
name: "CSSScope",
Expand Down

0 comments on commit 5e2670d

Please sign in to comment.