From 58e9329f9d98ba4505d62a83fe5c61dc46902357 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 10 Feb 2024 11:27:10 -0800 Subject: [PATCH 1/8] [Impeller] Use R8 instead of A8 for text atlas. --- impeller/entity/contents/text_contents.cc | 2 +- impeller/entity/shaders/glyph_atlas.frag | 2 +- .../backends/skia/typographer_context_skia.cc | 21 +++++++++++------- .../backends/stb/typographer_context_stb.cc | 8 +++---- impeller/typographer/glyph_atlas.cc | 2 +- impeller/typographer/glyph_atlas.h | 4 ++-- impeller/typographer/lazy_glyph_atlas.cc | 12 +++++----- impeller/typographer/text_frame.cc | 2 +- impeller/typographer/typographer_unittests.cc | 22 +++++++++---------- 9 files changed, 40 insertions(+), 35 deletions(-) diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index b83d29a8a62cf..61610b9191cb7 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -83,7 +83,7 @@ bool TextContents::Render(const ContentContext& renderer, pass.SetCommandLabel("TextFrame"); auto opts = OptionsFromPassAndEntity(pass, entity); opts.primitive_type = PrimitiveType::kTriangle; - if (type == GlyphAtlas::Type::kAlphaBitmap) { + if (type == GlyphAtlas::Type::kRedBitmap) { pass.SetPipeline(renderer.GetGlyphAtlasPipeline(opts)); } else { pass.SetPipeline(renderer.GetGlyphAtlasColorPipeline(opts)); diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag index 9a9b71780dd33..1c681c86c308d 100644 --- a/impeller/entity/shaders/glyph_atlas.frag +++ b/impeller/entity/shaders/glyph_atlas.frag @@ -16,5 +16,5 @@ out f16vec4 frag_color; void main() { f16vec4 value = texture(glyph_atlas_sampler, v_uv); - frag_color = value.aaaa * v_text_color; + frag_color = value.rrrr * v_text_color; } diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 1330d77495d30..2cfc3bd80beda 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -15,6 +15,8 @@ #include "impeller/typographer/backends/skia/typeface_skia.h" #include "impeller/typographer/rectangle_packer.h" #include "impeller/typographer/typographer_context.h" +#include "include/core/SkColor.h" +#include "include/core/SkSize.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkFont.h" @@ -120,12 +122,12 @@ static ISize OptimumAtlasSizeForFontGlyphPairs( GlyphAtlas::Type type, const ISize& max_texture_size) { static constexpr auto kMinAtlasSize = 8u; - static constexpr auto kMinAlphaBitmapSize = 1024u; + static constexpr auto kMinRedBitmapSize = 1024u; TRACE_EVENT0("impeller", __FUNCTION__); - ISize current_size = type == GlyphAtlas::Type::kAlphaBitmap - ? ISize(kMinAlphaBitmapSize, kMinAlphaBitmapSize) + ISize current_size = type == GlyphAtlas::Type::kRedBitmap + ? ISize(kMinRedBitmapSize, kMinRedBitmapSize) : ISize(kMinAtlasSize, kMinAtlasSize); size_t total_pairs = pairs.size() + 1; do { @@ -169,7 +171,7 @@ static void DrawGlyph(SkCanvas* canvas, sk_font.setHinting(SkFontHinting::kSlight); sk_font.setEmbolden(metrics.embolden); - auto glyph_color = has_color ? SK_ColorWHITE : SK_ColorBLACK; + auto glyph_color = has_color ? SK_ColorWHITE : SK_ColorRED; SkPaint glyph_paint; glyph_paint.setColor(glyph_color); @@ -219,8 +221,11 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, SkImageInfo image_info; switch (atlas.GetType()) { - case GlyphAtlas::Type::kAlphaBitmap: - image_info = SkImageInfo::MakeA8(atlas_size.width, atlas_size.height); + case GlyphAtlas::Type::kRedBitmap: + image_info = + SkImageInfo::Make(SkISize{static_cast(atlas_size.width), + static_cast(atlas_size.height)}, + kR8_unorm_SkColorType, kPremul_SkAlphaType); break; case GlyphAtlas::Type::kColorBitmap: image_info = @@ -465,8 +470,8 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- PixelFormat format; switch (type) { - case GlyphAtlas::Type::kAlphaBitmap: - format = PixelFormat::kA8UNormInt; + case GlyphAtlas::Type::kRedBitmap: + format = PixelFormat::kR8UNormInt; break; case GlyphAtlas::Type::kColorBitmap: format = PixelFormat::kR8G8B8A8UNormInt; diff --git a/impeller/typographer/backends/stb/typographer_context_stb.cc b/impeller/typographer/backends/stb/typographer_context_stb.cc index be909744edaed..20f4141ee5ed8 100644 --- a/impeller/typographer/backends/stb/typographer_context_stb.cc +++ b/impeller/typographer/backends/stb/typographer_context_stb.cc @@ -163,12 +163,12 @@ static ISize OptimumAtlasSizeForFontGlyphPairs( GlyphAtlas::Type type, const ISize& max_texture_size) { static constexpr auto kMinAtlasSize = 8u; - static constexpr auto kMinAlphaBitmapSize = 1024u; + static constexpr auto kMinRedBitmapSize = 1024u; TRACE_EVENT0("impeller", __FUNCTION__); - ISize current_size = type == GlyphAtlas::Type::kAlphaBitmap - ? ISize(kMinAlphaBitmapSize, kMinAlphaBitmapSize) + ISize current_size = type == GlyphAtlas::Type::kRedBitmap + ? ISize(kMinRedBitmapSize, kMinRedBitmapSize) : ISize(kMinAtlasSize, kMinAtlasSize); size_t total_pairs = pairs.size() + 1; do { @@ -515,7 +515,7 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( // --------------------------------------------------------------------------- PixelFormat format; switch (type) { - case GlyphAtlas::Type::kAlphaBitmap: + case GlyphAtlas::Type::kRedBitmap: format = PixelFormat::kA8UNormInt; break; case GlyphAtlas::Type::kColorBitmap: diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 1f9bfaff8d7ae..76ed49464c7d1 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -10,7 +10,7 @@ namespace impeller { GlyphAtlasContext::GlyphAtlasContext() - : atlas_(std::make_shared(GlyphAtlas::Type::kAlphaBitmap)), + : atlas_(std::make_shared(GlyphAtlas::Type::kRedBitmap)), atlas_size_(ISize(0, 0)) {} GlyphAtlasContext::~GlyphAtlasContext() {} diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index ea4ea295e4845..3c473a0fc2853 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -33,9 +33,9 @@ class GlyphAtlas { enum class Type { //-------------------------------------------------------------------------- /// The glyphs are reprsented at their requested size using only an 8-bit - /// alpha channel. + /// red channel. /// - kAlphaBitmap, + kRedBitmap, //-------------------------------------------------------------------------- /// The glyphs are reprsented at their requested size using N32 premul diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index 121e6186c9e09..efe6229022a53 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -30,7 +30,7 @@ LazyGlyphAtlas::~LazyGlyphAtlas() = default; void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale) { FML_DCHECK(alpha_atlas_ == nullptr && color_atlas_ == nullptr); - if (frame.GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) { + if (frame.GetAtlasType() == GlyphAtlas::Type::kRedBitmap) { frame.CollectUniqueFontGlyphPairs(alpha_glyph_map_, scale); } else { frame.CollectUniqueFontGlyphPairs(color_glyph_map_, scale); @@ -48,7 +48,7 @@ const std::shared_ptr& LazyGlyphAtlas::CreateOrGetGlyphAtlas( Context& context, GlyphAtlas::Type type) const { { - if (type == GlyphAtlas::Type::kAlphaBitmap && alpha_atlas_) { + if (type == GlyphAtlas::Type::kRedBitmap && alpha_atlas_) { return alpha_atlas_; } if (type == GlyphAtlas::Type::kColorBitmap && color_atlas_) { @@ -67,17 +67,17 @@ const std::shared_ptr& LazyGlyphAtlas::CreateOrGetGlyphAtlas( return kNullGlyphAtlas; } - auto& glyph_map = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_glyph_map_ - : color_glyph_map_; + auto& glyph_map = type == GlyphAtlas::Type::kRedBitmap ? alpha_glyph_map_ + : color_glyph_map_; const std::shared_ptr& atlas_context = - type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_; + type == GlyphAtlas::Type::kRedBitmap ? alpha_context_ : color_context_; std::shared_ptr atlas = typographer_context_->CreateGlyphAtlas( context, type, atlas_context, glyph_map); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Could not create valid atlas."; return kNullGlyphAtlas; } - if (type == GlyphAtlas::Type::kAlphaBitmap) { + if (type == GlyphAtlas::Type::kRedBitmap) { alpha_atlas_ = std::move(atlas); return alpha_atlas_; } diff --git a/impeller/typographer/text_frame.cc b/impeller/typographer/text_frame.cc index 5e38b68a76729..d9df5d943c47e 100644 --- a/impeller/typographer/text_frame.cc +++ b/impeller/typographer/text_frame.cc @@ -27,7 +27,7 @@ const std::vector& TextFrame::GetRuns() const { GlyphAtlas::Type TextFrame::GetAtlasType() const { return has_color_ ? GlyphAtlas::Type::kColorBitmap - : GlyphAtlas::Type::kAlphaBitmap; + : GlyphAtlas::Type::kRedBitmap; } bool TextFrame::MaybeHasOverlapping() const { diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 5a239b3b12d46..18d88ca3a5913 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -64,11 +64,11 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); - ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); + ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kRedBitmap); ASSERT_EQ(atlas->GetGlyphCount(), 4llu); std::optional first_scaled_font; @@ -117,12 +117,12 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { lazy_atlas.AddTextFrame(*frame, 1.0f); - // Creates different atlases for color and alpha bitmap. + // Creates different atlases for color and red bitmap. auto color_atlas = lazy_atlas.CreateOrGetGlyphAtlas( *GetContext(), GlyphAtlas::Type::kColorBitmap); auto bitmap_atlas = lazy_atlas.CreateOrGetGlyphAtlas( - *GetContext(), GlyphAtlas::Type::kAlphaBitmap); + *GetContext(), GlyphAtlas::Type::kRedBitmap); ASSERT_FALSE(color_atlas == bitmap_atlas); } @@ -135,7 +135,7 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { auto blob = SkTextBlob::MakeFromString("AGH", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -152,7 +152,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { auto blob = SkTextBlob::MakeFromString("spooky skellingtons", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -161,7 +161,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { // now attempt to re-create an atlas with the same text blob. auto next_atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); @@ -189,7 +189,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { font_glyph_map, 0.6 * index); }; auto atlas = - context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kAlphaBitmap, + context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kRedBitmap, atlas_context, font_glyph_map); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -219,7 +219,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); auto old_packer = atlas_context->GetRectPacker(); @@ -233,7 +233,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); auto next_atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob2)); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -252,7 +252,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); auto old_packer = atlas_context->GetRectPacker(); From b3d9c1e53d3c8d4b5837df13eb02723d3243ba2c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 10 Feb 2024 11:52:30 -0800 Subject: [PATCH 2/8] add case for texturegles. --- impeller/renderer/backend/gles/texture_gles.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index 74b2f6f503fcf..55577bcb6add9 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -130,6 +130,11 @@ struct TexImage2DData { external_format = GL_ALPHA; type = GL_UNSIGNED_BYTE; break; + case PixelFormat::kR8UNormInt: + internal_format = GL_RED; + external_format = GL_RED; + type = GL_UNSIGNED_BYTE; + break; case PixelFormat::kR8G8B8A8UNormInt: case PixelFormat::kB8G8R8A8UNormInt: case PixelFormat::kR8G8B8A8UNormIntSRGB: @@ -161,7 +166,6 @@ struct TexImage2DData { break; case PixelFormat::kUnknown: case PixelFormat::kD32FloatS8UInt: - case PixelFormat::kR8UNormInt: case PixelFormat::kR8G8UNormInt: case PixelFormat::kB10G10R10XRSRGB: case PixelFormat::kB10G10R10XR: From f3a608f676379d6b86f5981d3d253f01729cfc7f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 11 Feb 2024 13:09:16 -0800 Subject: [PATCH 3/8] ++ --- impeller/typographer/backends/stb/typographer_context_stb.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/typographer/backends/stb/typographer_context_stb.cc b/impeller/typographer/backends/stb/typographer_context_stb.cc index 20f4141ee5ed8..5b751e224d902 100644 --- a/impeller/typographer/backends/stb/typographer_context_stb.cc +++ b/impeller/typographer/backends/stb/typographer_context_stb.cc @@ -516,10 +516,10 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( PixelFormat format; switch (type) { case GlyphAtlas::Type::kRedBitmap: - format = PixelFormat::kA8UNormInt; + format = PixelFormat::kR8UNormInt; break; case GlyphAtlas::Type::kColorBitmap: - format = DISABLE_COLOR_FONT_SUPPORT ? PixelFormat::kA8UNormInt + format = DISABLE_COLOR_FONT_SUPPORT ? PixelFormat::kR8UNormInt : PixelFormat::kR8G8B8A8UNormInt; break; } From fdea14a46d292e726804d7dfd173f1d3e0d8bd5f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 13 Feb 2024 12:56:02 -0800 Subject: [PATCH 4/8] wip texture format. --- impeller/entity/shaders/glyph_atlas.frag | 2 +- impeller/renderer/capabilities.cc | 20 +++++++++++++++++-- impeller/renderer/capabilities.h | 9 +++++++++ .../backends/skia/typographer_context_skia.cc | 17 ++++++++-------- .../backends/stb/typographer_context_stb.cc | 8 ++++---- impeller/typographer/glyph_atlas.h | 5 ++--- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag index 1c681c86c308d..120520b6b6dcb 100644 --- a/impeller/entity/shaders/glyph_atlas.frag +++ b/impeller/entity/shaders/glyph_atlas.frag @@ -16,5 +16,5 @@ out f16vec4 frag_color; void main() { f16vec4 value = texture(glyph_atlas_sampler, v_uv); - frag_color = value.rrrr * v_text_color; + frag_color = max(value.rrrr, value.aaaa) * v_text_color; } diff --git a/impeller/renderer/capabilities.cc b/impeller/renderer/capabilities.cc index 4dcda955ab954..0ad4ff9a61de5 100644 --- a/impeller/renderer/capabilities.cc +++ b/impeller/renderer/capabilities.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/renderer/capabilities.h" +#include "impeller/core/formats.h" namespace impeller { @@ -74,10 +75,16 @@ class StandardCapabilities final : public Capabilities { return default_depth_stencil_format_; } + // |Capabilities| bool SupportsDeviceTransientTextures() const override { return supports_device_transient_textures_; } + // |Capabilities| + PixelFormat GetDefaultGlyphAtlasFormat() const override { + return default_glyph_atlas_format_; + } + private: StandardCapabilities(bool supports_offscreen_msaa, bool supports_ssbo, @@ -91,7 +98,8 @@ class StandardCapabilities final : public Capabilities { bool supports_device_transient_textures, PixelFormat default_color_format, PixelFormat default_stencil_format, - PixelFormat default_depth_stencil_format) + PixelFormat default_depth_stencil_format, + PixelFormat default_glyph_atlas_format) : supports_offscreen_msaa_(supports_offscreen_msaa), supports_ssbo_(supports_ssbo), supports_buffer_to_texture_blits_(supports_buffer_to_texture_blits), @@ -122,6 +130,7 @@ class StandardCapabilities final : public Capabilities { PixelFormat default_color_format_ = PixelFormat::kUnknown; PixelFormat default_stencil_format_ = PixelFormat::kUnknown; PixelFormat default_depth_stencil_format_ = PixelFormat::kUnknown; + PixelFormat default_glyph_atlas_format_ = PixelFormat::kUnknown; StandardCapabilities(const StandardCapabilities&) = delete; @@ -207,6 +216,12 @@ CapabilitiesBuilder& CapabilitiesBuilder::SetSupportsDeviceTransientTextures( return *this; } +CapabilitiesBuilder& CapabilitiesBuilder::SetDefaultGlyphAtlasFormat( + PixelFormat value) { + default_glyph_atlas_format_ = value; + return *this; +} + std::unique_ptr CapabilitiesBuilder::Build() { return std::unique_ptr(new StandardCapabilities( // supports_offscreen_msaa_, // @@ -221,7 +236,8 @@ std::unique_ptr CapabilitiesBuilder::Build() { supports_device_transient_textures_, // default_color_format_.value_or(PixelFormat::kUnknown), // default_stencil_format_.value_or(PixelFormat::kUnknown), // - default_depth_stencil_format_.value_or(PixelFormat::kUnknown) // + default_depth_stencil_format_.value_or(PixelFormat::kUnknown), // + default_glyph_atlas_format_.value_or(PixelFormat::kUnknown) // )); } diff --git a/impeller/renderer/capabilities.h b/impeller/renderer/capabilities.h index 9f3f15cefe897..34c5839bd5959 100644 --- a/impeller/renderer/capabilities.h +++ b/impeller/renderer/capabilities.h @@ -105,6 +105,12 @@ class Capabilities { /// format was found. virtual PixelFormat GetDefaultDepthStencilFormat() const = 0; + /// @brief Returns the default pixel format for the alpha bitmap glyph atlas. + /// + /// Some backends may use Red channel while others use grey. This + /// should not have any impact + virtual PixelFormat GetDefaultGlyphAtlasFormat() const = 0; + protected: Capabilities(); @@ -145,6 +151,8 @@ class CapabilitiesBuilder { CapabilitiesBuilder& SetSupportsDeviceTransientTextures(bool value); + CapabilitiesBuilder& SetDefaultGlyphAtlasFormat(PixelFormat value); + std::unique_ptr Build(); private: @@ -161,6 +169,7 @@ class CapabilitiesBuilder { std::optional default_color_format_ = std::nullopt; std::optional default_stencil_format_ = std::nullopt; std::optional default_depth_stencil_format_ = std::nullopt; + std::optional default_glyph_atlas_format_ = std::nullopt; CapabilitiesBuilder(const CapabilitiesBuilder&) = delete; diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 2cfc3bd80beda..0c8d191d53872 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -122,12 +122,12 @@ static ISize OptimumAtlasSizeForFontGlyphPairs( GlyphAtlas::Type type, const ISize& max_texture_size) { static constexpr auto kMinAtlasSize = 8u; - static constexpr auto kMinRedBitmapSize = 1024u; + static constexpr auto kMinAlphaBitmapSize = 1024u; TRACE_EVENT0("impeller", __FUNCTION__); - ISize current_size = type == GlyphAtlas::Type::kRedBitmap - ? ISize(kMinRedBitmapSize, kMinRedBitmapSize) + ISize current_size = type == GlyphAtlas::Type::kAlphaBitmap + ? ISize(kMinAlphaBitmapSize, kMinAlphaBitmapSize) : ISize(kMinAtlasSize, kMinAtlasSize); size_t total_pairs = pairs.size() + 1; do { @@ -171,7 +171,7 @@ static void DrawGlyph(SkCanvas* canvas, sk_font.setHinting(SkFontHinting::kSlight); sk_font.setEmbolden(metrics.embolden); - auto glyph_color = has_color ? SK_ColorWHITE : SK_ColorRED; + auto glyph_color = has_color ? SK_ColorWHITE : SK_ColorBLACK; SkPaint glyph_paint; glyph_paint.setColor(glyph_color); @@ -221,11 +221,10 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, SkImageInfo image_info; switch (atlas.GetType()) { - case GlyphAtlas::Type::kRedBitmap: + case GlyphAtlas::Type::kAlphaBitmap: image_info = - SkImageInfo::Make(SkISize{static_cast(atlas_size.width), - static_cast(atlas_size.height)}, - kR8_unorm_SkColorType, kPremul_SkAlphaType); + SkImageInfo::MakeA8(SkISize{static_cast(atlas_size.width), + static_cast(atlas_size.height)}); break; case GlyphAtlas::Type::kColorBitmap: image_info = @@ -470,7 +469,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- PixelFormat format; switch (type) { - case GlyphAtlas::Type::kRedBitmap: + case GlyphAtlas::Type::kAlphaBitmap: format = PixelFormat::kR8UNormInt; break; case GlyphAtlas::Type::kColorBitmap: diff --git a/impeller/typographer/backends/stb/typographer_context_stb.cc b/impeller/typographer/backends/stb/typographer_context_stb.cc index 5b751e224d902..0f1db9f4190c2 100644 --- a/impeller/typographer/backends/stb/typographer_context_stb.cc +++ b/impeller/typographer/backends/stb/typographer_context_stb.cc @@ -163,12 +163,12 @@ static ISize OptimumAtlasSizeForFontGlyphPairs( GlyphAtlas::Type type, const ISize& max_texture_size) { static constexpr auto kMinAtlasSize = 8u; - static constexpr auto kMinRedBitmapSize = 1024u; + static constexpr auto kMinAlphaBitmapSize = 1024u; TRACE_EVENT0("impeller", __FUNCTION__); - ISize current_size = type == GlyphAtlas::Type::kRedBitmap - ? ISize(kMinRedBitmapSize, kMinRedBitmapSize) + ISize current_size = type == GlyphAtlas::Type::kAlphaBitmap + ? ISize(kMinAlphaBitmapSize, kMinAlphaBitmapSize) : ISize(kMinAtlasSize, kMinAtlasSize); size_t total_pairs = pairs.size() + 1; do { @@ -515,7 +515,7 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( // --------------------------------------------------------------------------- PixelFormat format; switch (type) { - case GlyphAtlas::Type::kRedBitmap: + case GlyphAtlas::Type::kAlphaBitmap: format = PixelFormat::kR8UNormInt; break; case GlyphAtlas::Type::kColorBitmap: diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 3c473a0fc2853..92600062175e2 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -10,7 +10,6 @@ #include #include -#include "flutter/fml/macros.h" #include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/pipeline.h" @@ -33,9 +32,9 @@ class GlyphAtlas { enum class Type { //-------------------------------------------------------------------------- /// The glyphs are reprsented at their requested size using only an 8-bit - /// red channel. + /// alpha or red channel. /// - kRedBitmap, + kAlphaBitmap, //-------------------------------------------------------------------------- /// The glyphs are reprsented at their requested size using N32 premul From e06eca853937377691d7605dfd31498806eeee63 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 13 Feb 2024 13:38:35 -0800 Subject: [PATCH 5/8] Add capability check. --- impeller/entity/shaders/glyph_atlas.frag | 2 +- .../backend/gles/capabilities_gles.cc | 11 ++++++++++ .../renderer/backend/gles/capabilities_gles.h | 6 ++++++ .../renderer/backend/metal/context_mtl.mm | 1 + .../backend/vulkan/capabilities_vk.cc | 4 ++++ .../renderer/backend/vulkan/capabilities_vk.h | 3 +++ impeller/renderer/capabilities_unittests.cc | 21 +++++++++++++------ 7 files changed, 41 insertions(+), 7 deletions(-) diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag index 120520b6b6dcb..d0bca0579cee4 100644 --- a/impeller/entity/shaders/glyph_atlas.frag +++ b/impeller/entity/shaders/glyph_atlas.frag @@ -16,5 +16,5 @@ out f16vec4 frag_color; void main() { f16vec4 value = texture(glyph_atlas_sampler, v_uv); - frag_color = max(value.rrrr, value.aaaa) * v_text_color; + frag_color = vec4(max(value.r, value.a)) * v_text_color; } diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc index e832aca15f276..7791763782ae7 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/impeller/renderer/backend/gles/capabilities_gles.cc @@ -4,6 +4,7 @@ #include "impeller/renderer/backend/gles/capabilities_gles.h" +#include "impeller/core/formats.h" #include "impeller/renderer/backend/gles/proc_table_gles.h" namespace impeller { @@ -102,6 +103,12 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { num_shader_binary_formats = value; } + if (desc->IsES()) { + default_glyph_atlas_format_ = PixelFormat::kA8UNormInt; + } else { + default_glyph_atlas_format_ = PixelFormat::kR8UNormInt; + } + supports_framebuffer_fetch_ = desc->HasExtension(kFramebufferFetchExt); if (desc->HasExtension(kTextureBorderClampExt) || @@ -188,4 +195,8 @@ PixelFormat CapabilitiesGLES::GetDefaultDepthStencilFormat() const { return PixelFormat::kD24UnormS8Uint; } +PixelFormat CapabilitiesGLES::GetDefaultGlyphAtlasFormat() const { + return default_glyph_atlas_format_; +} + } // namespace impeller diff --git a/impeller/renderer/backend/gles/capabilities_gles.h b/impeller/renderer/backend/gles/capabilities_gles.h index baa6d76d378c2..2124970770c0d 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.h +++ b/impeller/renderer/backend/gles/capabilities_gles.h @@ -8,6 +8,7 @@ #include #include "impeller/base/backend_cast.h" +#include "impeller/core/formats.h" #include "impeller/core/shader_types.h" #include "impeller/geometry/size.h" #include "impeller/renderer/capabilities.h" @@ -116,11 +117,16 @@ class CapabilitiesGLES final // |Capabilities| PixelFormat GetDefaultDepthStencilFormat() const override; + // |Capabilities| + PixelFormat GetDefaultGlyphAtlasFormat() const override; + private: bool supports_framebuffer_fetch_ = false; bool supports_decal_sampler_address_mode_ = false; bool supports_offscreen_msaa_ = false; bool supports_implicit_msaa_ = false; + + PixelFormat default_glyph_atlas_format_ = PixelFormat::kUnknown; }; } // namespace impeller diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 8b23d4bed64a8..623f3c8335f4c 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -67,6 +67,7 @@ static bool DeviceSupportsComputeSubgroups(id device) { .SetSupportsComputeSubgroups(DeviceSupportsComputeSubgroups(device)) .SetSupportsReadFromResolve(true) .SetSupportsDeviceTransientTextures(true) + .SetDefaultGlyphAtlasFormat(PixelFormat::kA8UNormInt) .Build(); } diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 812aae651a501..ab5214747b5b5 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -484,4 +484,8 @@ bool CapabilitiesVK::HasOptionalDeviceExtension( optional_device_extensions_.end(); } +PixelFormat CapabilitiesVK::GetDefaultGlyphAtlasFormat() const { + return PixelFormat::kR8UNormInt; +} + } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index b2dad43260b99..d3f5d70fbffee 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -100,6 +100,9 @@ class CapabilitiesVK final : public Capabilities, // |Capabilities| PixelFormat GetDefaultDepthStencilFormat() const override; + // |Capabilities| + PixelFormat GetDefaultGlyphAtlasFormat() const override; + private: bool validations_enabled_ = false; std::map> exts_; diff --git a/impeller/renderer/capabilities_unittests.cc b/impeller/renderer/capabilities_unittests.cc index 06c9bc3f82282..2692d067ad0a3 100644 --- a/impeller/renderer/capabilities_unittests.cc +++ b/impeller/renderer/capabilities_unittests.cc @@ -31,31 +31,40 @@ CAPABILITY_TEST(SupportsDeviceTransientTextures, false); TEST(CapabilitiesTest, DefaultColorFormat) { auto defaults = CapabilitiesBuilder().Build(); - ASSERT_EQ(defaults->GetDefaultColorFormat(), PixelFormat::kUnknown); + EXPECT_EQ(defaults->GetDefaultColorFormat(), PixelFormat::kUnknown); auto mutated = CapabilitiesBuilder() .SetDefaultColorFormat(PixelFormat::kB10G10R10A10XR) .Build(); - ASSERT_EQ(mutated->GetDefaultColorFormat(), PixelFormat::kB10G10R10A10XR); + EXPECT_EQ(mutated->GetDefaultColorFormat(), PixelFormat::kB10G10R10A10XR); } TEST(CapabilitiesTest, DefaultStencilFormat) { auto defaults = CapabilitiesBuilder().Build(); - ASSERT_EQ(defaults->GetDefaultStencilFormat(), PixelFormat::kUnknown); + EXPECT_EQ(defaults->GetDefaultStencilFormat(), PixelFormat::kUnknown); auto mutated = CapabilitiesBuilder() .SetDefaultStencilFormat(PixelFormat::kS8UInt) .Build(); - ASSERT_EQ(mutated->GetDefaultStencilFormat(), PixelFormat::kS8UInt); + EXPECT_EQ(mutated->GetDefaultStencilFormat(), PixelFormat::kS8UInt); } TEST(CapabilitiesTest, DefaultDepthStencilFormat) { auto defaults = CapabilitiesBuilder().Build(); - ASSERT_EQ(defaults->GetDefaultDepthStencilFormat(), PixelFormat::kUnknown); + EXPECT_EQ(defaults->GetDefaultDepthStencilFormat(), PixelFormat::kUnknown); auto mutated = CapabilitiesBuilder() .SetDefaultDepthStencilFormat(PixelFormat::kD32FloatS8UInt) .Build(); - ASSERT_EQ(mutated->GetDefaultDepthStencilFormat(), + EXPECT_EQ(mutated->GetDefaultDepthStencilFormat(), PixelFormat::kD32FloatS8UInt); } +TEST(CapabilitiesTest, DefaultGlyphAtlasFormat) { + auto defaults = CapabilitiesBuilder().Build(); + EXPECT_EQ(defaults->GetDefaultGlyphAtlasFormat(), PixelFormat::kUnknown); + auto mutated = CapabilitiesBuilder() + .SetDefaultGlyphAtlasFormat(PixelFormat::kA8UNormInt) + .Build(); + EXPECT_EQ(mutated->GetDefaultGlyphAtlasFormat(), PixelFormat::kA8UNormInt); +} + } // namespace testing } // namespace impeller From f456c76e1fc940f9171dc2aa39e8d335dcd4c582 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 13 Feb 2024 14:46:36 -0800 Subject: [PATCH 6/8] more updates for capability. --- impeller/aiks/aiks_blur_unittests.cc | 1 + impeller/entity/contents/text_contents.cc | 2 +- .../backend/gles/capabilities_gles.cc | 9 ++++----- .../backend/vulkan/debug_report_vk.cc | 9 --------- impeller/renderer/testing/mocks.h | 1 + impeller/typographer/glyph_atlas.cc | 2 +- impeller/typographer/glyph_atlas.h | 4 +++- impeller/typographer/lazy_glyph_atlas.cc | 10 +++++----- impeller/typographer/text_frame.cc | 2 +- impeller/typographer/typographer_unittests.cc | 20 +++++++++---------- 10 files changed, 27 insertions(+), 33 deletions(-) diff --git a/impeller/aiks/aiks_blur_unittests.cc b/impeller/aiks/aiks_blur_unittests.cc index 03151a3ffbe67..370eee168332d 100644 --- a/impeller/aiks/aiks_blur_unittests.cc +++ b/impeller/aiks/aiks_blur_unittests.cc @@ -354,6 +354,7 @@ TEST_P(AiksTest, GaussianBlurWithoutDecalSupport) { FLT_FORWARD(mock_capabilities, old_capabilities, SupportsCompute); FLT_FORWARD(mock_capabilities, old_capabilities, SupportsTextureToTextureBlits); + FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultGlyphAtlasFormat); ASSERT_TRUE(SetCapabilities(mock_capabilities).ok()); auto texture = std::make_shared(CreateTextureForFixture("boston.jpg")); diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 61610b9191cb7..b83d29a8a62cf 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -83,7 +83,7 @@ bool TextContents::Render(const ContentContext& renderer, pass.SetCommandLabel("TextFrame"); auto opts = OptionsFromPassAndEntity(pass, entity); opts.primitive_type = PrimitiveType::kTriangle; - if (type == GlyphAtlas::Type::kRedBitmap) { + if (type == GlyphAtlas::Type::kAlphaBitmap) { pass.SetPipeline(renderer.GetGlyphAtlasPipeline(opts)); } else { pass.SetPipeline(renderer.GetGlyphAtlasColorPipeline(opts)); diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc index 03e3145d0f0be..a67486bd37047 100644 --- a/impeller/renderer/backend/gles/capabilities_gles.cc +++ b/impeller/renderer/backend/gles/capabilities_gles.cc @@ -197,13 +197,12 @@ PixelFormat CapabilitiesGLES::GetDefaultDepthStencilFormat() const { return PixelFormat::kD24UnormS8Uint; } -<<<<<<< HEAD -PixelFormat CapabilitiesGLES::GetDefaultGlyphAtlasFormat() const { - return default_glyph_atlas_format_; -======= bool CapabilitiesGLES::IsANGLE() const { return is_angle_; ->>>>>>> 9b183870ff6ce2f928e86b45d82e06ddcef28423 +} + +PixelFormat CapabilitiesGLES::GetDefaultGlyphAtlasFormat() const { + return default_glyph_atlas_format_; } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/debug_report_vk.cc b/impeller/renderer/backend/vulkan/debug_report_vk.cc index 983c25ee80d47..fce16f8751d4b 100644 --- a/impeller/renderer/backend/vulkan/debug_report_vk.cc +++ b/impeller/renderer/backend/vulkan/debug_report_vk.cc @@ -111,15 +111,6 @@ DebugReportVK::Result DebugReportVK::OnDebugCallback( return Result::kContinue; } - // This is a real error but we can't fix it due to our headers being too - // old. More more details see: - // https://vulkan.lunarg.com/doc/view/1.3.224.1/mac/1.3-extensions/vkspec.html#VUID-VkImageViewCreateInfo-imageViewFormatSwizzle-04465 - // This validation error currently only trips on macOS due to the use of - // texture swizzles. - if (data->messageIdNumber == static_cast(0x82ae5050)) { - return Result::kContinue; - } - std::vector> items; items.emplace_back("Severity", vk::to_string(severity)); diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index 78b1d388ea67e..45b519cc3c937 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -204,6 +204,7 @@ class MockCapabilities : public Capabilities { MOCK_METHOD(PixelFormat, GetDefaultColorFormat, (), (const, override)); MOCK_METHOD(PixelFormat, GetDefaultStencilFormat, (), (const, override)); MOCK_METHOD(PixelFormat, GetDefaultDepthStencilFormat, (), (const, override)); + MOCK_METHOD(PixelFormat, GetDefaultGlyphAtlasFormat, (), (const, override)); }; class MockCommandQueue : public CommandQueue { diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 76ed49464c7d1..1f9bfaff8d7ae 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -10,7 +10,7 @@ namespace impeller { GlyphAtlasContext::GlyphAtlasContext() - : atlas_(std::make_shared(GlyphAtlas::Type::kRedBitmap)), + : atlas_(std::make_shared(GlyphAtlas::Type::kAlphaBitmap)), atlas_size_(ISize(0, 0)) {} GlyphAtlasContext::~GlyphAtlasContext() {} diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 92600062175e2..9df7eecd4bd71 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -32,8 +32,10 @@ class GlyphAtlas { enum class Type { //-------------------------------------------------------------------------- /// The glyphs are reprsented at their requested size using only an 8-bit - /// alpha or red channel. + /// color channel. /// + /// This might be backed by a grey or red single channel texture, depending + /// on the backend capabilities. kAlphaBitmap, //-------------------------------------------------------------------------- diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index efe6229022a53..3617169f791ea 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -30,7 +30,7 @@ LazyGlyphAtlas::~LazyGlyphAtlas() = default; void LazyGlyphAtlas::AddTextFrame(const TextFrame& frame, Scalar scale) { FML_DCHECK(alpha_atlas_ == nullptr && color_atlas_ == nullptr); - if (frame.GetAtlasType() == GlyphAtlas::Type::kRedBitmap) { + if (frame.GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) { frame.CollectUniqueFontGlyphPairs(alpha_glyph_map_, scale); } else { frame.CollectUniqueFontGlyphPairs(color_glyph_map_, scale); @@ -48,7 +48,7 @@ const std::shared_ptr& LazyGlyphAtlas::CreateOrGetGlyphAtlas( Context& context, GlyphAtlas::Type type) const { { - if (type == GlyphAtlas::Type::kRedBitmap && alpha_atlas_) { + if (type == GlyphAtlas::Type::kAlphaBitmap && alpha_atlas_) { return alpha_atlas_; } if (type == GlyphAtlas::Type::kColorBitmap && color_atlas_) { @@ -67,17 +67,17 @@ const std::shared_ptr& LazyGlyphAtlas::CreateOrGetGlyphAtlas( return kNullGlyphAtlas; } - auto& glyph_map = type == GlyphAtlas::Type::kRedBitmap ? alpha_glyph_map_ + auto& glyph_map = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_glyph_map_ : color_glyph_map_; const std::shared_ptr& atlas_context = - type == GlyphAtlas::Type::kRedBitmap ? alpha_context_ : color_context_; + type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_; std::shared_ptr atlas = typographer_context_->CreateGlyphAtlas( context, type, atlas_context, glyph_map); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Could not create valid atlas."; return kNullGlyphAtlas; } - if (type == GlyphAtlas::Type::kRedBitmap) { + if (type == GlyphAtlas::Type::kAlphaBitmap) { alpha_atlas_ = std::move(atlas); return alpha_atlas_; } diff --git a/impeller/typographer/text_frame.cc b/impeller/typographer/text_frame.cc index d9df5d943c47e..5e38b68a76729 100644 --- a/impeller/typographer/text_frame.cc +++ b/impeller/typographer/text_frame.cc @@ -27,7 +27,7 @@ const std::vector& TextFrame::GetRuns() const { GlyphAtlas::Type TextFrame::GetAtlasType() const { return has_color_ ? GlyphAtlas::Type::kColorBitmap - : GlyphAtlas::Type::kRedBitmap; + : GlyphAtlas::Type::kAlphaBitmap; } bool TextFrame::MaybeHasOverlapping() const { diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 18d88ca3a5913..4fc6fc58a7041 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -64,11 +64,11 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); - ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kRedBitmap); + ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); ASSERT_EQ(atlas->GetGlyphCount(), 4llu); std::optional first_scaled_font; @@ -122,7 +122,7 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { *GetContext(), GlyphAtlas::Type::kColorBitmap); auto bitmap_atlas = lazy_atlas.CreateOrGetGlyphAtlas( - *GetContext(), GlyphAtlas::Type::kRedBitmap); + *GetContext(), GlyphAtlas::Type::kAlphaBitmap); ASSERT_FALSE(color_atlas == bitmap_atlas); } @@ -135,7 +135,7 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { auto blob = SkTextBlob::MakeFromString("AGH", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -152,7 +152,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { auto blob = SkTextBlob::MakeFromString("spooky skellingtons", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -161,7 +161,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { // now attempt to re-create an atlas with the same text blob. auto next_atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); @@ -189,7 +189,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { font_glyph_map, 0.6 * index); }; auto atlas = - context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kRedBitmap, + context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kAlphaBitmap, atlas_context, font_glyph_map); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -219,7 +219,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); auto old_packer = atlas_context->GetRectPacker(); @@ -233,7 +233,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); auto next_atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob2)); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -252,7 +252,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( - *GetContext(), context.get(), GlyphAtlas::Type::kRedBitmap, 1.0f, + *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); auto old_packer = atlas_context->GetRectPacker(); From 53116aa5141649ad2bf07533cc51659a9cb9f00e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 13 Feb 2024 17:10:26 -0800 Subject: [PATCH 7/8] use specialization constant. --- impeller/entity/contents/content_context.cc | 6 +++++- impeller/entity/shaders/glyph_atlas.frag | 8 +++++++- .../typographer/backends/skia/typographer_context_skia.cc | 2 +- .../typographer/backends/stb/typographer_context_stb.cc | 7 ++++--- impeller/typographer/lazy_glyph_atlas.cc | 2 +- 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index bde5a604b62ce..1d73fcff072b0 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -404,7 +404,11 @@ ContentContext::ContentContext( options_trianglestrip); srgb_to_linear_filter_pipelines_.CreateDefault(*context_, options_trianglestrip); - glyph_atlas_pipelines_.CreateDefault(*context_, options); + glyph_atlas_pipelines_.CreateDefault( + *context_, options, + {static_cast( + GetContext()->GetCapabilities()->GetDefaultGlyphAtlasFormat() == + PixelFormat::kA8UNormInt)}); glyph_atlas_color_pipelines_.CreateDefault(*context_, options); geometry_color_pipelines_.CreateDefault(*context_, options); yuv_to_rgb_filter_pipelines_.CreateDefault(*context_, options_trianglestrip); diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag index d0bca0579cee4..8db425d0e97ff 100644 --- a/impeller/entity/shaders/glyph_atlas.frag +++ b/impeller/entity/shaders/glyph_atlas.frag @@ -8,6 +8,8 @@ precision mediump float; uniform f16sampler2D glyph_atlas_sampler; +layout(constant_id = 0) const float use_alpha_color_channel = 1.0; + in highp vec2 v_uv; IMPELLER_MAYBE_FLAT in f16vec4 v_text_color; @@ -16,5 +18,9 @@ out f16vec4 frag_color; void main() { f16vec4 value = texture(glyph_atlas_sampler, v_uv); - frag_color = vec4(max(value.r, value.a)) * v_text_color; + if (use_alpha_color_channel == 1.0) { + frag_color = value.aaaa * v_text_color; + } else { + frag_color = value.rrrr * v_text_color; + } } diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 0c8d191d53872..b6f266dbed48b 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -470,7 +470,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( PixelFormat format; switch (type) { case GlyphAtlas::Type::kAlphaBitmap: - format = PixelFormat::kR8UNormInt; + format = context.GetCapabilities()->GetDefaultGlyphAtlasFormat(); break; case GlyphAtlas::Type::kColorBitmap: format = PixelFormat::kR8G8B8A8UNormInt; diff --git a/impeller/typographer/backends/stb/typographer_context_stb.cc b/impeller/typographer/backends/stb/typographer_context_stb.cc index 0f1db9f4190c2..6c5bcad667483 100644 --- a/impeller/typographer/backends/stb/typographer_context_stb.cc +++ b/impeller/typographer/backends/stb/typographer_context_stb.cc @@ -516,11 +516,12 @@ std::shared_ptr TypographerContextSTB::CreateGlyphAtlas( PixelFormat format; switch (type) { case GlyphAtlas::Type::kAlphaBitmap: - format = PixelFormat::kR8UNormInt; + format = context.GetCapabilities()->GetDefaultGlyphAtlasFormat(); break; case GlyphAtlas::Type::kColorBitmap: - format = DISABLE_COLOR_FONT_SUPPORT ? PixelFormat::kR8UNormInt - : PixelFormat::kR8G8B8A8UNormInt; + format = DISABLE_COLOR_FONT_SUPPORT + ? context.GetCapabilities()->GetDefaultGlyphAtlasFormat() + : PixelFormat::kR8G8B8A8UNormInt; break; } auto texture = UploadGlyphTextureAtlas(context.GetResourceAllocator(), bitmap, diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index 3617169f791ea..121e6186c9e09 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -68,7 +68,7 @@ const std::shared_ptr& LazyGlyphAtlas::CreateOrGetGlyphAtlas( } auto& glyph_map = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_glyph_map_ - : color_glyph_map_; + : color_glyph_map_; const std::shared_ptr& atlas_context = type == GlyphAtlas::Type::kAlphaBitmap ? alpha_context_ : color_context_; std::shared_ptr atlas = typographer_context_->CreateGlyphAtlas( From 42c7e0f77ab4611847df921278cd4f5f554d7540 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 13 Feb 2024 17:59:28 -0800 Subject: [PATCH 8/8] add missing assignment. --- impeller/renderer/capabilities.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/capabilities.cc b/impeller/renderer/capabilities.cc index 0ad4ff9a61de5..1ef1fe38a8062 100644 --- a/impeller/renderer/capabilities.cc +++ b/impeller/renderer/capabilities.cc @@ -113,7 +113,8 @@ class StandardCapabilities final : public Capabilities { supports_device_transient_textures_(supports_device_transient_textures), default_color_format_(default_color_format), default_stencil_format_(default_stencil_format), - default_depth_stencil_format_(default_depth_stencil_format) {} + default_depth_stencil_format_(default_depth_stencil_format), + default_glyph_atlas_format_(default_glyph_atlas_format) {} friend class CapabilitiesBuilder;