From cff12489bf6d4762f8db6e0400eb03bffe618ae8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 19 Apr 2023 16:38:54 -0700 Subject: [PATCH 01/17] [impeller] convert src over to src for solid color --- .../entity/contents/solid_color_contents.cc | 8 ++++++++ .../entity/contents/solid_color_contents.h | 4 ++++ impeller/entity/entity_unittests.cc | 20 +++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/impeller/entity/contents/solid_color_contents.cc b/impeller/entity/contents/solid_color_contents.cc index 9c14169833330..80f7c4cb07bde 100644 --- a/impeller/entity/contents/solid_color_contents.cc +++ b/impeller/entity/contents/solid_color_contents.cc @@ -46,6 +46,11 @@ bool SolidColorContents::ShouldRender( return Contents::ShouldRender(entity, stencil_coverage); } +bool SolidColorContents::ConvertToSrc(const Entity& entity) const { + return entity.GetBlendMode() == BlendMode::kSourceOver && + GetColor().alpha >= 1.0; +} + bool SolidColorContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { @@ -60,6 +65,9 @@ bool SolidColorContents::Render(const ContentContext& renderer, GetGeometry()->GetPositionBuffer(renderer, entity, pass); auto options = OptionsFromPassAndEntity(pass, entity); + if (ConvertToSrc(entity)) { + options.blend_mode = BlendMode::kSource; + } if (geometry_result.prevent_overdraw) { options.stencil_compare = CompareFunction::kEqual; options.stencil_operation = StencilOperation::kIncrementClamp; diff --git a/impeller/entity/contents/solid_color_contents.h b/impeller/entity/contents/solid_color_contents.h index 5348b906afa15..9486fc8f0db2d 100644 --- a/impeller/entity/contents/solid_color_contents.h +++ b/impeller/entity/contents/solid_color_contents.h @@ -46,6 +46,10 @@ class SolidColorContents final : public ColorSourceContents { const Entity& entity, RenderPass& pass) const override; + /// @brief Convert SrcOver blend modes into Src blend modes if the color has + /// no opacity. + bool ConvertToSrc(const Entity& entity) const; + private: Color color_; diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index f1285c3393ef7..b34055ba5f61b 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2547,5 +2547,25 @@ TEST_P(EntityTest, CoverageForStrokePathWithNegativeValuesInTransform) { ASSERT_RECT_NEAR(coverage.value(), Rect::MakeXYWH(102.5, 342.5, 85, 155)); } +TEST_P(EntityTest, ConvertToSrcBlend) { + Entity entity; + entity.SetBlendMode(BlendMode::kSourceOver); + + auto contents = SolidColorContents::Make( + PathBuilder{}.AddRect(Rect::MakeSize(Size(100, 100))).TakePath(), + Color::Red()); + + ASSERT_TRUE(contents->ConvertToSrc(entity)); + + // Color with alpha, should return false. + contents->SetInheritedOpacity(0.5); + ASSERT_FALSE(contents->ConvertToSrc(entity)); + + // Non source over blend mode, should return false. + contents->SetInheritedOpacity(1.0); + entity.SetBlendMode(BlendMode::kDestination); + ASSERT_FALSE(contents->ConvertToSrc(entity)); +} + } // namespace testing } // namespace impeller From b5eb0c2b7dfd8987c010fc752d614c2641c79eaa Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 12:35:32 -0700 Subject: [PATCH 02/17] [Impeller] use device buffer to SkBitmap allocation --- .../renderer/backend/gles/context_gles.cc | 1 + .../renderer/backend/metal/context_mtl.mm | 1 + impeller/renderer/capabilities.cc | 16 +++ impeller/renderer/capabilities.h | 5 + .../backends/skia/text_render_context_skia.cc | 103 +++++++++++++----- .../backends/skia/text_render_context_skia.h | 22 ++++ impeller/typographer/glyph_atlas.cc | 10 +- impeller/typographer/glyph_atlas.h | 7 +- impeller/typographer/lazy_glyph_atlas.cc | 5 +- impeller/typographer/text_render_context.cc | 4 +- impeller/typographer/text_render_context.h | 2 + impeller/typographer/typographer_unittests.cc | 54 ++++----- 12 files changed, 166 insertions(+), 64 deletions(-) diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index f2bba7c15c524..8a60020cec1b0 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -74,6 +74,7 @@ ContextGLES::ContextGLES(std::unique_ptr gl, .SetSupportsCompute(false, false) .SetSupportsReadFromResolve(false) .SetSupportsReadFromOnscreenTexture(false) + .SetSupportsBufferToTextureBlits(true) .Build(); } diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 2bca0a145dbdd..626251a19d1fb 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -55,6 +55,7 @@ static bool DeviceSupportsComputeSubgroups(id device) { .SetSupportsBufferToTextureBlits(true) .SetSupportsTextureToTextureBlits(true) .SetSupportsDecalTileMode(true) + .SetSupportsSharedDeviceBufferTextureMemory(true) .SetSupportsFramebufferFetch(DeviceSupportsFramebufferFetch(device)) .SetDefaultColorFormat(color_format) .SetDefaultStencilFormat(PixelFormat::kS8UInt) diff --git a/impeller/renderer/capabilities.cc b/impeller/renderer/capabilities.cc index 5b23d4ca1d7a8..9e1a51e556d1f 100644 --- a/impeller/renderer/capabilities.cc +++ b/impeller/renderer/capabilities.cc @@ -66,6 +66,11 @@ class StandardCapabilities final : public Capabilities { return supports_decal_tile_mode_; } + // |Capabilities| + bool SupportsSharedDeviceBufferTextureMemory() const override { + return supports_shared_device_buffer_texture_memory_; + } + // |Capabilities| PixelFormat GetDefaultColorFormat() const override { return default_color_format_; @@ -88,6 +93,7 @@ class StandardCapabilities final : public Capabilities { bool supports_read_from_onscreen_texture, bool supports_read_from_resolve, bool supports_decal_tile_mode, + bool supports_shared_device_buffer_texture_memory, PixelFormat default_color_format, PixelFormat default_stencil_format) : has_threading_restrictions_(has_threading_restrictions), @@ -102,6 +108,8 @@ class StandardCapabilities final : public Capabilities { supports_read_from_onscreen_texture), supports_read_from_resolve_(supports_read_from_resolve), supports_decal_tile_mode_(supports_decal_tile_mode), + supports_shared_device_buffer_texture_memory_( + supports_shared_device_buffer_texture_memory), default_color_format_(default_color_format), default_stencil_format_(default_stencil_format) {} @@ -118,6 +126,7 @@ class StandardCapabilities final : public Capabilities { bool supports_read_from_onscreen_texture_ = false; bool supports_read_from_resolve_ = false; bool supports_decal_tile_mode_ = false; + bool supports_shared_device_buffer_texture_memory_ = false; PixelFormat default_color_format_ = PixelFormat::kUnknown; PixelFormat default_stencil_format_ = PixelFormat::kUnknown; @@ -198,6 +207,12 @@ CapabilitiesBuilder& CapabilitiesBuilder::SetSupportsDecalTileMode(bool value) { return *this; } +CapabilitiesBuilder& +CapabilitiesBuilder::SetSupportsSharedDeviceBufferTextureMemory(bool value) { + supports_shared_device_buffer_texture_memory_ = value; + return *this; +} + std::unique_ptr CapabilitiesBuilder::Build() { return std::unique_ptr(new StandardCapabilities( // has_threading_restrictions_, // @@ -211,6 +226,7 @@ std::unique_ptr CapabilitiesBuilder::Build() { supports_read_from_onscreen_texture_, // supports_read_from_resolve_, // supports_decal_tile_mode_, // + supports_shared_device_buffer_texture_memory_, // *default_color_format_, // *default_stencil_format_ // )); diff --git a/impeller/renderer/capabilities.h b/impeller/renderer/capabilities.h index 78c0c209a7895..1756b915b4485 100644 --- a/impeller/renderer/capabilities.h +++ b/impeller/renderer/capabilities.h @@ -37,6 +37,8 @@ class Capabilities { virtual bool SupportsDecalTileMode() const = 0; + virtual bool SupportsSharedDeviceBufferTextureMemory() const = 0; + virtual PixelFormat GetDefaultColorFormat() const = 0; virtual PixelFormat GetDefaultStencilFormat() const = 0; @@ -78,6 +80,8 @@ class CapabilitiesBuilder { CapabilitiesBuilder& SetSupportsDecalTileMode(bool value); + CapabilitiesBuilder& SetSupportsSharedDeviceBufferTextureMemory(bool value); + std::unique_ptr Build(); private: @@ -92,6 +96,7 @@ class CapabilitiesBuilder { bool supports_read_from_onscreen_texture_ = false; bool supports_read_from_resolve_ = false; bool supports_decal_tile_mode_ = false; + bool supports_shared_device_buffer_texture_memory_ = false; std::optional default_color_format_ = std::nullopt; std::optional default_stencil_format_ = std::nullopt; diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 51ec87b712d30..271be82835cf0 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -10,11 +10,13 @@ #include "flutter/fml/trace_event.h" #include "impeller/base/allocation.h" #include "impeller/core/allocator.h" +#include "impeller/core/device_buffer.h" #include "impeller/typographer/backends/skia/typeface_skia.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" #include "third_party/skia/include/core/SkFontMetrics.h" +#include "third_party/skia/include/core/SkPixelRef.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 @@ -346,9 +348,12 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, return true; } -static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, - const ISize& atlas_size) { +static std::pair, std::shared_ptr> +CreateAtlasBitmap(const GlyphAtlas& atlas, + std::shared_ptr allocator, + const ISize& atlas_size) { TRACE_EVENT0("impeller", __FUNCTION__); + auto font_allocator = FontImpellerAllocator(allocator); auto bitmap = std::make_shared(); SkImageInfo image_info; @@ -363,17 +368,18 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, break; } - if (!bitmap->tryAllocPixels(image_info)) { - return nullptr; + bitmap->setInfo(image_info); + if (!bitmap->tryAllocPixels(&font_allocator)) { + return std::make_pair(nullptr, nullptr); } auto surface = SkSurface::MakeRasterDirect(bitmap->pixmap()); if (!surface) { - return nullptr; + return std::make_pair(nullptr, nullptr); } auto canvas = surface->getCanvas(); if (!canvas) { - return nullptr; + return std::make_pair(nullptr, nullptr); } bool has_color = atlas.GetType() == GlyphAtlas::Type::kColorBitmap; @@ -384,13 +390,16 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, return true; }); - return bitmap; + auto device_buffer = font_allocator.GetDeviceBuffer(); + if (!device_buffer.has_value()) { + return std::make_pair(nullptr, nullptr); + } + return std::make_pair(bitmap, device_buffer.value()); } static bool UpdateGlyphTextureAtlas(std::shared_ptr bitmap, const std::shared_ptr& texture) { TRACE_EVENT0("impeller", __FUNCTION__); - FML_DCHECK(bitmap != nullptr); auto texture_descriptor = texture->GetTextureDescriptor(); @@ -404,14 +413,12 @@ static bool UpdateGlyphTextureAtlas(std::shared_ptr bitmap, } static std::shared_ptr UploadGlyphTextureAtlas( - const std::shared_ptr& allocator, + Allocator& allocator, + std::shared_ptr device_buffer, std::shared_ptr bitmap, const ISize& atlas_size, PixelFormat format) { TRACE_EVENT0("impeller", __FUNCTION__); - if (!allocator) { - return nullptr; - } FML_DCHECK(bitmap != nullptr); const auto& pixmap = bitmap->pixmap(); @@ -426,27 +433,19 @@ static std::shared_ptr UploadGlyphTextureAtlas( return nullptr; } - auto texture = allocator->CreateTexture(texture_descriptor); + auto texture = device_buffer->AsTexture(allocator, texture_descriptor, + pixmap.rowBytes()); if (!texture || !texture->IsValid()) { return nullptr; } texture->SetLabel("GlyphAtlas"); - - auto mapping = std::make_shared( - reinterpret_cast(bitmap->getAddr(0, 0)), // data - texture_descriptor.GetByteSizeOfBaseMipLevel(), // size - [bitmap](auto, auto) mutable { bitmap.reset(); } // proc - ); - - if (!texture->SetContents(mapping)) { - return nullptr; - } return texture; } std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, FrameIterator frame_iterator) const { TRACE_EVENT0("impeller", __FUNCTION__); if (!IsValid()) { @@ -502,15 +501,18 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 5: Draw new font-glyph pairs into the existing bitmap. // --------------------------------------------------------------------------- - auto bitmap = atlas_context->GetBitmap(); + auto [bitmap, device_buffer] = atlas_context->GetBitmap(); if (!UpdateAtlasBitmap(*last_atlas, bitmap, new_glyphs)) { return nullptr; } // --------------------------------------------------------------------------- // Step 6: Update the existing texture with the updated bitmap. + // This is only necessary on backends that don't support creating + // a texture that shares memory with the underlying device buffer. // --------------------------------------------------------------------------- - if (!UpdateGlyphTextureAtlas(bitmap, last_atlas->GetTexture())) { + if (!capabilities->SupportsSharedDeviceBufferTextureMemory() && + !UpdateGlyphTextureAtlas(bitmap, last_atlas->GetTexture())) { return nullptr; } return last_atlas; @@ -552,11 +554,12 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 7: Draw font-glyph pairs in the correct spot in the atlas. // --------------------------------------------------------------------------- - auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size); + auto [bitmap, device_buffer] = CreateAtlasBitmap( + *glyph_atlas, GetContext()->GetResourceAllocator(), atlas_size); if (!bitmap) { return nullptr; } - atlas_context->UpdateBitmap(bitmap); + atlas_context->UpdateBitmap(bitmap, device_buffer); // --------------------------------------------------------------------------- // Step 8: Upload the atlas as a texture. @@ -574,8 +577,9 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( format = PixelFormat::kR8G8B8A8UNormInt; break; } - auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(), - bitmap, atlas_size, format); + auto texture = + UploadGlyphTextureAtlas(*GetContext()->GetResourceAllocator().get(), + device_buffer, bitmap, atlas_size, format); if (!texture) { return nullptr; } @@ -588,4 +592,43 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( return glyph_atlas; } +FontImpellerAllocator::FontImpellerAllocator( + std::shared_ptr allocator) + : allocator_(std::move(allocator)) {} + +std::optional> +FontImpellerAllocator::GetDeviceBuffer() const { + return buffer_; +} + +bool FontImpellerAllocator::allocPixelRef(SkBitmap* bitmap) { + const SkImageInfo& info = bitmap->info(); + if (kUnknown_SkColorType == info.colorType() || info.width() < 0 || + info.height() < 0 || !info.validRowBytes(bitmap->rowBytes())) { + return false; + } + + DeviceBufferDescriptor descriptor; + descriptor.storage_mode = StorageMode::kHostVisible; + descriptor.size = ((bitmap->height() - 1) * bitmap->rowBytes()) + + (bitmap->width() * bitmap->bytesPerPixel()); + + auto device_buffer = allocator_->CreateBuffer(descriptor); + + struct ImpellerPixelRef final : public SkPixelRef { + ImpellerPixelRef(int w, int h, void* s, size_t r) + : SkPixelRef(w, h, s, r) {} + + ~ImpellerPixelRef() override {} + }; + + auto pixel_ref = sk_sp( + new ImpellerPixelRef(info.width(), info.height(), + device_buffer->OnGetContents(), bitmap->rowBytes())); + + bitmap->setPixelRef(std::move(pixel_ref), 0, 0); + buffer_ = std::move(device_buffer); + return true; +} + } // namespace impeller diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 0c97a8716e7ad..959d46244ce84 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -6,9 +6,30 @@ #include "flutter/fml/macros.h" #include "impeller/typographer/text_render_context.h" +#include "third_party/skia/include/core/SkBitmap.h" namespace impeller { +class DeviceBuffer; +class Allocator; + +class FontImpellerAllocator : public SkBitmap::Allocator { + public: + explicit FontImpellerAllocator(std::shared_ptr allocator); + + ~FontImpellerAllocator() = default; + + // |Allocator| + bool allocPixelRef(SkBitmap* bitmap) override; + + std::optional> GetDeviceBuffer() + const; + + private: + std::shared_ptr allocator_; + std::optional> buffer_; +}; + class TextRenderContextSkia : public TextRenderContext { public: TextRenderContextSkia(std::shared_ptr context); @@ -19,6 +40,7 @@ class TextRenderContextSkia : public TextRenderContext { std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, FrameIterator iterator) const override; private: diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc index 6908b9eaa2029..c5c0be86436d5 100644 --- a/impeller/typographer/glyph_atlas.cc +++ b/impeller/typographer/glyph_atlas.cc @@ -22,8 +22,9 @@ const ISize& GlyphAtlasContext::GetAtlasSize() const { return atlas_size_; } -std::shared_ptr GlyphAtlasContext::GetBitmap() const { - return bitmap_; +std::pair, std::shared_ptr> +GlyphAtlasContext::GetBitmap() const { + return std::make_pair(bitmap_, device_buffer_); } std::shared_ptr GlyphAtlasContext::GetRectPacker() const { @@ -36,8 +37,11 @@ void GlyphAtlasContext::UpdateGlyphAtlas(std::shared_ptr atlas, atlas_size_ = size; } -void GlyphAtlasContext::UpdateBitmap(std::shared_ptr bitmap) { +void GlyphAtlasContext::UpdateBitmap( + std::shared_ptr bitmap, + std::shared_ptr device_buffer) { bitmap_ = std::move(bitmap); + device_buffer_ = std::move(device_buffer); } void GlyphAtlasContext::UpdateRectPacker( diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index e7282a0cdb6a0..8e8243067a29d 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -11,6 +11,7 @@ #include "flutter/fml/macros.h" #include "impeller/core/texture.h" +#include "impeller/core/device_buffer.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/pipeline.h" #include "impeller/typographer/font_glyph_pair.h" @@ -153,7 +154,7 @@ class GlyphAtlasContext { //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) SkBitmap instance. - std::shared_ptr GetBitmap() const; + std::pair, std::shared_ptr> GetBitmap() const; //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) rect packer. @@ -163,7 +164,8 @@ class GlyphAtlasContext { /// @brief Update the context with a newly constructed glyph atlas. void UpdateGlyphAtlas(std::shared_ptr atlas, ISize size); - void UpdateBitmap(std::shared_ptr bitmap); + void UpdateBitmap(std::shared_ptr bitmap, + std::shared_ptr device_buffer); void UpdateRectPacker(std::shared_ptr rect_packer); @@ -171,6 +173,7 @@ class GlyphAtlasContext { std::shared_ptr atlas_; ISize atlas_size_; std::shared_ptr bitmap_; + std::shared_ptr device_buffer_; std::shared_ptr rect_packer_; FML_DISALLOW_COPY_AND_ASSIGN(GlyphAtlasContext); diff --git a/impeller/typographer/lazy_glyph_atlas.cc b/impeller/typographer/lazy_glyph_atlas.cc index 3e415716a528c..23969d787ccf3 100644 --- a/impeller/typographer/lazy_glyph_atlas.cc +++ b/impeller/typographer/lazy_glyph_atlas.cc @@ -37,6 +37,7 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( } } + auto capabilities = context->GetCapabilities(); auto text_context = TextRenderContext::Create(std::move(context)); if (!text_context || !text_context->IsValid()) { return nullptr; @@ -50,8 +51,8 @@ std::shared_ptr LazyGlyphAtlas::CreateOrGetGlyphAtlas( i++; return &result; }; - auto atlas = - text_context->CreateGlyphAtlas(type, std::move(atlas_context), iterator); + auto atlas = text_context->CreateGlyphAtlas(type, std::move(atlas_context), + capabilities, iterator); if (!atlas || !atlas->IsValid()) { VALIDATION_LOG << "Could not create valid atlas."; return nullptr; diff --git a/impeller/typographer/text_render_context.cc b/impeller/typographer/text_render_context.cc index 8cc8dbf00a02e..1fdc17878ac0c 100644 --- a/impeller/typographer/text_render_context.cc +++ b/impeller/typographer/text_render_context.cc @@ -29,6 +29,7 @@ const std::shared_ptr& TextRenderContext::GetContext() const { std::shared_ptr TextRenderContext::CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, const TextFrame& frame) const { size_t count = 0; FrameIterator iterator = [&]() -> const TextFrame* { @@ -38,7 +39,8 @@ std::shared_ptr TextRenderContext::CreateGlyphAtlas( } return nullptr; }; - return CreateGlyphAtlas(type, std::move(atlas_context), iterator); + return CreateGlyphAtlas(type, std::move(atlas_context), capabilities, + iterator); } } // namespace impeller diff --git a/impeller/typographer/text_render_context.h b/impeller/typographer/text_render_context.h index c63ac912a1fa8..ca1dc74d129e8 100644 --- a/impeller/typographer/text_render_context.h +++ b/impeller/typographer/text_render_context.h @@ -45,11 +45,13 @@ class TextRenderContext { virtual std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, FrameIterator iterator) const = 0; std::shared_ptr CreateGlyphAtlas( GlyphAtlas::Type type, std::shared_ptr atlas_context, + const std::shared_ptr& capabilities, const TextFrame& frame) const; protected: diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 445cc283fe6c7..af527f5c43bde 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/testing/testing.h" #include "impeller/playground/playground_test.h" +#include "impeller/renderer/capabilities.h" #include "impeller/typographer/backends/skia/text_frame_skia.h" #include "impeller/typographer/backends/skia/text_render_context_skia.h" #include "impeller/typographer/lazy_glyph_atlas.h" @@ -41,9 +42,9 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); @@ -116,9 +117,9 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("AGH", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -133,18 +134,18 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky skellingtons", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas()); // now attempt to re-create an atlas with the same text blob. - auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto next_atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } @@ -173,8 +174,9 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { } return nullptr; }; - auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, - atlas_context, iterator); + auto atlas = + context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), iterator); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -191,9 +193,9 @@ TEST_P(TypographerTest, DISABLED_GlyphAtlasTextureIsRecycledIfUnchanged) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -205,9 +207,9 @@ TEST_P(TypographerTest, DISABLED_GlyphAtlasTextureIsRecycledIfUnchanged) { // Now create a new glyph atlas with a nearly identical blob. auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); - auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob2)); + auto next_atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob2)); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -224,9 +226,9 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { SkFont sk_font; auto blob = SkTextBlob::MakeFromString("spooky 1", sk_font); ASSERT_TRUE(blob); - auto atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap, atlas_context, - TextFrameFromTextBlob(blob)); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob)); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -239,9 +241,9 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { // but change the type. auto blob2 = SkTextBlob::MakeFromString("spooky 1", sk_font); - auto next_atlas = - context->CreateGlyphAtlas(GlyphAtlas::Type::kColorBitmap, atlas_context, - TextFrameFromTextBlob(blob2)); + auto next_atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kColorBitmap, atlas_context, + CapabilitiesBuilder().Build(), TextFrameFromTextBlob(blob2)); ASSERT_NE(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); From 82ff8ad1a3403576c40544fabc8e663232e927d8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 12:36:02 -0700 Subject: [PATCH 03/17] ++ --- .../typographer/backends/skia/text_render_context_skia.h | 6 +++--- impeller/typographer/glyph_atlas.h | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 959d46244ce84..cde384907267f 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -15,15 +15,15 @@ class Allocator; class FontImpellerAllocator : public SkBitmap::Allocator { public: - explicit FontImpellerAllocator(std::shared_ptr allocator); + explicit FontImpellerAllocator( + std::shared_ptr allocator); ~FontImpellerAllocator() = default; // |Allocator| bool allocPixelRef(SkBitmap* bitmap) override; - std::optional> GetDeviceBuffer() - const; + std::optional> GetDeviceBuffer() const; private: std::shared_ptr allocator_; diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h index 8e8243067a29d..0e342fab1e970 100644 --- a/impeller/typographer/glyph_atlas.h +++ b/impeller/typographer/glyph_atlas.h @@ -10,8 +10,8 @@ #include #include "flutter/fml/macros.h" -#include "impeller/core/texture.h" #include "impeller/core/device_buffer.h" +#include "impeller/core/texture.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/pipeline.h" #include "impeller/typographer/font_glyph_pair.h" @@ -154,7 +154,8 @@ class GlyphAtlasContext { //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) SkBitmap instance. - std::pair, std::shared_ptr> GetBitmap() const; + std::pair, std::shared_ptr> + GetBitmap() const; //---------------------------------------------------------------------------- /// @brief Retrieve the previous (if any) rect packer. From 0511d657f6f07f58c5faf73f70f2931fbd2acdba Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 12:42:08 -0700 Subject: [PATCH 04/17] ++ --- impeller/renderer/backend/gles/context_gles.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index 8432f00f646a3..d64d607d47e8f 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -74,7 +74,6 @@ ContextGLES::ContextGLES(std::unique_ptr gl, .SetSupportsCompute(false, false) .SetSupportsReadFromResolve(false) .SetSupportsReadFromOnscreenTexture(false) - .SetSupportsBufferToTextureBlits(true) .Build(); } From 725ea87534e18e99cba7f9363d13209d314a625c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 12:53:31 -0700 Subject: [PATCH 05/17] ++ --- impeller/renderer/backend/vulkan/capabilities_vk.cc | 6 ++++++ impeller/renderer/backend/vulkan/capabilities_vk.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index ff4410f15935a..91bb80073e6e9 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -348,10 +348,16 @@ bool CapabilitiesVK::SupportsReadFromOnscreenTexture() const { return false; } +// |Capabilities| bool CapabilitiesVK::SupportsDecalTileMode() const { return true; } +// |Capabilities| +bool CapabilitiesVK::SupportsSharedDeviceBufferTextureMemory() const { + return false; +} + // |Capabilities| PixelFormat CapabilitiesVK::GetDefaultColorFormat() const { return color_format_; diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index 006ed6f0e0889..0628b23d38d58 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -79,6 +79,9 @@ class CapabilitiesVK final : public Capabilities, // |Capabilities| bool SupportsDecalTileMode() const override; + // |Capabilities| + bool SupportsSharedDeviceBufferTextureMemory() const override; + // |Capabilities| PixelFormat GetDefaultColorFormat() const override; From 4c5cf04caace35c51d9c448f2e3218a8201e11a0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 13:12:50 -0700 Subject: [PATCH 06/17] ++ --- impeller/typographer/backends/skia/text_render_context_skia.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 271be82835cf0..0d371e18ab3f4 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -434,7 +434,7 @@ static std::shared_ptr UploadGlyphTextureAtlas( } auto texture = device_buffer->AsTexture(allocator, texture_descriptor, - pixmap.rowBytes()); + texture_descriptor.GetBytesPerRow()); if (!texture || !texture->IsValid()) { return nullptr; } From 706386a1c10178e04278b4e08b7a58d54b67ac38 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 17:14:28 -0700 Subject: [PATCH 07/17] ++ --- .../typographer/backends/skia/text_render_context_skia.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 0d371e18ab3f4..c1d09a135025e 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -141,7 +141,9 @@ ISize OptimumAtlasSizeForFontGlyphPairs( const FontGlyphPair::Set& pairs, std::vector& glyph_positions, const std::shared_ptr& atlas_context) { - static constexpr auto kMinAtlasSize = 8u; + // This size needs to be above the minimum required aligment for linear + // textures. This is 256 for older intel macs and decreases on iOS devices. + static constexpr auto kMinAtlasSize = 256u; static constexpr auto kMaxAtlasSize = 4096u; TRACE_EVENT0("impeller", __FUNCTION__); @@ -428,6 +430,9 @@ static std::shared_ptr UploadGlyphTextureAtlas( texture_descriptor.format = format; texture_descriptor.size = atlas_size; + // If the alignment isn't a multiple of the pixel format, we cannot use + // a linear texture and instead must blit to a new texture. + FML_DCHECK(allocator.MinimumBytesPerRow(format) >= pixmap.rowBytes()); if (pixmap.rowBytes() * pixmap.height() != texture_descriptor.GetByteSizeOfBaseMipLevel()) { return nullptr; From 7e7d217ec4510445ffe711b1c4dfd87ff0d6b543 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 17:43:17 -0700 Subject: [PATCH 08/17] correct check --- impeller/typographer/backends/skia/text_render_context_skia.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index c1d09a135025e..0071b93a1e4ef 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -432,12 +432,12 @@ static std::shared_ptr UploadGlyphTextureAtlas( // If the alignment isn't a multiple of the pixel format, we cannot use // a linear texture and instead must blit to a new texture. - FML_DCHECK(allocator.MinimumBytesPerRow(format) >= pixmap.rowBytes()); if (pixmap.rowBytes() * pixmap.height() != texture_descriptor.GetByteSizeOfBaseMipLevel()) { return nullptr; } + FML_DCHECK(allocator.MinimumBytesPerRow(format) <= pixmap.rowBytes()); auto texture = device_buffer->AsTexture(allocator, texture_descriptor, texture_descriptor.GetBytesPerRow()); if (!texture || !texture->IsValid()) { From 81ad980d330d9e97db817a68add49723f4e68ae0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 20 Apr 2023 18:33:15 -0700 Subject: [PATCH 09/17] ++ --- .../typographer/backends/skia/text_render_context_skia.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index 0071b93a1e4ef..c1c0f0babe920 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -355,7 +355,7 @@ CreateAtlasBitmap(const GlyphAtlas& atlas, std::shared_ptr allocator, const ISize& atlas_size) { TRACE_EVENT0("impeller", __FUNCTION__); - auto font_allocator = FontImpellerAllocator(allocator); + auto font_allocator = FontImpellerAllocator(std::move(allocator)); auto bitmap = std::make_shared(); SkImageInfo image_info; @@ -416,8 +416,8 @@ static bool UpdateGlyphTextureAtlas(std::shared_ptr bitmap, static std::shared_ptr UploadGlyphTextureAtlas( Allocator& allocator, - std::shared_ptr device_buffer, - std::shared_ptr bitmap, + const std::shared_ptr& device_buffer, + const std::shared_ptr& bitmap, const ISize& atlas_size, PixelFormat format) { TRACE_EVENT0("impeller", __FUNCTION__); From 69ec6b09b6e8da042b87555b6b877970993f2785 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 21 Apr 2023 09:54:14 -0700 Subject: [PATCH 10/17] ++ --- impeller/renderer/capabilities_unittests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/renderer/capabilities_unittests.cc b/impeller/renderer/capabilities_unittests.cc index 1dc9d17e3c678..e3027b475508c 100644 --- a/impeller/renderer/capabilities_unittests.cc +++ b/impeller/renderer/capabilities_unittests.cc @@ -29,6 +29,7 @@ CAPABILITY_TEST(SupportsComputeSubgroups, false); CAPABILITY_TEST(SupportsReadFromOnscreenTexture, false); CAPABILITY_TEST(SupportsReadFromResolve, false); CAPABILITY_TEST(SupportsDecalTileMode, false); +CAPABILITY_TEST(SetSupportsSharedDeviceBufferTextureMemory, false); } // namespace testing } // namespace impeller From dfb3f3c028bf186a10671fa10e21169300fbe068 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 21 Apr 2023 09:59:29 -0700 Subject: [PATCH 11/17] ++ --- impeller/renderer/capabilities_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/capabilities_unittests.cc b/impeller/renderer/capabilities_unittests.cc index e3027b475508c..342dcba2b8a99 100644 --- a/impeller/renderer/capabilities_unittests.cc +++ b/impeller/renderer/capabilities_unittests.cc @@ -29,7 +29,7 @@ CAPABILITY_TEST(SupportsComputeSubgroups, false); CAPABILITY_TEST(SupportsReadFromOnscreenTexture, false); CAPABILITY_TEST(SupportsReadFromResolve, false); CAPABILITY_TEST(SupportsDecalTileMode, false); -CAPABILITY_TEST(SetSupportsSharedDeviceBufferTextureMemory, false); +CAPABILITY_TEST(SupportsSharedDeviceBufferTextureMemory, false); } // namespace testing } // namespace impeller From f832c977e1a5de2a8a8d02389777981a27eebd39 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 21 Apr 2023 12:27:59 -0700 Subject: [PATCH 12/17] ++ --- impeller/typographer/typographer_unittests.cc | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index af527f5c43bde..1c477a9728a1c 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -184,9 +184,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { atlas->GetTexture()->GetSize().height); } -// TODO(jonahwilliams): Re-enable -// https://github.com/flutter/flutter/issues/122839 -TEST_P(TypographerTest, DISABLED_GlyphAtlasTextureIsRecycledIfUnchanged) { +TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto context = TextRenderContext::Create(GetContext()); auto atlas_context = std::make_shared(); ASSERT_TRUE(context && context->IsValid()); @@ -253,6 +251,30 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { ASSERT_NE(old_packer, new_packer); } +TEST_P(TypographerTest, GlyphAtlasUsesLinearTexture) { + if (!GetContext() + ->GetCapabilities() + ->SupportsSharedDeviceBufferTextureMemory()) { + GTEST_SKIP() + << "Skipping test that requires " + "SupportsSharedDeviceBufferTextureMemory on non metal platform"; + } + + auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); + ASSERT_TRUE(context && context->IsValid()); + SkFont sk_font; + auto blob = SkTextBlob::MakeFromString("s", sk_font); + ASSERT_TRUE(blob); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kAlphaBitmap, atlas_context, + GetContext()->GetCapabilities(), TextFrameFromTextBlob(blob)); + + auto* first_texture = atlas->GetTexture().get(); + + ASSERT_TRUE(first_texture->IsValid()); +} + TEST_P(TypographerTest, FontGlyphPairTypeChangesHashAndEquals) { Font font = Font(nullptr, {}); FontGlyphPair pair_1 = { From ffe0d884001bf4966349577d90985854a63a6485 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 24 Apr 2023 11:44:45 -0700 Subject: [PATCH 13/17] add doc comment --- .../typographer/backends/skia/text_render_context_skia.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index cde384907267f..950bf35b556da 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -13,6 +13,13 @@ namespace impeller { class DeviceBuffer; class Allocator; +/// @brief An implementation of an SkBitmap allocator that deferrs allocation to +/// an impeller allocator. This allows usage of Skia software rendering +/// to write to a host buffer or linear texture without an extra copy. +/// +/// This class is an exact copy of the implementation in +/// image_decode_impeller.cc due to the lack of a reasonable library +/// that could be shared. class FontImpellerAllocator : public SkBitmap::Allocator { public: explicit FontImpellerAllocator( From 6b3d886da2934180dd17cc62a2eb1809815a71e3 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 26 Apr 2023 10:17:26 -0700 Subject: [PATCH 14/17] Update impeller/typographer/backends/skia/text_render_context_skia.h Co-authored-by: Dan Field --- impeller/typographer/backends/skia/text_render_context_skia.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.h b/impeller/typographer/backends/skia/text_render_context_skia.h index 950bf35b556da..49200c8b4c12c 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.h +++ b/impeller/typographer/backends/skia/text_render_context_skia.h @@ -14,7 +14,7 @@ class DeviceBuffer; class Allocator; /// @brief An implementation of an SkBitmap allocator that deferrs allocation to -/// an impeller allocator. This allows usage of Skia software rendering +/// an Impeller allocator. This allows usage of Skia software rendering /// to write to a host buffer or linear texture without an extra copy. /// /// This class is an exact copy of the implementation in From 78fddf79bdaee986a7d50bde83616a8b7653a4f1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 26 Apr 2023 10:59:50 -0700 Subject: [PATCH 15/17] ++ --- .../backends/skia/text_render_context_skia.cc | 50 +++++++++++++------ impeller/typographer/typographer_unittests.cc | 26 +++++++++- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc index c1c0f0babe920..2a53d8869bfd3 100644 --- a/impeller/typographer/backends/skia/text_render_context_skia.cc +++ b/impeller/typographer/backends/skia/text_render_context_skia.cc @@ -38,6 +38,16 @@ std::unique_ptr TextRenderContext::Create( // https://github.com/flutter/flutter/issues/114563 constexpr auto kPadding = 2; +std::optional ComputeMinimumAlignment( + const std::shared_ptr& allocator, + const std::shared_ptr& capabilities, + PixelFormat format) { + if (!capabilities->SupportsSharedDeviceBufferTextureMemory()) { + return std::nullopt; + } + return allocator->MinimumBytesPerRow(format); +} + TextRenderContextSkia::TextRenderContextSkia(std::shared_ptr context) : TextRenderContext(std::move(context)) {} @@ -140,15 +150,21 @@ namespace { ISize OptimumAtlasSizeForFontGlyphPairs( const FontGlyphPair::Set& pairs, std::vector& glyph_positions, - const std::shared_ptr& atlas_context) { + const std::shared_ptr& atlas_context, + std::optional minimum_alignment) { // This size needs to be above the minimum required aligment for linear // textures. This is 256 for older intel macs and decreases on iOS devices. static constexpr auto kMinAtlasSize = 256u; static constexpr auto kMaxAtlasSize = 4096u; + // In case a device happens to have a larger minimum alignment, verify that + // 256 is sufficient here. + uint16_t minimum_size = minimum_alignment.value_or(0) > kMinAtlasSize + ? minimum_alignment.value() + : kMinAtlasSize; TRACE_EVENT0("impeller", __FUNCTION__); - ISize current_size(kMinAtlasSize, kMinAtlasSize); + ISize current_size(minimum_size, minimum_size); size_t total_pairs = pairs.size() + 1; do { auto rect_packer = std::shared_ptr( @@ -523,13 +539,25 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( return last_atlas; } // A new glyph atlas must be created. + PixelFormat format; + switch (type) { + case GlyphAtlas::Type::kSignedDistanceField: + case GlyphAtlas::Type::kAlphaBitmap: + format = PixelFormat::kA8UNormInt; + break; + case GlyphAtlas::Type::kColorBitmap: + format = PixelFormat::kR8G8B8A8UNormInt; + break; + } // --------------------------------------------------------------------------- // Step 4: Get the optimum size of the texture atlas. // --------------------------------------------------------------------------- auto glyph_atlas = std::make_shared(type); + auto min_alignment = ComputeMinimumAlignment( + GetContext()->GetResourceAllocator(), capabilities, format); auto atlas_size = OptimumAtlasSizeForFontGlyphPairs( - font_glyph_pairs, glyph_positions, atlas_context); + font_glyph_pairs, glyph_positions, atlas_context, min_alignment); atlas_context->UpdateGlyphAtlas(glyph_atlas, atlas_size); if (atlas_size.IsEmpty()) { @@ -569,18 +597,10 @@ std::shared_ptr TextRenderContextSkia::CreateGlyphAtlas( // --------------------------------------------------------------------------- // Step 8: Upload the atlas as a texture. // --------------------------------------------------------------------------- - 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; - case GlyphAtlas::Type::kColorBitmap: - format = PixelFormat::kR8G8B8A8UNormInt; - break; + if (type == GlyphAtlas::Type::kSignedDistanceField) { + ConvertBitmapToSignedDistanceField( + reinterpret_cast(bitmap->getPixels()), atlas_size.width, + atlas_size.height); } auto texture = UploadGlyphTextureAtlas(*GetContext()->GetResourceAllocator().get(), diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 1c477a9728a1c..5223553136b0d 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -251,7 +251,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { ASSERT_NE(old_packer, new_packer); } -TEST_P(TypographerTest, GlyphAtlasUsesLinearTexture) { +TEST_P(TypographerTest, GlyphAtlasUsesLinearTextureAlphaBitmap) { if (!GetContext() ->GetCapabilities() ->SupportsSharedDeviceBufferTextureMemory()) { @@ -275,6 +275,30 @@ TEST_P(TypographerTest, GlyphAtlasUsesLinearTexture) { ASSERT_TRUE(first_texture->IsValid()); } +TEST_P(TypographerTest, GlyphAtlasUsesLinearTextureColor) { + if (!GetContext() + ->GetCapabilities() + ->SupportsSharedDeviceBufferTextureMemory()) { + GTEST_SKIP() + << "Skipping test that requires " + "SupportsSharedDeviceBufferTextureMemory on non metal platform"; + } + + auto context = TextRenderContext::Create(GetContext()); + auto atlas_context = std::make_shared(); + ASSERT_TRUE(context && context->IsValid()); + SkFont sk_font; + auto blob = SkTextBlob::MakeFromString("s", sk_font); + ASSERT_TRUE(blob); + auto atlas = context->CreateGlyphAtlas( + GlyphAtlas::Type::kColorBitmap, atlas_context, + GetContext()->GetCapabilities(), TextFrameFromTextBlob(blob)); + + auto* first_texture = atlas->GetTexture().get(); + + ASSERT_TRUE(first_texture->IsValid()); +} + TEST_P(TypographerTest, FontGlyphPairTypeChangesHashAndEquals) { Font font = Font(nullptr, {}); FontGlyphPair pair_1 = { From 7ebfbe15216c1bac23c95c8f02970b7a14bb31fd Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 26 Apr 2023 16:54:38 -0700 Subject: [PATCH 16/17] disable use of linear textures on simulators --- impeller/renderer/backend/metal/context_mtl.mm | 10 +++++++++- impeller/renderer/backend/metal/device_buffer_mtl.h | 2 ++ impeller/renderer/backend/metal/device_buffer_mtl.mm | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 9609dc4bd3692..969742b478eb9 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -45,6 +45,14 @@ static bool DeviceSupportsComputeSubgroups(id device) { return supports_subgroups; } +static constexpr bool SupportsLinearTexture() { +#ifndef FML_OS_IOS_SIMULATOR + return true; +#else + return false; +#endif // FML_OS_IOS_SIMULATOR +} + static std::unique_ptr InferMetalCapabilities( id device, PixelFormat color_format) { @@ -55,7 +63,7 @@ static bool DeviceSupportsComputeSubgroups(id device) { .SetSupportsBufferToTextureBlits(true) .SetSupportsTextureToTextureBlits(true) .SetSupportsDecalTileMode(true) - .SetSupportsSharedDeviceBufferTextureMemory(true) + .SetSupportsSharedDeviceBufferTextureMemory(SupportsLinearTexture()) .SetSupportsFramebufferFetch(DeviceSupportsFramebufferFetch(device)) .SetDefaultColorFormat(color_format) .SetDefaultStencilFormat(PixelFormat::kS8UInt) diff --git a/impeller/renderer/backend/metal/device_buffer_mtl.h b/impeller/renderer/backend/metal/device_buffer_mtl.h index 061ad6004cc5a..06869eb661d7c 100644 --- a/impeller/renderer/backend/metal/device_buffer_mtl.h +++ b/impeller/renderer/backend/metal/device_buffer_mtl.h @@ -35,10 +35,12 @@ class DeviceBufferMTL final : public DeviceBuffer, // |DeviceBuffer| uint8_t* OnGetContents() const override; +#ifndef FML_OS_IOS_SIMULATOR // |DeviceBuffer| std::shared_ptr AsTexture(Allocator& allocator, const TextureDescriptor& descriptor, uint16_t row_bytes) const override; +#endif // FML_OS_IOS_SIMULATOR // |DeviceBuffer| bool OnCopyHostBuffer(const uint8_t* source, diff --git a/impeller/renderer/backend/metal/device_buffer_mtl.mm b/impeller/renderer/backend/metal/device_buffer_mtl.mm index a8534c9a5da2a..5a08c6d71072e 100644 --- a/impeller/renderer/backend/metal/device_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/device_buffer_mtl.mm @@ -29,6 +29,7 @@ return reinterpret_cast(buffer_.contents); } +#ifndef FML_OS_IOS_SIMULATOR std::shared_ptr DeviceBufferMTL::AsTexture( Allocator& allocator, const TextureDescriptor& descriptor, @@ -52,6 +53,7 @@ } return std::make_shared(descriptor, texture); } +#endif // FML_OS_IOS_SIMULATOR [[nodiscard]] bool DeviceBufferMTL::OnCopyHostBuffer(const uint8_t* source, Range source_range, From b17f242c9870d86883317889e0f41e2d6f25d5a0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 26 Apr 2023 16:55:21 -0700 Subject: [PATCH 17/17] ++ --- impeller/renderer/backend/metal/context_mtl.mm | 6 +++--- impeller/renderer/backend/metal/device_buffer_mtl.mm | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 969742b478eb9..576753f79f870 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -46,10 +46,10 @@ static bool DeviceSupportsComputeSubgroups(id device) { } static constexpr bool SupportsLinearTexture() { -#ifndef FML_OS_IOS_SIMULATOR - return true; -#else +#ifdef FML_OS_IOS_SIMULATOR return false; +#else + return true; #endif // FML_OS_IOS_SIMULATOR } diff --git a/impeller/renderer/backend/metal/device_buffer_mtl.mm b/impeller/renderer/backend/metal/device_buffer_mtl.mm index 5a08c6d71072e..3418df47f48de 100644 --- a/impeller/renderer/backend/metal/device_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/device_buffer_mtl.mm @@ -53,7 +53,7 @@ } return std::make_shared(descriptor, texture); } -#endif // FML_OS_IOS_SIMULATOR +#endif // FML_OS_IOS_SIMULATOR [[nodiscard]] bool DeviceBufferMTL::OnCopyHostBuffer(const uint8_t* source, Range source_range,