[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.

Author: flar

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

impeller/core/texture_descriptor.h

@@ -83,7 +83,7 @@ struct TextureDescriptor {
 
   constexpr bool IsValid() const {
     return format != PixelFormat::kUnknown &&  //
-           size.IsPositive() &&                //
+           !size.IsEmpty() &&                  //
            mip_count >= 1u &&                  //
            SamplingOptionsAreValid();
   }

impeller/geometry/BUILD.gn

@@ -63,6 +63,7 @@ impeller_component("geometry_unittests") {
     "geometry_unittests.cc",
     "matrix_unittests.cc",
     "rect_unittests.cc",
+    "size_unittests.cc",
   ]
 
   deps = [

impeller/geometry/rect.h

@@ -123,8 +123,7 @@ struct TRect {
     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(); }

impeller/geometry/rect_unittests.cc

@@ -55,5 +55,46 @@ TEST(RectTest, RectMakeSize) {
   }
 }
 
+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

impeller/geometry/size.h

@@ -96,13 +96,8 @@ struct TSize {
 
   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 @@ struct TSize {
 
   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)));

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

impeller/image/decompressed_image.cc

@@ -18,7 +18,7 @@ DecompressedImage::DecompressedImage(
     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;

impeller/renderer/snapshot.cc

@@ -16,7 +16,7 @@ std::optional<Rect> Snapshot::GetCoverage() const {
 }
 
 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())) *