Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 11f2cb3

Browse files
authored
[Impeller] NumberNear implements precision-based comparisons (#52001)
Fixes flutter/flutter#146455 Fuzzy comparisons based on the IEEE floating precision of the numbers being compared will help ensure that comparing large numbers and very small numbers (in unit tests) both have similar accuracy.
1 parent 490c25a commit 11f2cb3

File tree

5 files changed

+68
-37
lines changed

5 files changed

+68
-37
lines changed

impeller/geometry/geometry_asserts.h

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,41 @@
1616
#include "impeller/geometry/vector.h"
1717

1818
inline bool NumberNear(double a, double b) {
19-
static const double epsilon = 1e-3;
20-
return (a > (b - epsilon)) && (a < (b + epsilon));
19+
if (a == b) {
20+
return true;
21+
}
22+
if (std::isnan(a) || std::isnan(b)) {
23+
return false;
24+
}
25+
26+
// We used to compare based on an absolute difference of 1e-3 which
27+
// would allow up to 10 bits of mantissa difference in a float
28+
// (leaving 14 bits of accuracy being tested). Some numbers in the tests
29+
// will fail with a bit difference of up to 19 (a little over 4 bits) even
30+
// though the numbers print out identically using the float ostream output
31+
// at the default output precision. Choosing a max "units of least precision"
32+
// of 32 allows up to 5 bits of imprecision.
33+
static constexpr float kImpellerTestingMaxULP = 32;
34+
35+
// We also impose a minimum step size so that cases of comparing numbers
36+
// very close to 0.0 don't compute a huge number of ULPs due to the ever
37+
// increasing precision near 0. This value is approximately the step size
38+
// of numbers going less than 1.0f.
39+
static constexpr float kMinimumULPStep = (1.0f / (1 << 24));
40+
41+
auto adjust_step = [](float v) {
42+
return (std::abs(v) < kMinimumULPStep) ? std::copysignf(kMinimumULPStep, v)
43+
: v;
44+
};
45+
46+
float step_ab = adjust_step(a - std::nexttowardf(a, b));
47+
float step_ba = adjust_step(b - std::nexttowardf(b, a));
48+
49+
float ab_ulps = (a - b) / step_ab;
50+
float ba_ulps = (b - a) / step_ba;
51+
FML_CHECK(ab_ulps >= 0 && ba_ulps >= 0);
52+
53+
return (std::min(ab_ulps, ba_ulps) < kImpellerTestingMaxULP);
2154
}
2255

2356
inline ::testing::AssertionResult MatrixNear(impeller::Matrix a,

impeller/geometry/geometry_unittests.cc

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,25 @@ TEST(GeometryTest, MakeRow) {
6666

6767
TEST(GeometryTest, RotationMatrix) {
6868
auto rotation = Matrix::MakeRotationZ(Radians{kPiOver4});
69-
auto expect = Matrix{0.707, 0.707, 0, 0, //
70-
-0.707, 0.707, 0, 0, //
71-
0, 0, 1, 0, //
72-
0, 0, 0, 1};
69+
// clang-format off
70+
auto expect = Matrix{k1OverSqrt2, k1OverSqrt2, 0, 0,
71+
-k1OverSqrt2, k1OverSqrt2, 0, 0,
72+
0, 0, 1, 0,
73+
0, 0, 0, 1};
74+
// clang-format on
7375
ASSERT_MATRIX_NEAR(rotation, expect);
7476
}
7577

7678
TEST(GeometryTest, InvertMultMatrix) {
7779
{
7880
auto rotation = Matrix::MakeRotationZ(Radians{kPiOver4});
7981
auto invert = rotation.Invert();
80-
auto expect = Matrix{0.707, -0.707, 0, 0, //
81-
0.707, 0.707, 0, 0, //
82-
0, 0, 1, 0, //
83-
0, 0, 0, 1};
82+
// clang-format off
83+
auto expect = Matrix{k1OverSqrt2, -k1OverSqrt2, 0, 0,
84+
k1OverSqrt2, k1OverSqrt2, 0, 0,
85+
0, 0, 1, 0,
86+
0, 0, 0, 1};
87+
// clang-format on
8488
ASSERT_MATRIX_NEAR(invert, expect);
8589
}
8690
{

impeller/geometry/rect_unittests.cc

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,22 +2895,6 @@ TEST(RectTest, IRectRound) {
28952895
}
28962896
}
28972897

2898-
// EXPECT_RECT_NEAR will allow a fixed difference between the values that
2899-
// assumes a compatible range of values being tested. Some of the values
2900-
// below are well outside that range and so if the values are a single
2901-
// "bit" off, then they will differ by far more than the fixed allowable
2902-
// difference. The EXPECT_FLOAT_EQ macro will compare the values and
2903-
// fail only if their difference in mantissa is in the last N bits,
2904-
// which is a range agnostic way to compare floats with allowances for
2905-
// bit errors in computations.
2906-
#define EXPECT_RECT_EQ(a, b) \
2907-
do { \
2908-
EXPECT_FLOAT_EQ(a.GetLeft(), b.GetLeft()); \
2909-
EXPECT_FLOAT_EQ(a.GetTop(), b.GetTop()); \
2910-
EXPECT_FLOAT_EQ(a.GetRight(), b.GetRight()); \
2911-
EXPECT_FLOAT_EQ(a.GetBottom(), b.GetBottom()); \
2912-
} while (0)
2913-
29142898
TEST(RectTest, TransformAndClipBounds) {
29152899
{
29162900
// This matrix should clip no corners.
@@ -2961,7 +2945,7 @@ TEST(RectTest, TransformAndClipBounds) {
29612945

29622946
Rect expect = Rect::MakeLTRB(142.85715f, 142.85715f, 6553600.f, 6553600.f);
29632947
EXPECT_FALSE(src.TransformAndClipBounds(matrix).IsEmpty());
2964-
EXPECT_RECT_EQ(src.TransformAndClipBounds(matrix), expect);
2948+
EXPECT_RECT_NEAR(src.TransformAndClipBounds(matrix), expect);
29652949
}
29662950

29672951
{
@@ -2987,7 +2971,7 @@ TEST(RectTest, TransformAndClipBounds) {
29872971

29882972
Rect expect = Rect::MakeLTRB(222.2222f, 222.2222f, 5898373.f, 6553600.f);
29892973
EXPECT_FALSE(src.TransformAndClipBounds(matrix).IsEmpty());
2990-
EXPECT_RECT_EQ(src.TransformAndClipBounds(matrix), expect);
2974+
EXPECT_RECT_NEAR(src.TransformAndClipBounds(matrix), expect);
29912975
}
29922976

29932977
{
@@ -3013,7 +2997,7 @@ TEST(RectTest, TransformAndClipBounds) {
30132997

30142998
Rect expect = Rect::MakeLTRB(499.99988f, 499.99988f, 5898340.f, 4369400.f);
30152999
EXPECT_FALSE(src.TransformAndClipBounds(matrix).IsEmpty());
3016-
EXPECT_RECT_EQ(src.TransformAndClipBounds(matrix), expect);
3000+
EXPECT_RECT_NEAR(src.TransformAndClipBounds(matrix), expect);
30173001
}
30183002

30193003
{

impeller/scene/importer/importer_unittests.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ TEST(ImporterTest, CanParseUnskinnedGLTF) {
4444
ASSERT_VECTOR3_NEAR(position, Vector3(-0.0100185, -0.522907, 0.133178));
4545

4646
Vector3 normal = ToVector3(vertex.normal());
47-
ASSERT_VECTOR3_NEAR(normal, Vector3(0.556997, -0.810833, 0.179733));
47+
ASSERT_VECTOR3_NEAR(normal, Vector3(0.556984, -0.810839, 0.179746));
4848

4949
Vector4 tangent = ToVector4(vertex.tangent());
50-
ASSERT_VECTOR4_NEAR(tangent, Vector4(0.155901, -0.110485, -0.981574, 1));
50+
ASSERT_VECTOR4_NEAR(tangent, Vector4(0.155911, -0.110495, -0.981572, 1));
5151

5252
Vector2 texture_coords = ToVector2(vertex.texture_coords());
5353
ASSERT_POINT_NEAR(texture_coords, Vector2(0.727937, 0.713817));

impeller/tessellator/tessellator_unittests.cc

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ TEST(TessellatorTest, FilledCircleTessellationVertices) {
187187
double angle = kPiOver2 * i / (quadrant_count - 1);
188188
double degrees = angle * 180.0 / kPi;
189189
double rsin = sin(angle) * radius;
190-
double rcos = cos(angle) * radius;
190+
// Note that cos(radians(90 degrees)) isn't exactly 0.0 like it should be
191+
double rcos = (i == quadrant_count - 1) ? 0.0f : cos(angle) * radius;
191192
EXPECT_POINT_NEAR(vertices[i * 2],
192193
Point(center.x - rcos, center.y + rsin))
193194
<< "vertex " << i << ", angle = " << degrees << std::endl;
@@ -234,7 +235,9 @@ TEST(TessellatorTest, StrokedCircleTessellationVertices) {
234235
double angle = kPiOver2 * i / (quadrant_count - 1);
235236
double degrees = angle * 180.0 / kPi;
236237
double rsin = sin(angle) * (radius + half_width);
237-
double rcos = cos(angle) * (radius + half_width);
238+
// Note that cos(radians(90 degrees)) isn't exactly 0.0 like it should be
239+
double rcos =
240+
(i == quadrant_count - 1) ? 0.0f : cos(angle) * (radius + half_width);
238241
EXPECT_POINT_NEAR(vertices[i * 2],
239242
Point(center.x - rcos, center.y - rsin))
240243
<< "vertex " << i << ", angle = " << degrees << std::endl;
@@ -254,7 +257,9 @@ TEST(TessellatorTest, StrokedCircleTessellationVertices) {
254257
double angle = kPiOver2 * i / (quadrant_count - 1);
255258
double degrees = angle * 180.0 / kPi;
256259
double rsin = sin(angle) * (radius - half_width);
257-
double rcos = cos(angle) * (radius - half_width);
260+
// Note that cos(radians(90 degrees)) isn't exactly 0.0 like it should be
261+
double rcos =
262+
(i == quadrant_count - 1) ? 0.0f : cos(angle) * (radius - half_width);
258263
EXPECT_POINT_NEAR(vertices[i * 2 + 1],
259264
Point(center.x - rcos, center.y - rsin))
260265
<< "vertex " << i << ", angle = " << degrees << std::endl;
@@ -306,7 +311,9 @@ TEST(TessellatorTest, RoundCapLineTessellationVertices) {
306311
for (size_t i = 0; i < quadrant_count; i++) {
307312
double angle = kPiOver2 * i / (quadrant_count - 1);
308313
double degrees = angle * 180.0 / kPi;
309-
Point relative_along = along * cos(angle);
314+
// Note that cos(radians(90 degrees)) isn't exactly 0.0 like it should be
315+
Point relative_along =
316+
along * ((i == quadrant_count - 1) ? 0.0f : cos(angle));
310317
Point relative_across = across * sin(angle);
311318
EXPECT_POINT_NEAR(vertices[i * 2], //
312319
p0 - relative_along + relative_across)
@@ -370,7 +377,9 @@ TEST(TessellatorTest, FilledEllipseTessellationVertices) {
370377
for (size_t i = 0; i < quadrant_count; i++) {
371378
double angle = kPiOver2 * i / (quadrant_count - 1);
372379
double degrees = angle * 180.0 / kPi;
373-
double rcos = cos(angle) * half_size.width;
380+
// Note that cos(radians(90 degrees)) isn't exactly 0.0 like it should be
381+
double rcos =
382+
(i == quadrant_count - 1) ? 0.0f : cos(angle) * half_size.width;
374383
double rsin = sin(angle) * half_size.height;
375384
EXPECT_POINT_NEAR(vertices[i * 2],
376385
Point(center.x - rcos, center.y + rsin))
@@ -435,7 +444,8 @@ TEST(TessellatorTest, FilledRoundRectTessellationVertices) {
435444
for (size_t i = 0; i < quadrant_count; i++) {
436445
double angle = kPiOver2 * i / (quadrant_count - 1);
437446
double degrees = angle * 180.0 / kPi;
438-
double rcos = cos(angle) * radii.width;
447+
// Note that cos(radians(90 degrees)) isn't exactly 0.0 like it should be
448+
double rcos = (i == quadrant_count - 1) ? 0.0f : cos(angle) * radii.width;
439449
double rsin = sin(angle) * radii.height;
440450
EXPECT_POINT_NEAR(vertices[i * 2],
441451
Point(middle_left - rcos, middle_bottom + rsin))

0 commit comments

Comments
 (0)