diff --git a/impeller/renderer/backend/gles/blit_command_gles.cc b/impeller/renderer/backend/gles/blit_command_gles.cc index c7a5de96fa3ed..b3b6d84423a2b 100644 --- a/impeller/renderer/backend/gles/blit_command_gles.cc +++ b/impeller/renderer/backend/gles/blit_command_gles.cc @@ -276,7 +276,7 @@ bool BlitCopyBufferToTextureCommandGLES::Encode( 0u, // border data.external_format, // external format data.type, // type - tex_data // data + nullptr // data ); texture_gles.MarkSliceInitialized(slice); } diff --git a/impeller/renderer/blit_pass_unittests.cc b/impeller/renderer/blit_pass_unittests.cc index 732af99108a1a..e30d89fb049b4 100644 --- a/impeller/renderer/blit_pass_unittests.cc +++ b/impeller/renderer/blit_pass_unittests.cc @@ -104,5 +104,29 @@ TEST_P(BlitPassTest, ChecksInvalidSliceParameters) { std::nullopt, "", /*slice=*/0)); } +TEST_P(BlitPassTest, CanBlitSmallRegionToUninitializedTexture) { + auto context = GetContext(); + auto cmd_buffer = context->CreateCommandBuffer(); + auto blit_pass = cmd_buffer->CreateBlitPass(); + + TextureDescriptor dst_format; + dst_format.storage_mode = StorageMode::kDevicePrivate; + dst_format.format = PixelFormat::kR8G8B8A8UNormInt; + dst_format.size = {1000, 1000}; + auto dst = context->GetResourceAllocator()->CreateTexture(dst_format); + + DeviceBufferDescriptor src_format; + src_format.size = 4; + src_format.storage_mode = StorageMode::kHostVisible; + auto src = context->GetResourceAllocator()->CreateBuffer(src_format); + + ASSERT_TRUE(dst); + + EXPECT_TRUE(blit_pass->AddCopy(DeviceBuffer::AsBufferView(src), dst, + IRect::MakeLTRB(0, 0, 1, 1), "", /*slice=*/0)); + EXPECT_TRUE(blit_pass->EncodeCommands(GetContext()->GetResourceAllocator())); + EXPECT_TRUE(context->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok()); +} + } // namespace testing } // namespace impeller diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc index 25f36c73bc05c..8054bc2284ab3 100644 --- a/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -142,10 +142,12 @@ static size_t PairsFitInAtlasOfSize( )) { return pairs.size() - i; } - glyph_positions.emplace_back(Rect::MakeXYWH(location_in_atlas.x(), // - location_in_atlas.y(), // - glyph_size.width, // - glyph_size.height // + // The size of the glyph was padded by 1px on all sides. Shift the glyph + // origin by 1px so the padding surrounds the glyph. + glyph_positions.emplace_back(Rect::MakeXYWH(location_in_atlas.x() + 1, // + location_in_atlas.y() + 1, // + glyph_size.width, // + glyph_size.height // )); } @@ -180,10 +182,12 @@ static bool CanAppendToExistingAtlas( )) { return false; } - glyph_positions.emplace_back(Rect::MakeXYWH(location_in_atlas.x(), // - location_in_atlas.y(), // - glyph_size.width, // - glyph_size.height // + // The size of the glyph was padded by 1px on all sides. Shift the glyph + // origin by 1px so the padding surrounds the glyph. + glyph_positions.emplace_back(Rect::MakeXYWH(location_in_atlas.x() + 1, // + location_in_atlas.y() + 1, // + glyph_size.width, // + glyph_size.height // )); } @@ -234,7 +238,8 @@ static void DrawGlyph(SkCanvas* canvas, const Glyph& glyph, bool has_color) { const auto& metrics = scaled_font.font.GetMetrics(); - const auto position = SkPoint::Make(0, 0); + const auto position = + SkPoint::Make(1.0f / scaled_font.scale, 1.0f / scaled_font.scale); SkGlyphID glyph_id = glyph.index; SkFont sk_font( @@ -280,6 +285,12 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, continue; } + FML_DCHECK(pos->GetOrigin() != Point(0, 0)); + // Add 1px of padding around glyph so that linearly sampled skew/rotate + // glyphs do not sample uninitialized data. + size.width += 2; + size.height += 2; + SkBitmap bitmap; HostBufferAllocator allocator(host_buffer); bitmap.setInfo(GetImageInfo(atlas, size)); @@ -297,9 +308,11 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas, DrawGlyph(canvas, pair.scaled_font, pair.glyph, has_color); - if (!blit_pass->AddCopy(allocator.TakeBufferView(), texture, - IRect::MakeXYWH(pos->GetLeft(), pos->GetTop(), - size.width, size.height))) { + // Note: the position is shifted by (1, 1) to include the padding pixels. + if (!blit_pass->AddCopy( + allocator.TakeBufferView(), texture, + IRect::MakeXYWH(pos->GetLeft() - 1, pos->GetTop() - 1, size.width, + size.height))) { return false; } } @@ -414,6 +427,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( if (atlas_size.IsEmpty()) { return nullptr; } + // --------------------------------------------------------------------------- // Step 4b: Find location of font-glyph pairs in the atlas. We have this from // the last step. So no need to do create another rect packer. But just do a @@ -461,6 +475,7 @@ std::shared_ptr TypographerContextSkia::CreateGlyphAtlas( if (!new_texture) { return nullptr; } + // The texture needs to be cleared to transparent black so that linearly // samplex rotated/skewed glyphs do not grab uninitialized data. We could // instead use a render pass to clear to transparent black, but there are