-
Notifications
You must be signed in to change notification settings - Fork 6k
Update web lerpDouble to match C++ behaviour #21010
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@yjbanov happy to add tests like I did with the dart:ui side, but not sure what the standard approach is here -- do you just copy/paste then patch the dart:ui tests? If so, is there any mechanism to ensure they don't get out of sync when behaviour changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add test cases ? Copy/paste is fine for initial tests. Please add a comment to keep tests in sync if updated on the group of tests.
This updates the web_ui implementation of lerpDouble to match the behaviour of the C++ engine implementation in dart:ui. Specifically this covers the following changes: * #20871: stricter handling of NaN and infinity * #20879: Improve the precision of lerpDouble lerpDouble: stricter handling of NaN and infinity (#20871) ---------------------------------------------------------- Previously, the behaviour of lerpDouble with respect to NaN and infinity was relatively complex and difficult to reason about. This patch simplifies the behaviour with respect to those conditions and adds documentation and tests. In general, if `a == b` or both values are null, infinite, or NaN, `a` is returned. Otherwise we require `a` and `b` and `t` to be finite or null and the result of the linear interpolation is returned. Improve the precision of lerpDouble (#20879) -------------------------------------------- Reduces errors caused by the loss of floating point precision when the two extrema of the lerp differ significantly in magnitude. Previously, we used the calculation: a + (b - a) * t When the difference in magnitude between `a` and `b` exceeds the precision representable by double-precision floating point math, `b - a` results in the larger-magnitude value of `a` or `b`. The error between the value produced and the correct value is then scaled by t. A simple example of the impact can be seen when `a` is significantly larger in magnitude than `b`. In that case, `b - a` results in `a` and when `t` is 1.0, the resulting value is `a - (a) * 1.0 == 0`. The patch transforms the computation to the mathematically-equivalent expression: a * (1.0 - t) + b * t By scaling each value independently, the behaviour is more accurate. From the point of view of performance, this adds an extra multiplication, but multiplication is relatively cheap and the behaviour is significantly better. This patch also adds a `precisionErrorTolerance` constant to test_utils.dart and migrates existing tests to use `closeTo()` for testing. The tests themselves *do* currently use values that have an exact floating-point representation, but we should allow for flexibility in future implementation changes.
|
Updated w/ tests added back and updated for web. |
* 556cb23 Roll Skia from 6763a713f957 to d91cd6b5ee2b (3 revisions) (flutter/engine#20989) * b6a3c54 Roll Fuchsia Linux SDK from A0PKwETay... to gfAt63Ezd... (flutter/engine#21005) * cceb733 Roll Fuchsia Mac SDK from sih5f60Gt... to 9pfHLZEFU... (flutter/engine#21006) * 5539820 Roll Skia from d91cd6b5ee2b to a73a84f9b8e3 (1 revision) (flutter/engine#21007) * b4cc631 Roll Dart SDK from f3a9ca88b664 to e59935669cb0 (1 revision) (flutter/engine#21008) * 6cf0cc4 Roll Skia from a73a84f9b8e3 to d0fe7d37d678 (1 revision) (flutter/engine#21011) * 5b055bb Roll Skia from d0fe7d37d678 to 611a52108b01 (2 revisions) (flutter/engine#21012) * 575a519 Enable lazy-async-stacks by-default in all modes (Take 3) (flutter/engine#20895) * 040a794 Roll Fuchsia Mac SDK from 9pfHLZEFU... to tUwahggJ8... (flutter/engine#21013) * 22cca4c Roll Dart SDK from e59935669cb0 to f745f9447ddf (1 revision) (flutter/engine#21014) * 539cb69 Roll Fuchsia Linux SDK from gfAt63Ezd... to Ta3F40BV6... (flutter/engine#21015) * 7d927dd Roll Dart SDK from f745f9447ddf to b88c06c314f4 (1 revision) (flutter/engine#21016) * 09a5bf7 Tweak the mdns error message (flutter/engine#20991) * d0d9ce6 Roll Fuchsia Linux SDK from Ta3F40BV6... to coVjRTWSf... (flutter/engine#21018) * 808bb85 Roll Fuchsia Mac SDK from tUwahggJ8... to TyNHQXzNU... (flutter/engine#21019) * 5aa6921 Roll Skia from 611a52108b01 to cd54c8385c31 (2 revisions) (flutter/engine#21021) * e7d558f Roll Dart SDK from b88c06c314f4 to 33b6c95936e0 (2 revisions) (flutter/engine#21023) * af90dd3 Roll Skia from cd54c8385c31 to c0d3495e1ee2 (12 revisions) (flutter/engine#21024) * f0fb74b Avoid crashing and display error if the process cannot be prepared for JIT mode Dart VM. (flutter/engine#20980) * 6a6c23a Roll Skia from c0d3495e1ee2 to cf1a4f50121f (6 revisions) (flutter/engine#21026) * 716dce0 Android 30r3 (flutter/engine#21025) * 7431070 Roll Dart SDK from 33b6c95936e0 to a2c9cae4dcd8 (1 revision) (flutter/engine#21027) * cef383d Roll Skia from cf1a4f50121f to 04b9443274cf (2 revisions) (flutter/engine#21028) * cf8c6b8 Update web lerpDouble to match C++ behaviour (flutter/engine#21010) * 6866675 Roll Skia from 04b9443274cf to b8ae7fa12aa0 (1 revision) (flutter/engine#21030) * c538f40 Roll Dart SDK from a2c9cae4dcd8 to ffbfa2000435 (1 revision) (flutter/engine#21031) * 89d34b0 Roll Skia from b8ae7fa12aa0 to 445c8ebcb710 (1 revision) (flutter/engine#21032) * 7766d2e Roll Fuchsia Mac SDK from TyNHQXzNU... to Phn3nF_BJ... (flutter/engine#21034) * e3de8d0 Roll Fuchsia Linux SDK from coVjRTWSf... to eBus_y4DN... (flutter/engine#21035) * beb7df5 Roll Skia from 445c8ebcb710 to f9d5940fef55 (3 revisions) (flutter/engine#21037) * f3a17b6 Roll Skia from f9d5940fef55 to 81c6d6eeb4cf (1 revision) (flutter/engine#21038) * bbcc495 Roll Dart SDK from ffbfa2000435 to 2e838b7b4503 (2 revisions) (flutter/engine#21039) * 61062fd Roll Skia from 81c6d6eeb4cf to 81606b5d9774 (5 revisions) (flutter/engine#21041) * f7c7b41 Revert "Enable lazy-async-stacks by-default in all modes (Take 3) (#20895)" (flutter/engine#21043)
Description
This updates the web_ui implementation of lerpDouble to match the
behaviour of the C++ engine implementation in dart:ui.
Specifically this covers the following changes:
Tests
This adds tests for the web_ui implementation of
lerpDouble.Related Patches
lerpDouble: stricter handling of NaN and infinity (#20871)
Previously, the behaviour of lerpDouble with respect to NaN and infinity
was relatively complex and difficult to reason about. This patch
simplifies the behaviour with respect to those conditions and adds
documentation and tests.
In general, if
a == bor both values are null, infinite, or NaN,ais returned. Otherwise we require
aandbandtto be finite ornull and the result of the linear interpolation is returned.
Improve the precision of lerpDouble (#20879)
Reduces errors caused by the loss of floating point precision when the
two extrema of the lerp differ significantly in magnitude. Previously,
we used the calculation:
When the difference in magnitude between
aandbexceeds theprecision representable by double-precision floating point math,
b - aresults in the larger-magnitude value of
aorb. The error betweenthe value produced and the correct value is then scaled by t.
A simple example of the impact can be seen when
ais significantlylarger in magnitude than
b. In that case,b - aresults inaandwhen
tis 1.0, the resulting value isa - (a) * 1.0 == 0.The patch transforms the computation to the mathematically-equivalent
expression:
By scaling each value independently, the behaviour is more accurate.
From the point of view of performance, this adds an extra
multiplication, but multiplication is relatively cheap and the behaviour
is significantly better.
This patch also adds a
precisionErrorToleranceconstant totest_utils.dart and migrates existing tests to use
closeTo()fortesting.
The tests themselves do currently use values that have an exact
floating-point representation, but we should allow for flexibility in
future implementation changes.