mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-12-07 21:59:54 +00:00
LibWeb: Avoid early conversion to CSSPixels when simplifying calculation
Converting to CSSPixels caused us to lose precision and the sign of signed zeroes. The values we resolve against in Length::ResolutionContext are still themselves rounded too early but this is in the right direction.
This commit is contained in:
Callum Law
Sam Atkins
parent
43b06cbbdd
commit
0b45a68423
Notes:
github-actions[bot]
2025-10-23 08:35:32 +00:00
Author: https://github.com/Calme1709
Commit: 0b45a68423
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/6544
Reviewed-by: https://github.com/AtkinsSJ ✅
6 changed files with 63 additions and 39 deletions
|
|
@ -38,74 +38,84 @@ Length Length::percentage_of(Percentage const& percentage) const
|
|||
}
|
||||
|
||||
CSSPixels Length::font_relative_length_to_px(Length::FontMetrics const& font_metrics, Length::FontMetrics const& root_font_metrics) const
|
||||
{
|
||||
return CSSPixels::nearest_value_for(font_relative_length_to_px_without_rounding(font_metrics, root_font_metrics));
|
||||
}
|
||||
|
||||
double Length::font_relative_length_to_px_without_rounding(Length::FontMetrics const& font_metrics, Length::FontMetrics const& root_font_metrics) const
|
||||
{
|
||||
switch (m_unit) {
|
||||
case LengthUnit::Em:
|
||||
return CSSPixels::nearest_value_for(m_value * font_metrics.font_size.to_double());
|
||||
return m_value * font_metrics.font_size.to_double();
|
||||
case LengthUnit::Rem:
|
||||
return CSSPixels::nearest_value_for(m_value * root_font_metrics.font_size.to_double());
|
||||
return m_value * root_font_metrics.font_size.to_double();
|
||||
case LengthUnit::Ex:
|
||||
return CSSPixels::nearest_value_for(m_value * font_metrics.x_height.to_double());
|
||||
return m_value * font_metrics.x_height.to_double();
|
||||
case LengthUnit::Rex:
|
||||
return CSSPixels::nearest_value_for(m_value * root_font_metrics.x_height.to_double());
|
||||
return m_value * root_font_metrics.x_height.to_double();
|
||||
case LengthUnit::Cap:
|
||||
return CSSPixels::nearest_value_for(m_value * font_metrics.cap_height.to_double());
|
||||
return m_value * font_metrics.cap_height.to_double();
|
||||
case LengthUnit::Rcap:
|
||||
return CSSPixels::nearest_value_for(m_value * root_font_metrics.cap_height.to_double());
|
||||
return m_value * root_font_metrics.cap_height.to_double();
|
||||
case LengthUnit::Ch:
|
||||
return CSSPixels::nearest_value_for(m_value * font_metrics.zero_advance.to_double());
|
||||
return m_value * font_metrics.zero_advance.to_double();
|
||||
case LengthUnit::Rch:
|
||||
return CSSPixels::nearest_value_for(m_value * root_font_metrics.zero_advance.to_double());
|
||||
return m_value * root_font_metrics.zero_advance.to_double();
|
||||
case LengthUnit::Ic:
|
||||
// FIXME: Use the "advance measure of the “水” (CJK water ideograph, U+6C34) glyph"
|
||||
return CSSPixels::nearest_value_for(m_value * font_metrics.font_size.to_double());
|
||||
return m_value * font_metrics.font_size.to_double();
|
||||
case LengthUnit::Ric:
|
||||
// FIXME: Use the "advance measure of the “水” (CJK water ideograph, U+6C34) glyph"
|
||||
return CSSPixels::nearest_value_for(m_value * root_font_metrics.font_size.to_double());
|
||||
return m_value * root_font_metrics.font_size.to_double();
|
||||
case LengthUnit::Lh:
|
||||
return CSSPixels::nearest_value_for(m_value * font_metrics.line_height.to_double());
|
||||
return m_value * font_metrics.line_height.to_double();
|
||||
case LengthUnit::Rlh:
|
||||
return CSSPixels::nearest_value_for(m_value * root_font_metrics.line_height.to_double());
|
||||
return m_value * root_font_metrics.line_height.to_double();
|
||||
default:
|
||||
VERIFY_NOT_REACHED();
|
||||
}
|
||||
}
|
||||
|
||||
CSSPixels Length::viewport_relative_length_to_px(CSSPixelRect const& viewport_rect) const
|
||||
{
|
||||
return CSSPixels::nearest_value_for(viewport_relative_length_to_px_without_rounding(viewport_rect));
|
||||
}
|
||||
|
||||
double Length::viewport_relative_length_to_px_without_rounding(CSSPixelRect const& viewport_rect) const
|
||||
{
|
||||
switch (m_unit) {
|
||||
case LengthUnit::Vw:
|
||||
case LengthUnit::Svw:
|
||||
case LengthUnit::Lvw:
|
||||
case LengthUnit::Dvw:
|
||||
return viewport_rect.width() * (CSSPixels::nearest_value_for(m_value) / 100);
|
||||
return viewport_rect.width() * m_value / 100;
|
||||
case LengthUnit::Vh:
|
||||
case LengthUnit::Svh:
|
||||
case LengthUnit::Lvh:
|
||||
case LengthUnit::Dvh:
|
||||
return viewport_rect.height() * (CSSPixels::nearest_value_for(m_value) / 100);
|
||||
return viewport_rect.height() * m_value / 100;
|
||||
case LengthUnit::Vi:
|
||||
case LengthUnit::Svi:
|
||||
case LengthUnit::Lvi:
|
||||
case LengthUnit::Dvi:
|
||||
// FIXME: Select the width or height based on which is the inline axis.
|
||||
return viewport_rect.width() * (CSSPixels::nearest_value_for(m_value) / 100);
|
||||
return viewport_rect.width() * m_value / 100;
|
||||
case LengthUnit::Vb:
|
||||
case LengthUnit::Svb:
|
||||
case LengthUnit::Lvb:
|
||||
case LengthUnit::Dvb:
|
||||
// FIXME: Select the width or height based on which is the block axis.
|
||||
return viewport_rect.height() * (CSSPixels::nearest_value_for(m_value) / 100);
|
||||
return viewport_rect.height() * m_value / 100;
|
||||
case LengthUnit::Vmin:
|
||||
case LengthUnit::Svmin:
|
||||
case LengthUnit::Lvmin:
|
||||
case LengthUnit::Dvmin:
|
||||
return min(viewport_rect.width(), viewport_rect.height()) * (CSSPixels::nearest_value_for(m_value) / 100);
|
||||
return min(viewport_rect.width(), viewport_rect.height()) * m_value / 100;
|
||||
case LengthUnit::Vmax:
|
||||
case LengthUnit::Svmax:
|
||||
case LengthUnit::Lvmax:
|
||||
case LengthUnit::Dvmax:
|
||||
return max(viewport_rect.width(), viewport_rect.height()) * (CSSPixels::nearest_value_for(m_value) / 100);
|
||||
return max(viewport_rect.width(), viewport_rect.height()) * m_value / 100;
|
||||
default:
|
||||
VERIFY_NOT_REACHED();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -78,6 +78,18 @@ public:
|
|||
return to_px_slow_case(node);
|
||||
}
|
||||
|
||||
ALWAYS_INLINE double to_px_without_rounding(ResolutionContext const& context) const
|
||||
{
|
||||
if (is_absolute())
|
||||
return absolute_length_to_px_without_rounding();
|
||||
if (is_font_relative())
|
||||
return font_relative_length_to_px_without_rounding(context.font_metrics, context.root_font_metrics);
|
||||
if (is_viewport_relative())
|
||||
return viewport_relative_length_to_px_without_rounding(context.viewport_rect);
|
||||
|
||||
VERIFY_NOT_REACHED();
|
||||
}
|
||||
|
||||
ALWAYS_INLINE CSSPixels to_px(CSSPixelRect const& viewport_rect, FontMetrics const& font_metrics, FontMetrics const& root_font_metrics) const
|
||||
{
|
||||
if (is_absolute())
|
||||
|
|
@ -108,7 +120,9 @@ public:
|
|||
}
|
||||
|
||||
CSSPixels font_relative_length_to_px(FontMetrics const& font_metrics, FontMetrics const& root_font_metrics) const;
|
||||
double font_relative_length_to_px_without_rounding(FontMetrics const& font_metrics, FontMetrics const& root_font_metrics) const;
|
||||
CSSPixels viewport_relative_length_to_px(CSSPixelRect const& viewport_rect) const;
|
||||
double viewport_relative_length_to_px_without_rounding(CSSPixelRect const& viewport_rect) const;
|
||||
|
||||
// Returns empty optional if it's already absolute.
|
||||
Optional<Length> absolutize(CSSPixelRect const& viewport_rect, FontMetrics const& font_metrics, FontMetrics const& root_font_metrics) const;
|
||||
|
|
|
|||
|
|
@ -2531,7 +2531,7 @@ CalculatedStyleValue::CalculationResult CalculatedStyleValue::CalculationResult:
|
|||
return AK::NaN<double>;
|
||||
}
|
||||
|
||||
return length.to_px(context.length_resolution_context.value()).to_double();
|
||||
return length.to_px_without_rounding(context.length_resolution_context.value());
|
||||
},
|
||||
[](Resolution const& resolution) { return resolution.to_dots_per_pixel(); },
|
||||
[](Time const& time) { return time.to_seconds(); },
|
||||
|
|
@ -2900,7 +2900,7 @@ NonnullRefPtr<CalculationNode const> simplify_a_calculation_tree(CalculationNode
|
|||
if (length.is_absolute())
|
||||
return NumericCalculationNode::create(Length::make_px(length.absolute_length_to_px()).percentage_of(*percentage), context);
|
||||
if (resolution_context.length_resolution_context.has_value())
|
||||
return NumericCalculationNode::create(Length::make_px(length.to_px(resolution_context.length_resolution_context.value())).percentage_of(*percentage), context);
|
||||
return NumericCalculationNode::create(Length::make_px(length.to_px_without_rounding(resolution_context.length_resolution_context.value())).percentage_of(*percentage), context);
|
||||
return nullptr;
|
||||
},
|
||||
[&](Time const& time) -> RefPtr<NumericCalculationNode const> {
|
||||
|
|
@ -2938,9 +2938,9 @@ NonnullRefPtr<CalculationNode const> simplify_a_calculation_tree(CalculationNode
|
|||
if (length.unit() == LengthUnit::Px)
|
||||
return nullptr;
|
||||
if (length.is_absolute())
|
||||
return NumericCalculationNode::create(Length::make_px(length.absolute_length_to_px()), context);
|
||||
return NumericCalculationNode::create(Length::make_px(length.absolute_length_to_px_without_rounding()), context);
|
||||
if (resolution_context.length_resolution_context.has_value())
|
||||
return NumericCalculationNode::create(Length::make_px(length.to_px(resolution_context.length_resolution_context.value())), context);
|
||||
return NumericCalculationNode::create(Length::make_px(length.to_px_without_rounding(resolution_context.length_resolution_context.value())), context);
|
||||
return nullptr;
|
||||
},
|
||||
[&](Number const&) -> RefPtr<CalculationNode const> {
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@ Harness status: OK
|
|||
|
||||
Found 229 tests
|
||||
|
||||
217 Pass
|
||||
12 Fail
|
||||
219 Pass
|
||||
10 Fail
|
||||
Pass round(10,10) should be used-value-equivalent to 10
|
||||
Pass mod(1,1) should be used-value-equivalent to 0
|
||||
Pass rem(1,1) should be used-value-equivalent to 0
|
||||
|
|
@ -61,8 +61,8 @@ Pass calc(rem(mod(18,5),mod(17,5))) should be used-value-equivalent to 1
|
|||
Pass calc(mod(-140,-90)) should be used-value-equivalent to -50
|
||||
Pass calc(mod(rem(1,18)* -1,5)) should be used-value-equivalent to 4
|
||||
Pass round(10px,6px) should be used-value-equivalent to 12px
|
||||
Fail round(10cm,6cm) should be used-value-equivalent to 12cm
|
||||
Fail round(10mm,6mm) should be used-value-equivalent to 12mm
|
||||
Pass round(10cm,6cm) should be used-value-equivalent to 12cm
|
||||
Pass round(10mm,6mm) should be used-value-equivalent to 12mm
|
||||
Pass round(10Q, 6Q) should be used-value-equivalent to 12Q
|
||||
Pass round(10in,6in) should be used-value-equivalent to 12in
|
||||
Pass round(10pc,6pc) should be used-value-equivalent to 12pc
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@ Harness status: OK
|
|||
|
||||
Found 233 tests
|
||||
|
||||
221 Pass
|
||||
12 Fail
|
||||
227 Pass
|
||||
6 Fail
|
||||
Pass abs(1) should be used-value-equivalent to 1
|
||||
Pass sign(1) should be used-value-equivalent to 1
|
||||
Pass abs(-1) should be used-value-equivalent to 1
|
||||
|
|
@ -114,17 +114,17 @@ Pass clamp(-1, calc( 1 / sign(sign(0vmax))), 1) should be used-value-equivalent
|
|||
Pass calc(sign(-0px)) should be used-value-equivalent to 0
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0px))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0cm)) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, calc( 1 / sign(sign(-0cm))), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0cm))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0mm)) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, calc( 1 / sign(sign(-0mm))), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0mm))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0Q)) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, calc( 1 / sign(sign(-0Q))), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0Q))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0in)) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, calc( 1 / sign(sign(-0in))), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0in))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0pc)) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, calc( 1 / sign(sign(-0pc))), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0pc))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0pt)) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, calc( 1 / sign(sign(-0pt))), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0pt))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0em)) should be used-value-equivalent to 0
|
||||
Pass clamp(-1, calc( 1 / sign(sign(-0em))), 1) should be used-value-equivalent to -1
|
||||
Pass calc(sign(-0ex)) should be used-value-equivalent to 0
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@ Harness status: OK
|
|||
|
||||
Found 35 tests
|
||||
|
||||
29 Pass
|
||||
6 Fail
|
||||
32 Pass
|
||||
3 Fail
|
||||
Pass min(1em, 110px / 10px * 1px) should be used-value-equivalent to 10px
|
||||
Pass max(10px, 110px / 10px * 1px) should be used-value-equivalent to 11px
|
||||
Pass max(1em + 2px, 110px / 10px * 1px) should be used-value-equivalent to 12px
|
||||
|
|
@ -26,14 +26,14 @@ Pass e.style['width'] = "calc((1% * 1deg) / 1px)" should not set the property va
|
|||
Pass e.style['width'] = "calc((1% * 1% * 1%) / 1px)" should not set the property value
|
||||
Pass Property width value 'calc(1px * 10em / 0em)'
|
||||
Pass Property width value 'calc(1px / 1px * 10em * infinity)'
|
||||
Fail Property margin-left value 'calc(1px * 10em / -0em)'
|
||||
Pass Property margin-left value 'calc(1px * 10em / -0em)'
|
||||
Pass Property z-index value 'calc(10em / 0em)'
|
||||
Pass sign(-0em / 1px) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, 1 / sign(-0em / 1px), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, 1 / sign(-0em / 1px), 1) should be used-value-equivalent to -1
|
||||
Fail sign( 0cqi / 1px) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, 1 / sign( 0cqi / 1px), 1) should be used-value-equivalent to 1
|
||||
Pass sign(atan2(-0cap / 1px, 0em / 1px)) should be used-value-equivalent to 0
|
||||
Fail clamp(-1, 1 / sign(atan2(-0cap / 1px, 0em / 1px)), 1) should be used-value-equivalent to -1
|
||||
Pass clamp(-1, 1 / sign(atan2(-0cap / 1px, 0em / 1px)), 1) should be used-value-equivalent to -1
|
||||
Pass sign(exp(-1vh / 0px)) should be used-value-equivalent to 0
|
||||
Pass clamp(-1, 1 / sign(exp(-1vh / 0px)), 1) should be used-value-equivalent to 1
|
||||
Fail calc(20cqw / 1rem) should be used-value-equivalent to 2
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue