From 11f224bfdbef13b40ed0dbf429d3b46f3486ca5d Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 16 Sep 2022 08:47:45 -0700 Subject: [PATCH] Revert "[Impeller] add support for COLR fonts and bitmap emojis (#36161)" This reverts commit 61896b3bc756f716f1cc182c5f1bd98bcbb3d7a2. --- impeller/entity/contents/text_contents.cc | 17 +++----- impeller/entity/shaders/glyph_atlas.frag | 29 ++++--------- impeller/entity/shaders/glyph_atlas.vert | 9 ++-- .../backends/skia/text_render_context_skia.cc | 42 +++++++------------ impeller/typographer/glyph_atlas.cc | 24 ----------- impeller/typographer/glyph_atlas.h | 27 ------------ impeller/typographer/typographer_unittests.cc | 1 - 7 files changed, 34 insertions(+), 115 deletions(-) diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 6cc0c292219e7..3768dc05efe15 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -4,7 +4,6 @@ #include "impeller/entity/contents/text_contents.h" -#include #include #include "impeller/entity/contents/content_context.h" @@ -90,19 +89,16 @@ bool TextContents::Render(const ContentContext& renderer, VS::FrameInfo frame_info; frame_info.mvp = Matrix::MakeOrthographic(pass.GetRenderTargetSize()) * entity.GetTransformation(); + frame_info.atlas_size = + Point{static_cast(atlas->GetTexture()->GetSize().width), + static_cast(atlas->GetTexture()->GetSize().height)}; + frame_info.text_color = ToVector(color_.Premultiply()); VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info)); SamplerDescriptor sampler_desc; sampler_desc.min_filter = MinMagFilter::kLinear; sampler_desc.mag_filter = MinMagFilter::kLinear; - FS::FragInfo frag_info; - frag_info.text_color = ToVector(color_.Premultiply()); - frag_info.atlas_size = - Point{static_cast(atlas->GetTexture()->GetSize().width), - static_cast(atlas->GetTexture()->GetSize().height)}; - FS::BindFragInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frag_info)); - // Common fragment uniforms for all glyphs. FS::BindGlyphAtlasSampler( cmd, // command @@ -127,13 +123,11 @@ bool TextContents::Render(const ContentContext& renderer, auto font = run.GetFont(); auto glyph_size = ISize::Ceil(font.GetMetrics().GetBoundingBox().size); for (const auto& glyph_position : run.GetGlyphPositions()) { - FontGlyphPair font_glyph_pair{font, glyph_position.glyph}; - auto color_glyph = - atlas->IsColorFontGlyphPair(font_glyph_pair) ? 1.0 : 0.0; for (const auto& point : unit_vertex_points) { VS::PerVertexData vtx; vtx.unit_vertex = point; + FontGlyphPair font_glyph_pair{font, glyph_position.glyph}; auto atlas_glyph_pos = atlas->FindFontGlyphPosition(font_glyph_pair); if (!atlas_glyph_pos.has_value()) { VALIDATION_LOG << "Could not find glyph position in the atlas."; @@ -149,7 +143,6 @@ bool TextContents::Render(const ContentContext& renderer, 1 / atlas_glyph_pos->size.height}; vtx.atlas_glyph_size = Point{atlas_glyph_pos->size.width, atlas_glyph_pos->size.height}; - vtx.color_glyph = color_glyph; vertex_builder.AppendVertex(std::move(vtx)); } } diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag index ea8162c3e865e..f41d9a181d899 100644 --- a/impeller/entity/shaders/glyph_atlas.frag +++ b/impeller/entity/shaders/glyph_atlas.frag @@ -4,31 +4,20 @@ uniform sampler2D glyph_atlas_sampler; -uniform FragInfo { - vec2 atlas_size; - vec4 text_color; - float font_has_color; -} frag_info; - in vec2 v_unit_vertex; in vec2 v_atlas_position; in vec2 v_atlas_glyph_size; -in float v_color_glyph; +in vec2 v_atlas_size; +in vec4 v_text_color; out vec4 frag_color; void main() { - vec2 scale_perspective = v_atlas_glyph_size / frag_info.atlas_size; - vec2 offset = v_atlas_position / frag_info.atlas_size; - if (v_color_glyph == 1.0) { - frag_color = texture( - glyph_atlas_sampler, - v_unit_vertex * scale_perspective + offset - ); - } else { - frag_color = texture( - glyph_atlas_sampler, - v_unit_vertex * scale_perspective + offset - ).aaaa * frag_info.text_color; - } + vec2 scale_perspective = v_atlas_glyph_size / v_atlas_size; + vec2 offset = v_atlas_position / v_atlas_size; + + frag_color = texture( + glyph_atlas_sampler, + v_unit_vertex * scale_perspective + offset + ).aaaa * v_text_color; } diff --git a/impeller/entity/shaders/glyph_atlas.vert b/impeller/entity/shaders/glyph_atlas.vert index 1ac172c082909..1e1b04b637fc4 100644 --- a/impeller/entity/shaders/glyph_atlas.vert +++ b/impeller/entity/shaders/glyph_atlas.vert @@ -4,6 +4,8 @@ uniform FrameInfo { mat4 mvp; + vec2 atlas_size; + vec4 text_color; } frame_info; in vec2 unit_vertex; @@ -11,12 +13,12 @@ in vec2 glyph_position; in vec2 glyph_size; in vec2 atlas_position; in vec2 atlas_glyph_size; -in float color_glyph; out vec2 v_unit_vertex; out vec2 v_atlas_position; out vec2 v_atlas_glyph_size; -out float v_color_glyph; +out vec2 v_atlas_size; +out vec4 v_text_color; void main() { vec4 translate = frame_info.mvp[0] * glyph_position.x @@ -38,5 +40,6 @@ void main() { v_unit_vertex = unit_vertex; v_atlas_position = atlas_position; v_atlas_glyph_size = atlas_glyph_size; - v_color_glyph = color_glyph; + v_atlas_size = frame_info.atlas_size; + v_text_color = frame_info.text_color; } diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index d6e718c3a64f5..aab49ed2fece9 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -15,9 +15,8 @@ #include "third_party/skia/include/core/SkFontMetrics.h" #include "third_party/skia/include/core/SkRSXform.h" #include "third_party/skia/include/core/SkSurface.h" -#include "third_party/skia/src/core/SkIPoint16.h" //nogncheck -#include "third_party/skia/src/core/SkStrikeSpec.h" // nogncheck -#include "third_party/skia/src/gpu/GrRectanizer.h" //nogncheck +#include "third_party/skia/src/core/SkIPoint16.h" //nogncheck +#include "third_party/skia/src/gpu/GrRectanizer.h" //nogncheck namespace impeller { @@ -108,9 +107,7 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, size_t atlas_size) { TRACE_EVENT0("impeller", __FUNCTION__); auto bitmap = std::make_shared(); - auto image_info = atlas.ContainsColorGlyph() - ? SkImageInfo::MakeN32Premul(atlas_size, atlas_size) - : SkImageInfo::MakeA8(atlas_size, atlas_size); + auto image_info = SkImageInfo::MakeA8(atlas_size, atlas_size); if (!bitmap->tryAllocPixels(image_info)) { return nullptr; } @@ -125,21 +122,24 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, atlas.IterateGlyphs([canvas](const FontGlyphPair& font_glyph, const Rect& location) -> bool { - const auto& metrics = font_glyph.font.GetMetrics(); - const auto position = SkPoint::Make(location.origin.x / metrics.scale, - location.origin.y / metrics.scale); + const auto position = + SkPoint::Make(location.origin.x / font_glyph.font.GetMetrics().scale, + location.origin.y / font_glyph.font.GetMetrics().scale); SkGlyphID glyph_id = font_glyph.glyph.index; SkFont sk_font( TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(), - metrics.point_size); + font_glyph.font.GetMetrics().point_size); + + const auto& metrics = font_glyph.font.GetMetrics(); auto glyph_color = SK_ColorWHITE; SkPaint glyph_paint; glyph_paint.setColor(glyph_color); canvas->resetMatrix(); - canvas->scale(metrics.scale, metrics.scale); + canvas->scale(font_glyph.font.GetMetrics().scale, + font_glyph.font.GetMetrics().scale); canvas->drawGlyphs(1u, // count &glyph_id, // glyphs &position, // positions @@ -157,8 +157,7 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, static std::shared_ptr UploadGlyphTextureAtlas( std::shared_ptr allocator, std::shared_ptr bitmap, - size_t atlas_size, - PixelFormat format) { + size_t atlas_size) { TRACE_EVENT0("impeller", __FUNCTION__); if (!allocator) { return nullptr; @@ -169,7 +168,7 @@ static std::shared_ptr UploadGlyphTextureAtlas( TextureDescriptor texture_descriptor; texture_descriptor.storage_mode = StorageMode::kHostVisible; - texture_descriptor.format = format; + texture_descriptor.format = PixelFormat::kA8UNormInt; texture_descriptor.size = ISize::MakeWH(atlas_size, atlas_size); if (pixmap.rowBytes() * pixmap.height() != @@ -203,16 +202,6 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( } auto glyph_atlas = std::make_shared(); - glyph_atlas->SetFontColorCallback([](const FontGlyphPair& font_glyph) { - // TODO(jonahwilliams): ask Skia for a public API to look this up. - SkFont sk_font( - TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(), - font_glyph.font.GetMetrics().point_size); - - SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(sk_font); - SkBulkGlyphMetricsAndPaths paths{strikeSpec}; - return paths.glyph(font_glyph.glyph.index)->isColor(); - }); // --------------------------------------------------------------------------- // Step 1: Collect unique font-glyph pairs in the frame. @@ -262,11 +251,8 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 6: Upload the atlas as a texture. // --------------------------------------------------------------------------- - auto format = glyph_atlas->ContainsColorGlyph() - ? PixelFormat::kR8G8B8A8UNormInt - : PixelFormat::kA8UNormInt; auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(), - bitmap, atlas_size, format); + bitmap, atlas_size); if (!texture) { return nullptr; } diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 0248aa8e5bc12..7ed4578bbd923 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "impeller/typographer/glyph_atlas.h" -#include namespace impeller { @@ -15,10 +14,6 @@ bool GlyphAtlas::IsValid() const { return !!texture_; } -bool GlyphAtlas::ContainsColorGlyph() const { - return has_color_glyph; -} - const std::shared_ptr& GlyphAtlas::GetTexture() const { return texture_; } @@ -28,28 +23,9 @@ void GlyphAtlas::SetTexture(std::shared_ptr texture) { } void GlyphAtlas::AddTypefaceGlyphPosition(FontGlyphPair pair, Rect rect) { - if (callback_.has_value()) { - auto has_color = callback_.value()(pair); - has_color_glyph |= has_color; - colors_[pair] = has_color; - } - positions_[pair] = rect; } -void GlyphAtlas::SetFontColorCallback( - std::function callback) { - callback_ = std::move(callback); -} - -bool GlyphAtlas::IsColorFontGlyphPair(const FontGlyphPair& pair) const { - auto found = colors_.find(pair); - if (found == colors_.end()) { - return false; - } - return found->second; -} - std::optional GlyphAtlas::FindFontGlyphPosition( const FontGlyphPair& pair) const { auto found = positions_.find(pair); diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index cec7222ae9e5b..896ca7c8f2d6e 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -32,11 +32,6 @@ class GlyphAtlas { bool IsValid() const; - //---------------------------------------------------------------------------- - /// @brief Whether at least one font-glyph pair has colors. - /// - bool ContainsColorGlyph() const; - //---------------------------------------------------------------------------- /// @brief Set the texture for the glyph atlas. /// @@ -44,20 +39,6 @@ class GlyphAtlas { /// void SetTexture(std::shared_ptr texture); - //---------------------------------------------------------------------------- - /// @brief Set a callback that determines if a glyph-font pair - /// has color. - /// - /// @param[in] callback The callback - /// - void SetFontColorCallback( - std::function callback); - - //---------------------------------------------------------------------------- - /// @brief Whether the provided glyph-font pair contains color. - /// - bool IsColorFontGlyphPair(const FontGlyphPair& pair) const; - //---------------------------------------------------------------------------- /// @brief Get the texture for the glyph atlas. /// @@ -105,8 +86,6 @@ class GlyphAtlas { private: std::shared_ptr texture_; - std::optional> callback_; - bool has_color_glyph = false; std::unordered_map positions_; - std::unordered_map - colors_; - FML_DISALLOW_COPY_AND_ASSIGN(GlyphAtlas); }; diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index d0a0e318b76a1..80b17678ab201 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -40,7 +40,6 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { ASSERT_TRUE(blob); auto atlas = context->CreateGlyphAtlas(TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); - ASSERT_FALSE(atlas->ContainsColorGlyph()); OpenPlaygroundHere([](RenderTarget&) { return true; }); }