[Impeller] Make IsEmpty methods on Rect and Size NaN-aware (#47725)
Removes unused or redundant overloads and fixes the IsEmpty method on Size to be both simpler (2 comparisons vs. 4) and NaN-aware. Rect automatically uses the new code via delegation to the Size object.
diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files
index 3981ef1..18a25c8 100644
--- a/ci/licenses_golden/excluded_files
+++ b/ci/licenses_golden/excluded_files
@@ -150,6 +150,7 @@
../../../flutter/impeller/geometry/geometry_unittests.cc
../../../flutter/impeller/geometry/matrix_unittests.cc
../../../flutter/impeller/geometry/rect_unittests.cc
+../../../flutter/impeller/geometry/size_unittests.cc
../../../flutter/impeller/golden_tests/README.md
../../../flutter/impeller/golden_tests_harvester/.dart_tool
../../../flutter/impeller/golden_tests_harvester/.gitignore
diff --git a/impeller/core/texture_descriptor.h b/impeller/core/texture_descriptor.h
index feb012b..aa1b574 100644
--- a/impeller/core/texture_descriptor.h
+++ b/impeller/core/texture_descriptor.h
@@ -83,7 +83,7 @@
constexpr bool IsValid() const {
return format != PixelFormat::kUnknown && //
- size.IsPositive() && //
+ !size.IsEmpty() && //
mip_count >= 1u && //
SamplingOptionsAreValid();
}
diff --git a/impeller/geometry/BUILD.gn b/impeller/geometry/BUILD.gn
index 0ce6382..2ee3221 100644
--- a/impeller/geometry/BUILD.gn
+++ b/impeller/geometry/BUILD.gn
@@ -63,6 +63,7 @@
"geometry_unittests.cc",
"matrix_unittests.cc",
"rect_unittests.cc",
+ "size_unittests.cc",
]
deps = [
diff --git a/impeller/geometry/rect.h b/impeller/geometry/rect.h
index c40ce26..fa33757 100644
--- a/impeller/geometry/rect.h
+++ b/impeller/geometry/rect.h
@@ -123,8 +123,7 @@
return Union(o).size == size;
}
- constexpr bool IsZero() const { return size.IsZero(); }
-
+ /// Returns true if either of the width or height are 0, negative, or NaN.
constexpr bool IsEmpty() const { return size.IsEmpty(); }
constexpr bool IsMaximum() const { return *this == MakeMaximum(); }
diff --git a/impeller/geometry/rect_unittests.cc b/impeller/geometry/rect_unittests.cc
index 583c04f..27bedfa 100644
--- a/impeller/geometry/rect_unittests.cc
+++ b/impeller/geometry/rect_unittests.cc
@@ -55,5 +55,46 @@
}
}
+TEST(SizeTest, RectIsEmpty) {
+ auto nan = std::numeric_limits<Scalar>::quiet_NaN();
+
+ // Non-empty
+ EXPECT_FALSE(Rect::MakeXYWH(1.5, 2.3, 10.5, 7.2).IsEmpty());
+
+ // Empty both width and height both 0 or negative, in all combinations
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, 0.0, 0.0).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, -1.0, -1.0).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, 0.0, -1.0).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, -1.0, 0.0).IsEmpty());
+
+ // Empty for 0 or negative width or height (but not both at the same time)
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, 10.5, 0.0).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, 10.5, -1.0).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, 0.0, 7.2).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, -1.0, 7.2).IsEmpty());
+
+ // Empty for NaN in width or height or both
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, 10.5, nan).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, nan, 7.2).IsEmpty());
+ EXPECT_TRUE(Rect::MakeXYWH(1.5, 2.3, nan, nan).IsEmpty());
+}
+
+TEST(SizeTest, IRectIsEmpty) {
+ // Non-empty
+ EXPECT_FALSE(IRect::MakeXYWH(1, 2, 10, 7).IsEmpty());
+
+ // Empty both width and height both 0 or negative, in all combinations
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, 0, 0).IsEmpty());
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, -1, -1).IsEmpty());
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, -1, 0).IsEmpty());
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, 0, -1).IsEmpty());
+
+ // Empty for 0 or negative width or height (but not both at the same time)
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, 10, 0).IsEmpty());
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, 10, -1).IsEmpty());
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, 0, 7).IsEmpty());
+ EXPECT_TRUE(IRect::MakeXYWH(1, 2, -1, 7).IsEmpty());
+}
+
} // namespace testing
} // namespace impeller
diff --git a/impeller/geometry/size.h b/impeller/geometry/size.h
index 680e205..7ee34d1 100644
--- a/impeller/geometry/size.h
+++ b/impeller/geometry/size.h
@@ -96,13 +96,8 @@
constexpr Type Area() const { return width * height; }
- constexpr bool IsPositive() const { return width > 0 && height > 0; }
-
- constexpr bool IsNegative() const { return width < 0 || height < 0; }
-
- constexpr bool IsZero() const { return width == 0 || height == 0; }
-
- constexpr bool IsEmpty() const { return IsNegative() || IsZero(); }
+ /// Returns true if either of the width or height are 0, negative, or NaN.
+ constexpr bool IsEmpty() const { return !(width > 0 && height > 0); }
template <class U>
static constexpr TSize Ceil(const TSize<U>& other) {
@@ -112,7 +107,7 @@
constexpr size_t MipCount() const {
constexpr size_t minimum_mip = 1u;
- if (!IsPositive()) {
+ if (IsEmpty()) {
return minimum_mip;
}
size_t result = std::max(ceil(log2(width)), ceil(log2(height)));
diff --git a/impeller/geometry/size_unittests.cc b/impeller/geometry/size_unittests.cc
new file mode 100644
index 0000000..df8d26b
--- /dev/null
+++ b/impeller/geometry/size_unittests.cc
@@ -0,0 +1,54 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "gtest/gtest.h"
+
+#include "flutter/impeller/geometry/size.h"
+
+namespace impeller {
+namespace testing {
+
+TEST(SizeTest, SizeIsEmpty) {
+ auto nan = std::numeric_limits<Scalar>::quiet_NaN();
+
+ // Non-empty
+ EXPECT_FALSE(Size(10.5, 7.2).IsEmpty());
+
+ // Empty both width and height both 0 or negative, in all combinations
+ EXPECT_TRUE(Size(0.0, 0.0).IsEmpty());
+ EXPECT_TRUE(Size(-1.0, -1.0).IsEmpty());
+ EXPECT_TRUE(Size(-1.0, 0.0).IsEmpty());
+ EXPECT_TRUE(Size(0.0, -1.0).IsEmpty());
+
+ // Empty for 0 or negative width or height (but not both at the same time)
+ EXPECT_TRUE(Size(10.5, 0.0).IsEmpty());
+ EXPECT_TRUE(Size(10.5, -1.0).IsEmpty());
+ EXPECT_TRUE(Size(0.0, 7.2).IsEmpty());
+ EXPECT_TRUE(Size(-1.0, 7.2).IsEmpty());
+
+ // Empty for NaN in width or height or both
+ EXPECT_TRUE(Size(10.5, nan).IsEmpty());
+ EXPECT_TRUE(Size(nan, 7.2).IsEmpty());
+ EXPECT_TRUE(Size(nan, nan).IsEmpty());
+}
+
+TEST(SizeTest, ISizeIsEmpty) {
+ // Non-empty
+ EXPECT_FALSE(ISize(10, 7).IsEmpty());
+
+ // Empty both width and height both 0 or negative, in all combinations
+ EXPECT_TRUE(ISize(0, 0).IsEmpty());
+ EXPECT_TRUE(ISize(-1, -1).IsEmpty());
+ EXPECT_TRUE(ISize(-1, 0).IsEmpty());
+ EXPECT_TRUE(ISize(0, -1).IsEmpty());
+
+ // Empty for 0 or negative width or height (but not both at the same time)
+ EXPECT_TRUE(ISize(10, 0).IsEmpty());
+ EXPECT_TRUE(ISize(10, -1).IsEmpty());
+ EXPECT_TRUE(ISize(0, 7).IsEmpty());
+ EXPECT_TRUE(ISize(-1, 7).IsEmpty());
+}
+
+} // namespace testing
+} // namespace impeller
diff --git a/impeller/image/decompressed_image.cc b/impeller/image/decompressed_image.cc
index c31c184..396aada 100644
--- a/impeller/image/decompressed_image.cc
+++ b/impeller/image/decompressed_image.cc
@@ -18,7 +18,7 @@
Format format,
std::shared_ptr<const fml::Mapping> allocation)
: size_(size), format_(format), allocation_(std::move(allocation)) {
- if (!allocation_ || !size.IsPositive() || format_ == Format::kInvalid) {
+ if (!allocation_ || size.IsEmpty() || format_ == Format::kInvalid) {
return;
}
is_valid_ = true;
diff --git a/impeller/renderer/snapshot.cc b/impeller/renderer/snapshot.cc
index a73a210..b55a971 100644
--- a/impeller/renderer/snapshot.cc
+++ b/impeller/renderer/snapshot.cc
@@ -16,7 +16,7 @@
}
std::optional<Matrix> Snapshot::GetUVTransform() const {
- if (!texture || texture->GetSize().IsZero()) {
+ if (!texture || texture->GetSize().IsEmpty()) {
return std::nullopt;
}
return Matrix::MakeScale(1 / Vector2(texture->GetSize())) *
