From aa966cb2f1e5bcdacbc1b1a3d032ed8db94c09e4 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 5 May 2023 16:43:03 -0700 Subject: [PATCH] [Impeller] remove SDF code paths. (flutter/engine#41754) From my investigations, we're not likely to be able to use SDF in the near term. To make planned refactors of the text rendering easier to land, I've moved the primary piece of code for generating the SDFs into a new TU for testing. The shader itself has been deleted so that we don't ship + register it. --- .../entity/contents/content_context.cc | 2 - .../entity/contents/content_context.h | 10 -- .../impeller/entity/contents/text_contents.cc | 50 ++----- .../impeller/entity/contents/text_contents.h | 5 - .../impeller/entity/entity_unittests.cc | 28 ---- .../backends/skia/text_render_context_skia.cc | 123 ------------------ .../impeller/typographer/glyph_atlas.h | 7 - 7 files changed, 11 insertions(+), 214 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/content_context.cc b/engine/src/flutter/impeller/entity/contents/content_context.cc index a8f1e8e5415..48257e49400 100644 --- a/engine/src/flutter/impeller/entity/contents/content_context.cc +++ b/engine/src/flutter/impeller/entity/contents/content_context.cc @@ -286,8 +286,6 @@ ContentContext::ContentContext(std::shared_ptr context) CreateDefaultPipeline(*context_); glyph_atlas_pipelines_[{}] = CreateDefaultPipeline(*context_); - glyph_atlas_sdf_pipelines_[{}] = - CreateDefaultPipeline(*context_); geometry_color_pipelines_[{}] = CreateDefaultPipeline(*context_); yuv_to_rgb_filter_pipelines_[{}] = diff --git a/engine/src/flutter/impeller/entity/contents/content_context.h b/engine/src/flutter/impeller/entity/contents/content_context.h index 65941ceda19..971b0cc4b08 100644 --- a/engine/src/flutter/impeller/entity/contents/content_context.h +++ b/engine/src/flutter/impeller/entity/contents/content_context.h @@ -32,8 +32,6 @@ #include "impeller/entity/conical_gradient_fill.frag.h" #include "impeller/entity/glyph_atlas.frag.h" #include "impeller/entity/glyph_atlas.vert.h" -#include "impeller/entity/glyph_atlas_sdf.frag.h" -#include "impeller/entity/glyph_atlas_sdf.vert.h" #include "impeller/entity/gradient_fill.vert.h" #include "impeller/entity/linear_gradient_fill.frag.h" #include "impeller/entity/linear_to_srgb_filter.frag.h" @@ -173,8 +171,6 @@ using SrgbToLinearFilterPipeline = SrgbToLinearFilterFragmentShader>; using GlyphAtlasPipeline = RenderPipelineT; -using GlyphAtlasSdfPipeline = - RenderPipelineT; using PorterDuffBlendPipeline = RenderPipelineT; // Instead of requiring new shaders for clips, the solid fill stages are used @@ -473,11 +469,6 @@ class ContentContext { return GetPipeline(glyph_atlas_pipelines_, opts); } - std::shared_ptr> GetGlyphAtlasSdfPipeline( - ContentContextOptions opts) const { - return GetPipeline(glyph_atlas_sdf_pipelines_, opts); - } - std::shared_ptr> GetGeometryColorPipeline( ContentContextOptions opts) const { return GetPipeline(geometry_color_pipelines_, opts); @@ -731,7 +722,6 @@ class ContentContext { mutable Variants srgb_to_linear_filter_pipelines_; mutable Variants clip_pipelines_; mutable Variants glyph_atlas_pipelines_; - mutable Variants glyph_atlas_sdf_pipelines_; mutable Variants geometry_color_pipelines_; mutable Variants yuv_to_rgb_filter_pipelines_; mutable Variants porter_duff_blend_pipelines_; diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.cc b/engine/src/flutter/impeller/entity/contents/text_contents.cc index 6a4f0be4753..1fba560b176 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/text_contents.cc @@ -74,7 +74,6 @@ std::optional TextContents::GetCoverage(const Entity& entity) const { return bounds->TransformBounds(entity.GetTransformation()); } -template static bool CommonRender( const ContentContext& renderer, const Entity& entity, @@ -85,11 +84,11 @@ static bool CommonRender( std::shared_ptr atlas, // NOLINT(performance-unnecessary-value-param) Command& cmd) { - using VS = typename TPipeline::VertexShader; - using FS = typename TPipeline::FragmentShader; + using VS = GlyphAtlasPipeline::VertexShader; + using FS = GlyphAtlasPipeline::FragmentShader; // Common vertex uniforms for all glyphs. - typename VS::FrameInfo frame_info; + VS::FrameInfo frame_info; frame_info.mvp = Matrix::MakeOrthographic(pass.GetRenderTargetSize()); VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info)); @@ -109,7 +108,7 @@ static bool CommonRender( } sampler_desc.mip_filter = MipFilter::kNearest; - typename FS::FragInfo frag_info; + FS::FragInfo frag_info; frag_info.text_color = ToVector(color.Premultiply()); FS::BindFragInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frag_info)); @@ -183,7 +182,7 @@ static bool CommonRender( .Round(); for (const auto& point : unit_points) { - typename VS::PerVertexData vtx; + VS::PerVertexData vtx; if (entity.GetTransformation().IsTranslationScaleOnly()) { // Rouding up here prevents the bounds from becoming 1 pixel too small @@ -199,19 +198,16 @@ static bool CommonRender( point * glyph_position.glyph.bounds.size); } vtx.uv = uv_origin + point * uv_size; + vtx.has_color = + glyph_position.glyph.type == Glyph::Type::kBitmap ? 1.0 : 0.0; - if constexpr (std::is_same_v) { - vtx.has_color = - glyph_position.glyph.type == Glyph::Type::kBitmap ? 1.0 : 0.0; - } - - vertex_builder.AppendVertex(std::move(vtx)); + vertex_builder.AppendVertex(vtx); } } } auto vertex_buffer = vertex_builder.CreateVertexBuffer(pass.GetTransientsBuffer()); - cmd.BindVertices(std::move(vertex_buffer)); + cmd.BindVertices(vertex_buffer); if (!pass.AddCommand(cmd)) { return false; @@ -220,30 +216,6 @@ static bool CommonRender( return true; } -bool TextContents::RenderSdf(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const { - auto atlas = - ResolveAtlas(GlyphAtlas::Type::kSignedDistanceField, - renderer.GetGlyphAtlasContext(), renderer.GetContext()); - - if (!atlas || !atlas->IsValid()) { - VALIDATION_LOG << "Cannot render glyphs without prepared atlas."; - return false; - } - - // Information shared by all glyph draw calls. - Command cmd; - cmd.label = "TextFrameSDF"; - auto opts = OptionsFromPassAndEntity(pass, entity); - opts.primitive_type = PrimitiveType::kTriangle; - cmd.pipeline = renderer.GetGlyphAtlasSdfPipeline(opts); - cmd.stencil_reference = entity.GetStencilDepth(); - - return CommonRender(renderer, entity, pass, GetColor(), - frame_, offset_, atlas, cmd); -} - bool TextContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { @@ -276,8 +248,8 @@ bool TextContents::Render(const ContentContext& renderer, cmd.pipeline = renderer.GetGlyphAtlasPipeline(opts); cmd.stencil_reference = entity.GetStencilDepth(); - return CommonRender(renderer, entity, pass, color, frame_, - offset_, atlas, cmd); + return CommonRender(renderer, entity, pass, color, frame_, offset_, atlas, + cmd); } } // namespace impeller diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.h b/engine/src/flutter/impeller/entity/contents/text_contents.h index b32f37258d9..e313abad9b7 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.h +++ b/engine/src/flutter/impeller/entity/contents/text_contents.h @@ -48,11 +48,6 @@ class TextContents final : public Contents { const Entity& entity, RenderPass& pass) const override; - // TODO(dnfield): remove this https://github.com/flutter/flutter/issues/111640 - bool RenderSdf(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) const; - private: TextFrame frame_; Color color_; diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index 007c6757a16..77acc57dd91 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/engine/src/flutter/impeller/entity/entity_unittests.cc @@ -2151,34 +2151,6 @@ TEST_P(EntityTest, TTTBlendColor) { } } -TEST_P(EntityTest, SdfText) { - auto callback = [&](ContentContext& context, RenderPass& pass) -> bool { - SkFont font; - font.setSize(30); - auto blob = SkTextBlob::MakeFromString( - "the quick brown fox jumped over the lazy dog (but with sdf).", font); - auto frame = TextFrameFromTextBlob(blob); - auto lazy_glyph_atlas = std::make_shared(); - lazy_glyph_atlas->AddTextFrame(frame); - - EXPECT_FALSE(lazy_glyph_atlas->HasColor()); - - auto text_contents = std::make_shared(); - text_contents->SetTextFrame(frame); - text_contents->SetGlyphAtlas(std::move(lazy_glyph_atlas)); - text_contents->SetColor(Color(1.0, 0.0, 0.0, 1.0)); - Entity entity; - entity.SetTransformation( - Matrix::MakeTranslation(Vector3{200.0, 200.0, 0.0}) * - Matrix::MakeScale(GetContentScale())); - entity.SetContents(text_contents); - - // Force SDF rendering. - return text_contents->RenderSdf(context, entity, pass); - }; - ASSERT_TRUE(OpenPlaygroundHere(callback)); -} - TEST_P(EntityTest, AtlasContentsSubAtlas) { auto boston = CreateTextureForFixture("boston.jpg"); diff --git a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc index 51ec87b712d..886674ea456 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/engine/src/flutter/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -49,9 +49,6 @@ static FontGlyphPair::Set CollectUniqueFontGlyphPairs( while (const TextFrame* frame = frame_iterator()) { for (const TextRun& run : frame->GetRuns()) { const Font& font = run.GetFont(); - // TODO(dnfield): If we're doing SDF here, we should be using a consistent - // point size. - // https://github.com/flutter/flutter/issues/112016 for (const TextRun::GlyphPosition& glyph_position : run.GetGlyphPositions()) { set.insert({font, glyph_position.glyph}); @@ -171,121 +168,6 @@ ISize OptimumAtlasSizeForFontGlyphPairs( } } // namespace -/// Compute signed-distance field for an 8-bpp grayscale image (values greater -/// than 127 are considered "on") For details of this algorithm, see "The 'dead -/// reckoning' signed distance transform" [Grevera 2004] -static void ConvertBitmapToSignedDistanceField(uint8_t* pixels, - uint16_t width, - uint16_t height) { - if (!pixels || width == 0 || height == 0) { - return; - } - - using ShortPoint = TPoint; - - // distance to nearest boundary point map - std::vector distance_map(width * height); - // nearest boundary point map - std::vector boundary_point_map(width * height); - - // Some helpers for manipulating the above arrays -#define image(_x, _y) (pixels[(_y)*width + (_x)] > 0x7f) -#define distance(_x, _y) distance_map[(_y)*width + (_x)] -#define nearestpt(_x, _y) boundary_point_map[(_y)*width + (_x)] - - const Scalar maxDist = hypot(width, height); - const Scalar distUnit = 1; - const Scalar distDiag = sqrt(2); - - // Initialization phase: set all distances to "infinity"; zero out nearest - // boundary point map - for (uint16_t y = 0; y < height; ++y) { - for (uint16_t x = 0; x < width; ++x) { - distance(x, y) = maxDist; - nearestpt(x, y) = ShortPoint{0, 0}; - } - } - - // Immediate interior/exterior phase: mark all points along the boundary as - // such - for (uint16_t y = 1; y < height - 1; ++y) { - for (uint16_t x = 1; x < width - 1; ++x) { - bool inside = image(x, y); - if (image(x - 1, y) != inside || image(x + 1, y) != inside || - image(x, y - 1) != inside || image(x, y + 1) != inside) { - distance(x, y) = 0; - nearestpt(x, y) = ShortPoint{x, y}; - } - } - } - - // Forward dead-reckoning pass - for (uint16_t y = 1; y < height - 2; ++y) { - for (uint16_t x = 1; x < width - 2; ++x) { - if (distance_map[(y - 1) * width + (x - 1)] + distDiag < distance(x, y)) { - nearestpt(x, y) = nearestpt(x - 1, y - 1); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - if (distance(x, y - 1) + distUnit < distance(x, y)) { - nearestpt(x, y) = nearestpt(x, y - 1); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - if (distance(x + 1, y - 1) + distDiag < distance(x, y)) { - nearestpt(x, y) = nearestpt(x + 1, y - 1); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - if (distance(x - 1, y) + distUnit < distance(x, y)) { - nearestpt(x, y) = nearestpt(x - 1, y); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - } - } - - // Backward dead-reckoning pass - for (uint16_t y = height - 2; y >= 1; --y) { - for (uint16_t x = width - 2; x >= 1; --x) { - if (distance(x + 1, y) + distUnit < distance(x, y)) { - nearestpt(x, y) = nearestpt(x + 1, y); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - if (distance(x - 1, y + 1) + distDiag < distance(x, y)) { - nearestpt(x, y) = nearestpt(x - 1, y + 1); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - if (distance(x, y + 1) + distUnit < distance(x, y)) { - nearestpt(x, y) = nearestpt(x, y + 1); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - if (distance(x + 1, y + 1) + distDiag < distance(x, y)) { - nearestpt(x, y) = nearestpt(x + 1, y + 1); - distance(x, y) = hypot(x - nearestpt(x, y).x, y - nearestpt(x, y).y); - } - } - } - - // Interior distance negation pass; distances outside the figure are - // considered negative - // Also does final quantization. - for (uint16_t y = 0; y < height; ++y) { - for (uint16_t x = 0; x < width; ++x) { - if (!image(x, y)) { - distance(x, y) = -distance(x, y); - } - - float norm_factor = 13.5; - float dist = distance(x, y); - float clamped_dist = fmax(-norm_factor, fmin(dist, norm_factor)); - float scaled_dist = clamped_dist / norm_factor; - uint8_t quantized_value = ((scaled_dist + 1) / 2) * UINT8_MAX; - pixels[y * width + x] = quantized_value; - } - } - -#undef image -#undef distance -#undef nearestpt -} - static void DrawGlyph(SkCanvas* canvas, const FontGlyphPair& font_glyph, const Rect& location, @@ -353,7 +235,6 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, SkImageInfo image_info; switch (atlas.GetType()) { - case GlyphAtlas::Type::kSignedDistanceField: case GlyphAtlas::Type::kAlphaBitmap: image_info = SkImageInfo::MakeA8(atlas_size.width, atlas_size.height); break; @@ -563,10 +444,6 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- PixelFormat format; switch (type) { - case GlyphAtlas::Type::kSignedDistanceField: - ConvertBitmapToSignedDistanceField( - reinterpret_cast(bitmap->getPixels()), atlas_size.width, - atlas_size.height); case GlyphAtlas::Type::kAlphaBitmap: format = PixelFormat::kA8UNormInt; break; diff --git a/engine/src/flutter/impeller/typographer/glyph_atlas.h b/engine/src/flutter/impeller/typographer/glyph_atlas.h index e7282a0cdb6..e8f2cbe4513 100644 --- a/engine/src/flutter/impeller/typographer/glyph_atlas.h +++ b/engine/src/flutter/impeller/typographer/glyph_atlas.h @@ -32,13 +32,6 @@ class GlyphAtlas { //---------------------------------------------------------------------------- /// @brief Describes how the glyphs are represented in the texture. enum class Type { - //-------------------------------------------------------------------------- - /// The glyphs are represented at a fixed size in an 8-bit grayscale texture - /// where the value of each pixel represents a signed-distance field that - /// stores the glyph outlines. - /// - kSignedDistanceField, - //-------------------------------------------------------------------------- /// The glyphs are reprsented at their requested size using only an 8-bit /// alpha channel.