From bba589bf364810541f7e043291dd40944c4b3702 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 12 Mar 2024 14:31:46 -0700 Subject: [PATCH 1/2] [Impeller] Make masks type safe. Uses the utility added in https://github.com/flutter/engine/pull/51361 --- impeller/base/base_unittests.cc | 21 ++++--- impeller/base/mask.h | 61 +++++++++++++++---- impeller/core/formats.cc | 6 +- impeller/core/formats.h | 26 ++++---- impeller/core/texture_descriptor.h | 3 +- impeller/entity/contents/content_context.cc | 3 +- .../entity/entity_pass_target_unittests.cc | 2 +- .../renderer/backend/gles/render_pass_gles.cc | 16 +++-- .../renderer/backend/gles/surface_gles.cc | 5 +- .../renderer/backend/gles/texture_gles.cc | 3 +- impeller/renderer/backend/metal/formats_mtl.h | 13 ++-- .../renderer/backend/metal/formats_mtl.mm | 8 +-- .../renderer/backend/metal/surface_mtl.mm | 6 +- .../renderer/backend/vulkan/allocator_vk.cc | 6 +- impeller/renderer/backend/vulkan/formats_vk.h | 13 ++-- .../renderer/backend/vulkan/render_pass_vk.cc | 2 +- .../vulkan/swapchain/khr/khr_surface_vk.cc | 5 +- .../swapchain/khr/khr_swapchain_impl_vk.cc | 7 +-- impeller/renderer/render_target.cc | 12 ++-- impeller/renderer/renderer_unittests.cc | 16 ++--- impeller/scene/scene_encoder.cc | 3 +- lib/gpu/texture.cc | 2 +- shell/platform/embedder/embedder.cc | 8 +-- 23 files changed, 130 insertions(+), 117 deletions(-) diff --git a/impeller/base/base_unittests.cc b/impeller/base/base_unittests.cc index 5653809110607..6a865a9eac8db 100644 --- a/impeller/base/base_unittests.cc +++ b/impeller/base/base_unittests.cc @@ -9,6 +9,18 @@ #include "impeller/base/thread.h" namespace impeller { + +enum class MyMaskBits : uint32_t { + kFoo = 0, + kBar = 1 << 0, + kBaz = 1 << 1, + kBang = 1 << 2, +}; + +using MyMask = Mask; + +IMPELLER_ENUM_IS_MASK(MyMaskBits); + namespace testing { struct Foo { @@ -251,15 +263,6 @@ 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; diff --git a/impeller/base/mask.h b/impeller/base/mask.h index 5703c1507c1b9..5f253ea465026 100644 --- a/impeller/base/mask.h +++ b/impeller/base/mask.h @@ -9,6 +9,21 @@ namespace impeller { +template +struct MaskTraits { + static constexpr bool is_mask = false; +}; + +//------------------------------------------------------------------------------ +/// @brief Declare this in the "impeller" namespace to make the enum +/// maskable. +/// +#define IMPELLER_ENUM_IS_MASK(enum_name) \ + template <> \ + struct MaskTraits { \ + static constexpr bool is_mask = true; \ + }; + //------------------------------------------------------------------------------ /// @brief A mask of typed enums. /// @@ -110,42 +125,56 @@ struct Mask { // Construction from Enum Types -template +template < + typename EnumType, + typename std::enable_if::is_mask, bool>::type = true> inline constexpr Mask operator|(const EnumType& lhs, const EnumType& rhs) { return Mask{lhs} | rhs; } -template +template < + typename EnumType, + typename std::enable_if::is_mask, bool>::type = true> inline constexpr Mask operator&(const EnumType& lhs, const EnumType& rhs) { return Mask{lhs} & rhs; } -template +template < + typename EnumType, + typename std::enable_if::is_mask, bool>::type = true> inline constexpr Mask operator^(const EnumType& lhs, const EnumType& rhs) { return Mask{lhs} ^ rhs; } -template +template < + typename EnumType, + typename std::enable_if::is_mask, bool>::type = true> inline constexpr Mask operator~(const EnumType& other) { return ~Mask{other}; } -template +template < + typename EnumType, + typename std::enable_if::is_mask, bool>::type = true> inline constexpr Mask operator|(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} | rhs; } -template +template < + typename EnumType, + typename std::enable_if::is_mask, bool>::type = true> inline constexpr Mask operator&(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} & rhs; } -template +template < + typename EnumType, + typename std::enable_if::is_mask, bool>::type = true> inline constexpr Mask operator^(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} ^ rhs; @@ -154,37 +183,43 @@ inline constexpr Mask operator^(const EnumType& lhs, // Relational operators with EnumType promotion. These can be replaced by a // defaulted spaceship operator post C++20. -template +template ::is_mask, bool> = true> inline constexpr bool operator<(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} < rhs; } -template +template ::is_mask, bool> = true> inline constexpr bool operator>(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} > rhs; } -template +template ::is_mask, bool> = true> inline constexpr bool operator<=(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} <= rhs; } -template +template ::is_mask, bool> = true> inline constexpr bool operator>=(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} >= rhs; } -template +template ::is_mask, bool> = true> inline constexpr bool operator==(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} == rhs; } -template +template ::is_mask, bool> = true> inline constexpr bool operator!=(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} != rhs; diff --git a/impeller/core/formats.cc b/impeller/core/formats.cc index 61cb783b656f6..77cbef96ceb0b 100644 --- a/impeller/core/formats.cc +++ b/impeller/core/formats.cc @@ -80,13 +80,13 @@ bool Attachment::IsValid() const { std::string TextureUsageMaskToString(TextureUsageMask mask) { std::vector usages; - if (mask & static_cast(TextureUsage::kShaderRead)) { + if (mask & TextureUsage::kShaderRead) { usages.push_back(TextureUsage::kShaderRead); } - if (mask & static_cast(TextureUsage::kShaderWrite)) { + if (mask & TextureUsage::kShaderWrite) { usages.push_back(TextureUsage::kShaderWrite); } - if (mask & static_cast(TextureUsage::kRenderTarget)) { + if (mask & TextureUsage::kRenderTarget) { usages.push_back(TextureUsage::kRenderTarget); } std::stringstream stream; diff --git a/impeller/core/formats.h b/impeller/core/formats.h index 00caca8efec18..aa859b45f6818 100644 --- a/impeller/core/formats.h +++ b/impeller/core/formats.h @@ -13,6 +13,7 @@ #include "flutter/fml/hash_combine.h" #include "flutter/fml/logging.h" +#include "impeller/base/mask.h" #include "impeller/geometry/color.h" #include "impeller/geometry/rect.h" #include "impeller/geometry/scalar.h" @@ -297,18 +298,15 @@ enum class SampleCount : uint8_t { kCount4 = 4, }; -using TextureUsageMask = uint64_t; - -enum class TextureUsage : TextureUsageMask { +enum class TextureUsage { kUnknown = 0, kShaderRead = 1 << 0, kShaderWrite = 1 << 1, kRenderTarget = 1 << 2, }; +IMPELLER_ENUM_IS_MASK(TextureUsage); -constexpr bool TextureUsageIsRenderTarget(TextureUsageMask mask) { - return static_cast(TextureUsage::kRenderTarget) & mask; -} +using TextureUsageMask = Mask; constexpr const char* TextureUsageToString(TextureUsage usage) { switch (usage) { @@ -435,7 +433,7 @@ enum class SamplerAddressMode { kDecal, }; -enum class ColorWriteMask : uint64_t { +enum class ColorWriteMaskBits : uint64_t { kNone = 0, kRed = 1 << 0, kGreen = 1 << 1, @@ -443,6 +441,9 @@ enum class ColorWriteMask : uint64_t { kAlpha = 1 << 3, kAll = kRed | kGreen | kBlue | kAlpha, }; +IMPELLER_ENUM_IS_MASK(ColorWriteMaskBits); + +using ColorWriteMask = Mask; constexpr size_t BytesPerPixelForPixelFormat(PixelFormat format) { switch (format) { @@ -508,8 +509,7 @@ struct ColorAttachmentDescriptor { BlendOperation alpha_blend_op = BlendOperation::kAdd; BlendFactor dst_alpha_blend_factor = BlendFactor::kOneMinusSourceAlpha; - std::underlying_type_t write_mask = - static_cast(ColorWriteMask::kAll); + ColorWriteMask write_mask = ColorWriteMaskBits::kAll; constexpr bool operator==(const ColorAttachmentDescriptor& o) const { return format == o.format && // @@ -524,10 +524,10 @@ struct ColorAttachmentDescriptor { } constexpr size_t Hash() const { - return fml::HashCombine(format, blending_enabled, src_color_blend_factor, - color_blend_op, dst_color_blend_factor, - src_alpha_blend_factor, alpha_blend_op, - dst_alpha_blend_factor, write_mask); + return fml::HashCombine( + format, blending_enabled, src_color_blend_factor, color_blend_op, + dst_color_blend_factor, src_alpha_blend_factor, alpha_blend_op, + dst_alpha_blend_factor, static_cast(write_mask)); } }; diff --git a/impeller/core/texture_descriptor.h b/impeller/core/texture_descriptor.h index 8c1868f3ce1e0..66a293d0ca8e0 100644 --- a/impeller/core/texture_descriptor.h +++ b/impeller/core/texture_descriptor.h @@ -40,8 +40,7 @@ struct TextureDescriptor { PixelFormat format = PixelFormat::kUnknown; ISize size; size_t mip_count = 1u; // Size::MipCount is usually appropriate. - TextureUsageMask usage = - static_cast(TextureUsage::kShaderRead); + TextureUsageMask usage = TextureUsage::kShaderRead; SampleCount sample_count = SampleCount::kCount1; CompressionType compression_type = CompressionType::kLossless; diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index e6a2328de0dd7..ef67c00782df1 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -458,8 +458,7 @@ ContentContext::ContentContext( auto clip_color_attachments = clip_pipeline_descriptor->GetColorAttachmentDescriptors(); for (auto& color_attachment : clip_color_attachments) { - color_attachment.second.write_mask = - static_cast(ColorWriteMask::kNone); + color_attachment.second.write_mask = ColorWriteMaskBits::kNone; } clip_pipeline_descriptor->SetColorAttachmentDescriptors( std::move(clip_color_attachments)); diff --git a/impeller/entity/entity_pass_target_unittests.cc b/impeller/entity/entity_pass_target_unittests.cc index c510a44d85a59..654498dd4b118 100644 --- a/impeller/entity/entity_pass_target_unittests.cc +++ b/impeller/entity/entity_pass_target_unittests.cc @@ -71,7 +71,7 @@ TEST_P(EntityPassTargetTest, SwapWithMSAAImplicitResolve) { color0_tex_desc.sample_count = SampleCount::kCount4; color0_tex_desc.format = pixel_format; color0_tex_desc.size = ISize{100, 100}; - color0_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); + color0_tex_desc.usage = TextureUsage::kRenderTarget; auto color0_msaa_tex = allocator.CreateTexture(color0_tex_desc); diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 12586b917fe47..6cc570ff05570 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -59,18 +59,16 @@ void ConfigureBlending(const ProcTableGLES& gl, } { - const auto is_set = [](std::underlying_type_t mask, + const auto is_set = [](ColorWriteMask mask, ColorWriteMask check) -> GLboolean { - using RawType = decltype(mask); - return (static_cast(mask) & static_cast(check)) - ? GL_TRUE - : GL_FALSE; + return (mask & check) ? GL_TRUE : GL_FALSE; }; - gl.ColorMask(is_set(color->write_mask, ColorWriteMask::kRed), // red - is_set(color->write_mask, ColorWriteMask::kGreen), // green - is_set(color->write_mask, ColorWriteMask::kBlue), // blue - is_set(color->write_mask, ColorWriteMask::kAlpha) // alpha + gl.ColorMask( + is_set(color->write_mask, ColorWriteMaskBits::kRed), // red + is_set(color->write_mask, ColorWriteMaskBits::kGreen), // green + is_set(color->write_mask, ColorWriteMaskBits::kBlue), // blue + is_set(color->write_mask, ColorWriteMaskBits::kAlpha) // alpha ); } } diff --git a/impeller/renderer/backend/gles/surface_gles.cc b/impeller/renderer/backend/gles/surface_gles.cc index f32c5131d1fa7..d225831d0896d 100644 --- a/impeller/renderer/backend/gles/surface_gles.cc +++ b/impeller/renderer/backend/gles/surface_gles.cc @@ -29,7 +29,7 @@ std::unique_ptr SurfaceGLES::WrapFBO( color0_tex.type = TextureType::kTexture2D; color0_tex.format = color_format; color0_tex.size = fbo_size; - color0_tex.usage = static_cast(TextureUsage::kRenderTarget); + color0_tex.usage = TextureUsage::kRenderTarget; color0_tex.sample_count = SampleCount::kCount1; color0_tex.storage_mode = StorageMode::kDevicePrivate; @@ -44,8 +44,7 @@ std::unique_ptr SurfaceGLES::WrapFBO( depth_stencil_texture_desc.type = TextureType::kTexture2D; depth_stencil_texture_desc.format = color_format; depth_stencil_texture_desc.size = fbo_size; - depth_stencil_texture_desc.usage = - static_cast(TextureUsage::kRenderTarget); + depth_stencil_texture_desc.usage = TextureUsage::kRenderTarget; depth_stencil_texture_desc.sample_count = SampleCount::kCount1; auto depth_stencil_tex = std::make_shared( diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 55577bcb6add9..23e867bc069df 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -45,8 +45,7 @@ static bool IsDepthStencilFormat(PixelFormat format) { static TextureGLES::Type GetTextureTypeFromDescriptor( const TextureDescriptor& desc) { const auto usage = static_cast(desc.usage); - const auto render_target = - static_cast(TextureUsage::kRenderTarget); + const auto render_target = TextureUsage::kRenderTarget; const auto is_msaa = desc.sample_count == SampleCount::kCount4; if (usage == render_target && IsDepthStencilFormat(desc.format)) { return is_msaa ? TextureGLES::Type::kRenderBufferMultisampled diff --git a/impeller/renderer/backend/metal/formats_mtl.h b/impeller/renderer/backend/metal/formats_mtl.h index f476688709333..28ca80aa60f2a 100644 --- a/impeller/renderer/backend/metal/formats_mtl.h +++ b/impeller/renderer/backend/metal/formats_mtl.h @@ -207,25 +207,22 @@ constexpr MTLBlendOperation ToMTLBlendOperation(BlendOperation type) { return MTLBlendOperationAdd; }; -constexpr MTLColorWriteMask ToMTLColorWriteMask( - std::underlying_type_t type) { - using UnderlyingType = decltype(type); - +constexpr MTLColorWriteMask ToMTLColorWriteMask(ColorWriteMask type) { MTLColorWriteMask mask = MTLColorWriteMaskNone; - if (type & static_cast(ColorWriteMask::kRed)) { + if (type & ColorWriteMaskBits::kRed) { mask |= MTLColorWriteMaskRed; } - if (type & static_cast(ColorWriteMask::kGreen)) { + if (type & ColorWriteMaskBits::kGreen) { mask |= MTLColorWriteMaskGreen; } - if (type & static_cast(ColorWriteMask::kBlue)) { + if (type & ColorWriteMaskBits::kBlue) { mask |= MTLColorWriteMaskBlue; } - if (type & static_cast(ColorWriteMask::kAlpha)) { + if (type & ColorWriteMaskBits::kAlpha) { mask |= MTLColorWriteMaskAlpha; } diff --git a/impeller/renderer/backend/metal/formats_mtl.mm b/impeller/renderer/backend/metal/formats_mtl.mm index 46a2d3ef919fc..256ffea934895 100644 --- a/impeller/renderer/backend/metal/formats_mtl.mm +++ b/impeller/renderer/backend/metal/formats_mtl.mm @@ -95,16 +95,16 @@ mtl_desc.height = desc.size.height; mtl_desc.mipmapLevelCount = desc.mip_count; mtl_desc.usage = MTLTextureUsageUnknown; - if (desc.usage & static_cast(TextureUsage::kUnknown)) { + if (desc.usage & TextureUsage::kUnknown) { mtl_desc.usage |= MTLTextureUsageUnknown; } - if (desc.usage & static_cast(TextureUsage::kShaderRead)) { + if (desc.usage & TextureUsage::kShaderRead) { mtl_desc.usage |= MTLTextureUsageShaderRead; } - if (desc.usage & static_cast(TextureUsage::kShaderWrite)) { + if (desc.usage & TextureUsage::kShaderWrite) { mtl_desc.usage |= MTLTextureUsageShaderWrite; } - if (desc.usage & static_cast(TextureUsage::kRenderTarget)) { + if (desc.usage & TextureUsage::kRenderTarget) { mtl_desc.usage |= MTLTextureUsageRenderTarget; } return mtl_desc; diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index f23cbfbe13895..117ef36ab13e9 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -67,8 +67,8 @@ - (void)flutterPrepareForPresent:(nonnull id)commandBuffer; TextureDescriptor resolve_tex_desc; resolve_tex_desc.format = FromMTLPixelFormat(texture.pixelFormat); resolve_tex_desc.size = root_size; - resolve_tex_desc.usage = static_cast(TextureUsage::kRenderTarget) | - static_cast(TextureUsage::kShaderRead); + resolve_tex_desc.usage = + TextureUsage::kRenderTarget | TextureUsage::kShaderRead; resolve_tex_desc.sample_count = SampleCount::kCount1; resolve_tex_desc.storage_mode = StorageMode::kDevicePrivate; @@ -98,7 +98,7 @@ - (void)flutterPrepareForPresent:(nonnull id)commandBuffer; msaa_tex_desc.sample_count = SampleCount::kCount4; msaa_tex_desc.format = resolve_tex->GetTextureDescriptor().format; msaa_tex_desc.size = resolve_tex->GetSize(); - msaa_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); + msaa_tex_desc.usage = TextureUsage::kRenderTarget; auto msaa_tex = allocator.CreateTexture(msaa_tex_desc); if (!msaa_tex) { diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index ecbbc2f776ff8..85c3e119ad235 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -188,7 +188,7 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( break; } - if (usage & static_cast(TextureUsage::kRenderTarget)) { + if (usage & TextureUsage::kRenderTarget) { if (PixelFormatIsDepthStencil(format)) { vk_usage |= vk::ImageUsageFlagBits::eDepthStencilAttachment; } else { @@ -197,7 +197,7 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( vk_usage |= vk::ImageUsageFlagBits::eInputAttachment; } - if (usage & static_cast(TextureUsage::kShaderRead)) { + if (usage & TextureUsage::kShaderRead) { vk_usage |= vk::ImageUsageFlagBits::eSampled; // Device transient images can only be used as attachments. The caller // specified incorrect usage flags and is attempting to read a device @@ -208,7 +208,7 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( } } - if (usage & static_cast(TextureUsage::kShaderWrite)) { + if (usage & TextureUsage::kShaderWrite) { vk_usage |= vk::ImageUsageFlagBits::eStorage; // Device transient images can only be used as attachments. The caller // specified incorrect usage flags and is attempting to read a device diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h index d73705f90f0b2..05e9aacbfa8f6 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -76,25 +76,22 @@ constexpr vk::BlendOp ToVKBlendOp(BlendOperation op) { FML_UNREACHABLE(); } -constexpr vk::ColorComponentFlags ToVKColorComponentFlags( - std::underlying_type_t type) { - using UnderlyingType = decltype(type); - +constexpr vk::ColorComponentFlags ToVKColorComponentFlags(ColorWriteMask type) { vk::ColorComponentFlags mask; - if (type & static_cast(ColorWriteMask::kRed)) { + if (type & ColorWriteMaskBits::kRed) { mask |= vk::ColorComponentFlagBits::eR; } - if (type & static_cast(ColorWriteMask::kGreen)) { + if (type & ColorWriteMaskBits::kGreen) { mask |= vk::ColorComponentFlagBits::eG; } - if (type & static_cast(ColorWriteMask::kBlue)) { + if (type & ColorWriteMaskBits::kBlue) { mask |= vk::ColorComponentFlagBits::eB; } - if (type & static_cast(ColorWriteMask::kAlpha)) { + if (type & ColorWriteMaskBits::kAlpha) { mask |= vk::ColorComponentFlagBits::eA; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index d8c0c4ce82e01..70dedb19a59b9 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -633,7 +633,7 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { const std::shared_ptr& result_texture = resolve_image_vk_ ? resolve_image_vk_ : color_image_vk_; if (result_texture->GetTextureDescriptor().usage & - static_cast(TextureUsage::kShaderRead)) { + TextureUsage::kShaderRead) { BarrierVK barrier; barrier.cmd_buffer = command_buffer_vk_; barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite | diff --git a/impeller/renderer/backend/vulkan/swapchain/khr/khr_surface_vk.cc b/impeller/renderer/backend/vulkan/swapchain/khr/khr_surface_vk.cc index 81e62017f0ac5..2fa35f63f37f8 100644 --- a/impeller/renderer/backend/vulkan/swapchain/khr/khr_surface_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain/khr/khr_surface_vk.cc @@ -28,7 +28,7 @@ std::unique_ptr KHRSurfaceVK::WrapSwapchainImage( msaa_tex_desc.sample_count = SampleCount::kCount4; msaa_tex_desc.format = swapchain_image->GetPixelFormat(); msaa_tex_desc.size = swapchain_image->GetSize(); - msaa_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); + msaa_tex_desc.usage = TextureUsage::kRenderTarget; if (!swapchain_image->GetMSAATexture()) { msaa_tex = context->GetResourceAllocator()->CreateTexture(msaa_tex_desc); @@ -46,8 +46,7 @@ std::unique_ptr KHRSurfaceVK::WrapSwapchainImage( resolve_tex_desc.type = TextureType::kTexture2D; resolve_tex_desc.format = swapchain_image->GetPixelFormat(); resolve_tex_desc.size = swapchain_image->GetSize(); - resolve_tex_desc.usage = - static_cast(TextureUsage::kRenderTarget); + resolve_tex_desc.usage = TextureUsage::kRenderTarget; resolve_tex_desc.sample_count = SampleCount::kCount1; resolve_tex_desc.storage_mode = StorageMode::kDevicePrivate; diff --git a/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc index 962c6112f1cd8..375c536038359 100644 --- a/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc @@ -222,8 +222,7 @@ KHRSwapchainImplVK::KHRSwapchainImplVK(const std::shared_ptr& context, } TextureDescriptor texture_desc; - texture_desc.usage = - static_cast(TextureUsage::kRenderTarget); + texture_desc.usage = TextureUsage::kRenderTarget; texture_desc.storage_mode = StorageMode::kDevicePrivate; texture_desc.format = ToPixelFormat(swapchain_info.imageFormat); texture_desc.size = ISize::MakeWH(swapchain_info.imageExtent.width, @@ -237,7 +236,7 @@ KHRSwapchainImplVK::KHRSwapchainImplVK(const std::shared_ptr& context, msaa_desc.sample_count = SampleCount::kCount4; msaa_desc.format = texture_desc.format; msaa_desc.size = texture_desc.size; - msaa_desc.usage = static_cast(TextureUsage::kRenderTarget); + msaa_desc.usage = TextureUsage::kRenderTarget; // The depth+stencil configuration matches the configuration used by // RenderTarget::SetupDepthStencilAttachments and matching the swapchain @@ -254,7 +253,7 @@ KHRSwapchainImplVK::KHRSwapchainImplVK(const std::shared_ptr& context, depth_stencil_desc.format = context->GetCapabilities()->GetDefaultDepthStencilFormat(); depth_stencil_desc.size = texture_desc.size; - depth_stencil_desc.usage = static_cast(TextureUsage::kRenderTarget); + depth_stencil_desc.usage = TextureUsage::kRenderTarget; std::shared_ptr msaa_texture; if (enable_msaa) { diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index e551ecf5ebf84..6afa486bab7bb 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -282,8 +282,8 @@ RenderTarget RenderTargetAllocator::CreateOffscreen( color0_tex_desc.format = pixel_format; color0_tex_desc.size = size; color0_tex_desc.mip_count = mip_count; - color0_tex_desc.usage = static_cast(TextureUsage::kRenderTarget) | - static_cast(TextureUsage::kShaderRead); + color0_tex_desc.usage = + TextureUsage::kRenderTarget | TextureUsage::kShaderRead; color0_tex = allocator_->CreateTexture(color0_tex_desc); if (!color0_tex) { return {}; @@ -338,7 +338,7 @@ RenderTarget RenderTargetAllocator::CreateOffscreenMSAA( color0_tex_desc.sample_count = SampleCount::kCount4; color0_tex_desc.format = pixel_format; color0_tex_desc.size = size; - color0_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); + color0_tex_desc.usage = TextureUsage::kRenderTarget; if (context.GetCapabilities()->SupportsImplicitResolvingMSAA()) { // See below ("SupportsImplicitResolvingMSAA") for more details. color0_tex_desc.storage_mode = StorageMode::kDevicePrivate; @@ -364,8 +364,7 @@ RenderTarget RenderTargetAllocator::CreateOffscreenMSAA( color0_resolve_tex_desc.size = size; color0_resolve_tex_desc.compression_type = CompressionType::kLossy; color0_resolve_tex_desc.usage = - static_cast(TextureUsage::kRenderTarget) | - static_cast(TextureUsage::kShaderRead); + TextureUsage::kRenderTarget | TextureUsage::kShaderRead; color0_resolve_tex_desc.mip_count = mip_count; color0_resolve_tex = allocator_->CreateTexture(color0_resolve_tex_desc); if (!color0_resolve_tex) { @@ -433,8 +432,7 @@ void RenderTarget::SetupDepthStencilAttachments( depth_stencil_texture_desc.format = context.GetCapabilities()->GetDefaultDepthStencilFormat(); depth_stencil_texture_desc.size = size; - depth_stencil_texture_desc.usage = - static_cast(TextureUsage::kRenderTarget); + depth_stencil_texture_desc.usage = TextureUsage::kRenderTarget; depth_stencil_texture = allocator.CreateTexture(depth_stencil_texture_desc); if (!depth_stencil_texture) { return; // Error messages are handled by `Allocator::CreateTexture`. diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index b9225570d3b92..bc492c612e046 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -336,8 +336,7 @@ TEST_P(RendererTest, CanRenderToTexture) { texture_descriptor.storage_mode = StorageMode::kHostVisible; texture_descriptor.size = {400, 400}; texture_descriptor.mip_count = 1u; - texture_descriptor.usage = - static_cast(TextureUsage::kRenderTarget); + texture_descriptor.usage = TextureUsage::kRenderTarget; color0.texture = context->GetResourceAllocator()->CreateTexture(texture_descriptor); @@ -353,8 +352,7 @@ TEST_P(RendererTest, CanRenderToTexture) { stencil_texture_desc.storage_mode = StorageMode::kDeviceTransient; stencil_texture_desc.size = texture_descriptor.size; stencil_texture_desc.format = PixelFormat::kS8UInt; - stencil_texture_desc.usage = - static_cast(TextureUsage::kRenderTarget); + stencil_texture_desc.usage = TextureUsage::kRenderTarget; stencil0.texture = context->GetResourceAllocator()->CreateTexture(stencil_texture_desc); @@ -477,9 +475,7 @@ TEST_P(RendererTest, CanBlitTextureToTexture) { texture_desc.format = PixelFormat::kR8G8B8A8UNormInt; texture_desc.size = {800, 600}; texture_desc.mip_count = 1u; - texture_desc.usage = - static_cast(TextureUsage::kRenderTarget) | - static_cast(TextureUsage::kShaderRead); + texture_desc.usage = TextureUsage::kRenderTarget | TextureUsage::kShaderRead; auto texture = context->GetResourceAllocator()->CreateTexture(texture_desc); ASSERT_TRUE(texture); @@ -601,10 +597,8 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) { texture_desc.format = PixelFormat::kR8G8B8A8UNormInt; texture_desc.size = bridge->GetTextureDescriptor().size; texture_desc.mip_count = 1u; - texture_desc.usage = - static_cast(TextureUsage::kRenderTarget) | - static_cast(TextureUsage::kShaderWrite) | - static_cast(TextureUsage::kShaderRead); + texture_desc.usage = TextureUsage::kRenderTarget | + TextureUsage::kShaderWrite | TextureUsage::kShaderRead; DeviceBufferDescriptor device_buffer_desc; device_buffer_desc.storage_mode = StorageMode::kHostVisible; device_buffer_desc.size = diff --git a/impeller/scene/scene_encoder.cc b/impeller/scene/scene_encoder.cc index 9f934f39e4fee..a998ee00689a9 100644 --- a/impeller/scene/scene_encoder.cc +++ b/impeller/scene/scene_encoder.cc @@ -54,8 +54,7 @@ std::shared_ptr SceneEncoder::BuildSceneCommandBuffer( ds_texture.type = TextureType::kTexture2DMultisample; ds_texture.format = PixelFormat::kD32FloatS8UInt; ds_texture.size = render_target.GetRenderTargetSize(); - ds_texture.usage = - static_cast(TextureUsage::kRenderTarget); + ds_texture.usage = TextureUsage::kRenderTarget; ds_texture.sample_count = SampleCount::kCount4; ds_texture.storage_mode = StorageMode::kDeviceTransient; auto texture = diff --git a/lib/gpu/texture.cc b/lib/gpu/texture.cc index 3ef47da36c67d..5acd5ad325b12 100644 --- a/lib/gpu/texture.cc +++ b/lib/gpu/texture.cc @@ -86,7 +86,7 @@ bool InternalFlutterGpu_Texture_Initialize(Dart_Handle wrapper, desc.storage_mode = flutter::gpu::ToImpellerStorageMode(storage_mode); desc.size = {width, height}; desc.format = flutter::gpu::ToImpellerPixelFormat(format); - desc.usage = 0; + desc.usage = {}; if (enable_render_target_usage) { desc.usage |= static_cast( impeller::TextureUsage::kRenderTarget); diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 4633fcad552d2..f35dd805ecb77 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1048,9 +1048,8 @@ MakeRenderTargetFromBackingStoreImpeller( resolve_tex_desc.size = size; resolve_tex_desc.sample_count = impeller::SampleCount::kCount1; resolve_tex_desc.storage_mode = impeller::StorageMode::kDevicePrivate; - resolve_tex_desc.usage = - static_cast(impeller::TextureUsage::kRenderTarget) | - static_cast(impeller::TextureUsage::kShaderRead); + resolve_tex_desc.usage = impeller::TextureUsage::kRenderTarget | + impeller::TextureUsage::kShaderRead; auto resolve_tex = impeller::WrapTextureMTL( resolve_tex_desc, metal->texture.texture, @@ -1068,8 +1067,7 @@ MakeRenderTargetFromBackingStoreImpeller( msaa_tex_desc.sample_count = impeller::SampleCount::kCount4; msaa_tex_desc.format = resolve_tex->GetTextureDescriptor().format; msaa_tex_desc.size = size; - msaa_tex_desc.usage = - static_cast(impeller::TextureUsage::kRenderTarget); + msaa_tex_desc.usage = impeller::TextureUsage::kRenderTarget; auto msaa_tex = aiks_context->GetContext()->GetResourceAllocator()->CreateTexture( From c08e1c60554fc37070390fc1c8fd2502a2d4199e Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 12 Mar 2024 16:26:59 -0700 Subject: [PATCH 2/2] PR comments and tidy checks. --- impeller/base/mask.h | 30 +++++++++++++++--------------- lib/gpu/texture.cc | 9 +++------ 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/impeller/base/mask.h b/impeller/base/mask.h index 5f253ea465026..97f30d7a7418e 100644 --- a/impeller/base/mask.h +++ b/impeller/base/mask.h @@ -11,7 +11,7 @@ namespace impeller { template struct MaskTraits { - static constexpr bool is_mask = false; + static constexpr bool kIsMask = false; }; //------------------------------------------------------------------------------ @@ -21,7 +21,7 @@ struct MaskTraits { #define IMPELLER_ENUM_IS_MASK(enum_name) \ template <> \ struct MaskTraits { \ - static constexpr bool is_mask = true; \ + static constexpr bool kIsMask = true; \ }; //------------------------------------------------------------------------------ @@ -127,7 +127,7 @@ struct Mask { template < typename EnumType, - typename std::enable_if::is_mask, bool>::type = true> + typename std::enable_if::kIsMask, bool>::type = true> inline constexpr Mask operator|(const EnumType& lhs, const EnumType& rhs) { return Mask{lhs} | rhs; @@ -135,7 +135,7 @@ inline constexpr Mask operator|(const EnumType& lhs, template < typename EnumType, - typename std::enable_if::is_mask, bool>::type = true> + typename std::enable_if::kIsMask, bool>::type = true> inline constexpr Mask operator&(const EnumType& lhs, const EnumType& rhs) { return Mask{lhs} & rhs; @@ -143,7 +143,7 @@ inline constexpr Mask operator&(const EnumType& lhs, template < typename EnumType, - typename std::enable_if::is_mask, bool>::type = true> + typename std::enable_if::kIsMask, bool>::type = true> inline constexpr Mask operator^(const EnumType& lhs, const EnumType& rhs) { return Mask{lhs} ^ rhs; @@ -151,14 +151,14 @@ inline constexpr Mask operator^(const EnumType& lhs, template < typename EnumType, - typename std::enable_if::is_mask, bool>::type = true> + typename std::enable_if::kIsMask, bool>::type = true> inline constexpr Mask operator~(const EnumType& other) { return ~Mask{other}; } template < typename EnumType, - typename std::enable_if::is_mask, bool>::type = true> + typename std::enable_if::kIsMask, bool>::type = true> inline constexpr Mask operator|(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} | rhs; @@ -166,7 +166,7 @@ inline constexpr Mask operator|(const EnumType& lhs, template < typename EnumType, - typename std::enable_if::is_mask, bool>::type = true> + typename std::enable_if::kIsMask, bool>::type = true> inline constexpr Mask operator&(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} & rhs; @@ -174,7 +174,7 @@ inline constexpr Mask operator&(const EnumType& lhs, template < typename EnumType, - typename std::enable_if::is_mask, bool>::type = true> + typename std::enable_if::kIsMask, bool>::type = true> inline constexpr Mask operator^(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} ^ rhs; @@ -184,42 +184,42 @@ inline constexpr Mask operator^(const EnumType& lhs, // defaulted spaceship operator post C++20. template ::is_mask, bool> = true> + typename std::enable_if_t::kIsMask, bool> = true> inline constexpr bool operator<(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} < rhs; } template ::is_mask, bool> = true> + typename std::enable_if_t::kIsMask, bool> = true> inline constexpr bool operator>(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} > rhs; } template ::is_mask, bool> = true> + typename std::enable_if_t::kIsMask, bool> = true> inline constexpr bool operator<=(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} <= rhs; } template ::is_mask, bool> = true> + typename std::enable_if_t::kIsMask, bool> = true> inline constexpr bool operator>=(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} >= rhs; } template ::is_mask, bool> = true> + typename std::enable_if_t::kIsMask, bool> = true> inline constexpr bool operator==(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} == rhs; } template ::is_mask, bool> = true> + typename std::enable_if_t::kIsMask, bool> = true> inline constexpr bool operator!=(const EnumType& lhs, const Mask& rhs) { return Mask{lhs} != rhs; diff --git a/lib/gpu/texture.cc b/lib/gpu/texture.cc index 5acd5ad325b12..740e3dc51ae44 100644 --- a/lib/gpu/texture.cc +++ b/lib/gpu/texture.cc @@ -88,16 +88,13 @@ bool InternalFlutterGpu_Texture_Initialize(Dart_Handle wrapper, desc.format = flutter::gpu::ToImpellerPixelFormat(format); desc.usage = {}; if (enable_render_target_usage) { - desc.usage |= static_cast( - impeller::TextureUsage::kRenderTarget); + desc.usage |= impeller::TextureUsage::kRenderTarget; } if (enable_shader_read_usage) { - desc.usage |= static_cast( - impeller::TextureUsage::kShaderRead); + desc.usage |= impeller::TextureUsage::kShaderRead; } if (enable_shader_write_usage) { - desc.usage |= static_cast( - impeller::TextureUsage::kShaderWrite); + desc.usage |= impeller::TextureUsage::kShaderWrite; } switch (sample_count) { case 1: