From 67ee7c76feb7bf87fd9fc313f2cc65f1eedc1490 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 12 Mar 2024 13:59:08 -0700 Subject: [PATCH] [Impeller] Add utility for type safe masks. (flutter/engine#51361) We use and recommend type safe enums. But when it comes to masks of those enums (`TextureUsageMask`, `ColorWriteMask`, etc.), these are all untyped and you can create one either from a scalar of the enums underlying type, or using ugly and error prone static casts. At the callee, there is no type checking and another error prone static cast. This patch adds a type-safe mask type. This should allow for the migration of something like: ```c++ using TextureUsageMask = uint64_t; ``` with ```c++ using TextureUsageMask = Mask; ``` Subsequently, all static casts can be removed with full type checking throughout. This doesn't migrate existing uses yet. That will come in a separate patch. --- .../ci/licenses_golden/licenses_flutter | 2 + engine/src/flutter/impeller/base/BUILD.gn | 1 + .../flutter/impeller/base/base_unittests.cc | 194 +++++++++++++++++ engine/src/flutter/impeller/base/mask.h | 195 ++++++++++++++++++ 4 files changed, 392 insertions(+) create mode 100644 engine/src/flutter/impeller/base/mask.h diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 11225ea34e..0b1d147095 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -39503,6 +39503,7 @@ ORIGIN: ../../../flutter/impeller/base/backend_cast.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/base/comparable.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/base/comparable.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/base/config.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/base/mask.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/base/promise.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/base/promise.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/base/strings.cc + ../../../flutter/LICENSE @@ -42354,6 +42355,7 @@ FILE: ../../../flutter/impeller/base/backend_cast.h FILE: ../../../flutter/impeller/base/comparable.cc FILE: ../../../flutter/impeller/base/comparable.h FILE: ../../../flutter/impeller/base/config.h +FILE: ../../../flutter/impeller/base/mask.h FILE: ../../../flutter/impeller/base/promise.cc FILE: ../../../flutter/impeller/base/promise.h FILE: ../../../flutter/impeller/base/strings.cc diff --git a/engine/src/flutter/impeller/base/BUILD.gn b/engine/src/flutter/impeller/base/BUILD.gn index c021a96e35..03ee14f645 100644 --- a/engine/src/flutter/impeller/base/BUILD.gn +++ b/engine/src/flutter/impeller/base/BUILD.gn @@ -12,6 +12,7 @@ impeller_component("base") { "comparable.cc", "comparable.h", "config.h", + "mask.h", "promise.cc", "promise.h", "strings.cc", diff --git a/engine/src/flutter/impeller/base/base_unittests.cc b/engine/src/flutter/impeller/base/base_unittests.cc index db2c97852b..5653809110 100644 --- a/engine/src/flutter/impeller/base/base_unittests.cc +++ b/engine/src/flutter/impeller/base/base_unittests.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "flutter/testing/testing.h" +#include "impeller/base/mask.h" #include "impeller/base/promise.h" #include "impeller/base/strings.h" #include "impeller/base/thread.h" @@ -250,5 +251,198 @@ TEST(BaseTest, NoExceptionPromiseEmpty) { wrapper.reset(); } +enum class MyMaskBits : uint32_t { + kFoo = 0, + kBar = 1 << 0, + kBaz = 1 << 1, + kBang = 1 << 2, +}; + +using MyMask = Mask; + +TEST(BaseTest, CanUseTypedMasks) { + { + MyMask mask; + ASSERT_EQ(static_cast(mask), 0u); + ASSERT_FALSE(mask); + } + + { + MyMask mask(MyMaskBits::kBar); + ASSERT_EQ(static_cast(mask), 1u); + ASSERT_TRUE(mask); + } + + { + MyMask mask2(MyMaskBits::kBar); + MyMask mask(mask2); + ASSERT_EQ(static_cast(mask), 1u); + ASSERT_TRUE(mask); + } + + { + MyMask mask2(MyMaskBits::kBar); + MyMask mask(std::move(mask2)); // NOLINT + ASSERT_EQ(static_cast(mask), 1u); + ASSERT_TRUE(mask); + } + + ASSERT_LT(MyMaskBits::kBar, MyMaskBits::kBaz); + ASSERT_LE(MyMaskBits::kBar, MyMaskBits::kBaz); + ASSERT_GT(MyMaskBits::kBaz, MyMaskBits::kBar); + ASSERT_GE(MyMaskBits::kBaz, MyMaskBits::kBar); + ASSERT_EQ(MyMaskBits::kBaz, MyMaskBits::kBaz); + ASSERT_NE(MyMaskBits::kBaz, MyMaskBits::kBang); + + { + MyMask m1(MyMaskBits::kBar); + MyMask m2(MyMaskBits::kBaz); + ASSERT_EQ(static_cast(m1 & m2), 0u); + ASSERT_FALSE(m1 & m2); + } + + { + MyMask m1(MyMaskBits::kBar); + MyMask m2(MyMaskBits::kBaz); + ASSERT_EQ(static_cast(m1 | m2), ((1u << 0u) | (1u << 1u))); + ASSERT_TRUE(m1 | m2); + } + + { + MyMask m1(MyMaskBits::kBar); + MyMask m2(MyMaskBits::kBaz); + ASSERT_EQ(static_cast(m1 ^ m2), ((1u << 0u) ^ (1u << 1u))); + ASSERT_TRUE(m1 ^ m2); + } + + { + MyMask m1(MyMaskBits::kBar); + ASSERT_EQ(static_cast(~m1), (~(1u << 0u))); + ASSERT_TRUE(m1); + } + + { + MyMask m1 = MyMaskBits::kBar; + MyMask m2 = MyMaskBits::kBaz; + m2 = m1; + ASSERT_EQ(m2, MyMaskBits::kBar); + } + + { + MyMask m = MyMaskBits::kBar | MyMaskBits::kBaz; + ASSERT_TRUE(m); + } + + { + MyMask m = MyMaskBits::kBar & MyMaskBits::kBaz; + ASSERT_FALSE(m); + } + + { + MyMask m = MyMaskBits::kBar ^ MyMaskBits::kBaz; + ASSERT_TRUE(m); + } + + { + MyMask m = ~MyMaskBits::kBar; + ASSERT_TRUE(m); + } + + { + MyMask m1 = MyMaskBits::kBar; + MyMask m2 = MyMaskBits::kBaz; + m2 |= m1; + ASSERT_EQ(m1, MyMaskBits::kBar); + MyMask pred = MyMaskBits::kBar | MyMaskBits::kBaz; + ASSERT_EQ(m2, pred); + } + + { + MyMask m1 = MyMaskBits::kBar; + MyMask m2 = MyMaskBits::kBaz; + m2 &= m1; + ASSERT_EQ(m1, MyMaskBits::kBar); + MyMask pred = MyMaskBits::kBar & MyMaskBits::kBaz; + ASSERT_EQ(m2, pred); + } + + { + MyMask m1 = MyMaskBits::kBar; + MyMask m2 = MyMaskBits::kBaz; + m2 ^= m1; + ASSERT_EQ(m1, MyMaskBits::kBar); + MyMask pred = MyMaskBits::kBar ^ MyMaskBits::kBaz; + ASSERT_EQ(m2, pred); + } + + { + MyMask x = MyMaskBits::kBar; + MyMask m = x | MyMaskBits::kBaz; + ASSERT_TRUE(m); + } + + { + MyMask x = MyMaskBits::kBar; + MyMask m = MyMaskBits::kBaz | x; + ASSERT_TRUE(m); + } + + { + MyMask x = MyMaskBits::kBar; + MyMask m = x & MyMaskBits::kBaz; + ASSERT_FALSE(m); + } + + { + MyMask x = MyMaskBits::kBar; + MyMask m = MyMaskBits::kBaz & x; + ASSERT_FALSE(m); + } + + { + MyMask x = MyMaskBits::kBar; + MyMask m = x ^ MyMaskBits::kBaz; + ASSERT_TRUE(m); + } + + { + MyMask x = MyMaskBits::kBar; + MyMask m = MyMaskBits::kBaz ^ x; + ASSERT_TRUE(m); + } + + { + MyMask x = MyMaskBits::kBar; + MyMask m = ~x; + ASSERT_TRUE(m); + } + + { + MyMaskBits x = MyMaskBits::kBar; + MyMask m = MyMaskBits::kBaz; + ASSERT_TRUE(x < m); + ASSERT_TRUE(x <= m); + } + + { + MyMaskBits x = MyMaskBits::kBar; + MyMask m = MyMaskBits::kBaz; + ASSERT_FALSE(x == m); + } + + { + MyMaskBits x = MyMaskBits::kBar; + MyMask m = MyMaskBits::kBaz; + ASSERT_TRUE(x != m); + } + + { + MyMaskBits x = MyMaskBits::kBar; + MyMask m = MyMaskBits::kBaz; + ASSERT_FALSE(x > m); + ASSERT_FALSE(x >= m); + } +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/base/mask.h b/engine/src/flutter/impeller/base/mask.h new file mode 100644 index 0000000000..5703c1507c --- /dev/null +++ b/engine/src/flutter/impeller/base/mask.h @@ -0,0 +1,195 @@ +// 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. + +#ifndef FLUTTER_IMPELLER_BASE_MASK_H_ +#define FLUTTER_IMPELLER_BASE_MASK_H_ + +#include + +namespace impeller { + +//------------------------------------------------------------------------------ +/// @brief A mask of typed enums. +/// +/// @tparam EnumType_ The type of the enum. Must be an enum class. +/// +template +struct Mask { + using EnumType = EnumType_; + using MaskType = typename std::underlying_type::type; + + constexpr Mask() = default; + + constexpr Mask(const Mask& other) = default; + + constexpr Mask(Mask&& other) = default; + + constexpr Mask(EnumType type) // NOLINT(google-explicit-constructor) + : mask_(static_cast(type)) {} + + explicit constexpr Mask(MaskType mask) : mask_(static_cast(mask)) {} + + // All casts must be explicit. + + explicit constexpr operator MaskType() const { return mask_; } + + explicit constexpr operator bool() const { return !!mask_; } + + // The following relational operators can be replaced with a defaulted + // spaceship operator post C++20. + + constexpr bool operator<(const Mask& other) const { + return mask_ < other.mask_; + } + + constexpr bool operator>(const Mask& other) const { + return mask_ > other.mask_; + } + + constexpr bool operator>=(const Mask& other) const { + return mask_ >= other.mask_; + } + + constexpr bool operator<=(const Mask& other) const { + return mask_ <= other.mask_; + } + + constexpr bool operator==(const Mask& other) const { + return mask_ == other.mask_; + } + + constexpr bool operator!=(const Mask& other) const { + return mask_ != other.mask_; + } + + // Logical operators. + + constexpr bool operator!() const { return !mask_; } + + // Bitwise operators. + + constexpr Mask operator&(const Mask& other) const { + return Mask{mask_ & other.mask_}; + } + + constexpr Mask operator|(const Mask& other) const { + return Mask{mask_ | other.mask_}; + } + + constexpr Mask operator^(const Mask& other) const { + return Mask{mask_ ^ other.mask_}; + } + + constexpr Mask operator~() const { return Mask{~mask_}; } + + // Assignment operators. + + constexpr Mask& operator=(const Mask&) = default; + + constexpr Mask& operator=(Mask&&) = default; + + constexpr Mask& operator|=(const Mask& other) { + mask_ |= other.mask_; + return *this; + } + + constexpr Mask& operator&=(const Mask& other) { + mask_ &= other.mask_; + return *this; + } + + constexpr Mask& operator^=(const Mask& other) { + mask_ ^= other.mask_; + return *this; + } + + private: + MaskType mask_ = {}; +}; + +// Construction from Enum Types + +template +inline constexpr Mask operator|(const EnumType& lhs, + const EnumType& rhs) { + return Mask{lhs} | rhs; +} + +template +inline constexpr Mask operator&(const EnumType& lhs, + const EnumType& rhs) { + return Mask{lhs} & rhs; +} + +template +inline constexpr Mask operator^(const EnumType& lhs, + const EnumType& rhs) { + return Mask{lhs} ^ rhs; +} + +template +inline constexpr Mask operator~(const EnumType& other) { + return ~Mask{other}; +} + +template +inline constexpr Mask operator|(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} | rhs; +} + +template +inline constexpr Mask operator&(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} & rhs; +} + +template +inline constexpr Mask operator^(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} ^ rhs; +} + +// Relational operators with EnumType promotion. These can be replaced by a +// defaulted spaceship operator post C++20. + +template +inline constexpr bool operator<(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} < rhs; +} + +template +inline constexpr bool operator>(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} > rhs; +} + +template +inline constexpr bool operator<=(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} <= rhs; +} + +template +inline constexpr bool operator>=(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} >= rhs; +} + +template +inline constexpr bool operator==(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} == rhs; +} + +template +inline constexpr bool operator!=(const EnumType& lhs, + const Mask& rhs) { + return Mask{lhs} != rhs; +} + +} // namespace impeller + +#endif // FLUTTER_IMPELLER_BASE_MASK_H_