Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 20ea633

Browse files
jonahwilliamsjonahwilliams
authored
[Impeller] Update BlitPass::AddCopy to use destination_region instead of origin for buffer to texture copies. (#52555)
Based on #52510 Work towards flutter/flutter#138798 Change IPoint destination_origin to IRect destination_region, which allows us to specify an area smaller than the entire texture to replace. This will eventually allow us to only upload individual glyphs. This fixes the cubemap issue I previously hit: each face needs to track initialization separately.
1 parent 4bac4a6 commit 20ea633

19 files changed

+165
-48
lines changed

‎impeller/aiks/aiks_unittests.cc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "impeller/aiks/image_filter.h"
2121
#include "impeller/aiks/paint_pass_delegate.h"
2222
#include "impeller/aiks/testing/context_spy.h"
23+
#include "impeller/core/device_buffer.h"
2324
#include "impeller/entity/contents/solid_color_contents.h"
2425
#include "impeller/geometry/color.h"
2526
#include "impeller/geometry/constants.h"
@@ -3151,6 +3152,39 @@ TEST_P(AiksTest, CanRenderTextWithLargePerspectiveTransform) {
31513152
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
31523153
}
31533154

3155+
TEST_P(AiksTest, SetContentsWithRegion) {
3156+
auto bridge = CreateTextureForFixture("bay_bridge.jpg");
3157+
3158+
// Replace part of the texture with a red rectangle.
3159+
std::vector<uint8_t> bytes(100 * 100 * 4);
3160+
for (auto i = 0u; i < bytes.size(); i += 4) {
3161+
bytes[i] = 255;
3162+
bytes[i + 1] = 0;
3163+
bytes[i + 2] = 0;
3164+
bytes[i + 3] = 255;
3165+
}
3166+
auto mapping =
3167+
std::make_shared<fml::NonOwnedMapping>(bytes.data(), bytes.size());
3168+
auto device_buffer =
3169+
GetContext()->GetResourceAllocator()->CreateBufferWithCopy(*mapping);
3170+
auto cmd_buffer = GetContext()->CreateCommandBuffer();
3171+
auto blit_pass = cmd_buffer->CreateBlitPass();
3172+
blit_pass->AddCopy(DeviceBuffer::AsBufferView(device_buffer), bridge,
3173+
IRect::MakeLTRB(50, 50, 150, 150));
3174+
3175+
auto did_submit =
3176+
blit_pass->EncodeCommands(GetContext()->GetResourceAllocator()) &&
3177+
GetContext()->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok();
3178+
ASSERT_TRUE(did_submit);
3179+
3180+
auto image = std::make_shared<Image>(bridge);
3181+
3182+
Canvas canvas;
3183+
canvas.DrawImage(image, {0, 0}, {});
3184+
3185+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
3186+
}
3187+
31543188
} // namespace testing
31553189
} // namespace impeller
31563190

‎impeller/renderer/backend/gles/blit_command_gles.cc

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,9 @@ bool BlitCopyBufferToTextureCommandGLES::Encode(
219219
}
220220

221221
if (!tex_descriptor.IsValid() ||
222-
source.range.length < tex_descriptor.GetByteSizeOfBaseMipLevel()) {
222+
source.range.length !=
223+
BytesPerPixelForPixelFormat(tex_descriptor.format) *
224+
destination_region.Area()) {
223225
return false;
224226
}
225227

@@ -263,9 +265,9 @@ bool BlitCopyBufferToTextureCommandGLES::Encode(
263265
const GLvoid* tex_data =
264266
data.buffer_view.buffer->OnGetContents() + data.buffer_view.range.offset;
265267

266-
{
267-
TRACE_EVENT1("impeller", "TexImage2DUpload", "Bytes",
268-
std::to_string(data.buffer_view.range.length).c_str());
268+
// GL_INVALID_OPERATION if the texture array has not been
269+
// defined by a previous glTexImage2D operation.
270+
if (!texture_gles.IsSliceInitialized(slice)) {
269271
gl.TexImage2D(texture_target, // target
270272
0u, // LOD level
271273
data.internal_format, // internal format
@@ -276,8 +278,24 @@ bool BlitCopyBufferToTextureCommandGLES::Encode(
276278
data.type, // type
277279
tex_data // data
278280
);
281+
texture_gles.MarkSliceInitialized(slice);
282+
}
283+
284+
{
285+
TRACE_EVENT1("impeller", "TexImage2DUpload", "Bytes",
286+
std::to_string(data.buffer_view.range.length).c_str());
287+
gl.TexSubImage2D(texture_target, // target
288+
0u, // LOD level
289+
destination_region.GetX(), // xoffset
290+
destination_region.GetY(), // yoffset
291+
destination_region.GetWidth(), // width
292+
destination_region.GetHeight(), // height
293+
data.external_format, // external format
294+
data.type, // type
295+
tex_data // data
296+
297+
);
279298
}
280-
texture_gles.MarkContentsInitialized();
281299
return true;
282300
}
283301

‎impeller/renderer/backend/gles/blit_pass_gles.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,14 @@ bool BlitPassGLES::OnCopyTextureToBufferCommand(
130130
bool BlitPassGLES::OnCopyBufferToTextureCommand(
131131
BufferView source,
132132
std::shared_ptr<Texture> destination,
133-
IPoint destination_origin,
133+
IRect destination_region,
134134
std::string label,
135135
uint32_t slice) {
136136
auto command = std::make_unique<BlitCopyBufferToTextureCommandGLES>();
137137
command->label = label;
138138
command->source = std::move(source);
139139
command->destination = std::move(destination);
140-
command->destination_origin = destination_origin;
140+
command->destination_region = destination_region;
141141
command->label = label;
142142
command->slice = slice;
143143

‎impeller/renderer/backend/gles/blit_pass_gles.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class BlitPassGLES final : public BlitPass,
5757
// |BlitPass|
5858
bool OnCopyBufferToTextureCommand(BufferView source,
5959
std::shared_ptr<Texture> destination,
60-
IPoint destination_origin,
60+
IRect destination_region,
6161
std::string label,
6262
uint32_t slice) override;
6363

‎impeller/renderer/backend/gles/proc_table_gles.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ struct GLProc {
170170
PROC(StencilMaskSeparate); \
171171
PROC(StencilOpSeparate); \
172172
PROC(TexImage2D); \
173+
PROC(TexSubImage2D); \
173174
PROC(TexParameteri); \
174175
PROC(TexParameterfv); \
175176
PROC(Uniform1fv); \

‎impeller/renderer/backend/gles/texture_gles.cc

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,8 @@ bool TextureGLES::OnSetContents(std::shared_ptr<const fml::Mapping> mapping,
304304
}
305305
};
306306

307-
contents_initialized_ = reactor_->AddOperation(texture_upload);
308-
return contents_initialized_;
307+
slices_initialized_ = reactor_->AddOperation(texture_upload);
308+
return slices_initialized_[0];
309309
}
310310

311311
// |Texture|
@@ -343,13 +343,10 @@ static std::optional<GLenum> ToRenderBufferFormat(PixelFormat format) {
343343
}
344344

345345
void TextureGLES::InitializeContentsIfNecessary() const {
346-
if (!IsValid()) {
347-
return;
348-
}
349-
if (contents_initialized_) {
346+
if (!IsValid() || slices_initialized_[0]) {
350347
return;
351348
}
352-
contents_initialized_ = true;
349+
slices_initialized_[0] = true;
353350

354351
if (is_wrapped_) {
355352
return;
@@ -455,8 +452,12 @@ bool TextureGLES::Bind() const {
455452
return true;
456453
}
457454

458-
void TextureGLES::MarkContentsInitialized() const {
459-
contents_initialized_ = true;
455+
void TextureGLES::MarkSliceInitialized(size_t slice) const {
456+
slices_initialized_[slice] = true;
457+
}
458+
459+
bool TextureGLES::IsSliceInitialized(size_t slice) const {
460+
return slices_initialized_[slice];
460461
}
461462

462463
bool TextureGLES::GenerateMipmap() {

‎impeller/renderer/backend/gles/texture_gles.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_TEXTURE_GLES_H_
66
#define FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_TEXTURE_GLES_H_
77

8+
#include <bitset>
9+
810
#include "impeller/base/backend_cast.h"
911
#include "impeller/core/texture.h"
1012
#include "impeller/renderer/backend/gles/handle_gles.h"
@@ -60,13 +62,18 @@ class TextureGLES final : public Texture,
6062

6163
std::optional<GLuint> GetFBO() const { return wrapped_fbo_; }
6264

63-
void MarkContentsInitialized() const;
65+
// For non cubemap textures, 0 indicates uninitialized and 1 indicates
66+
// initialized. For cubemap textures, each face is initialized separately with
67+
// each bit tracking the initialization of the corresponding slice.
68+
void MarkSliceInitialized(size_t slice) const;
69+
70+
bool IsSliceInitialized(size_t slice) const;
6471

6572
private:
6673
ReactorGLES::Ref reactor_;
6774
const Type type_;
6875
HandleGLES handle_;
69-
mutable bool contents_initialized_ = false;
76+
mutable std::bitset<6> slices_initialized_ = 0;
7077
const bool is_wrapped_;
7178
const std::optional<GLuint> wrapped_fbo_;
7279
bool is_valid_ = false;

‎impeller/renderer/backend/metal/blit_command_mtl.mm

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,22 +114,21 @@
114114
}
115115

116116
auto destination_origin_mtl =
117-
MTLOriginMake(destination_origin.x, destination_origin.y, 0);
118-
119-
auto image_size = destination->GetTextureDescriptor().size;
120-
auto source_size_mtl = MTLSizeMake(image_size.width, image_size.height, 1);
117+
MTLOriginMake(destination_region.GetX(), destination_region.GetY(), 0);
118+
auto source_size_mtl = MTLSizeMake(destination_region.GetWidth(),
119+
destination_region.GetHeight(), 1);
121120

122121
auto destination_bytes_per_pixel =
123122
BytesPerPixelForPixelFormat(destination->GetTextureDescriptor().format);
124123
auto destination_bytes_per_row =
125-
source_size_mtl.width * destination_bytes_per_pixel;
126-
auto destination_bytes_per_image =
127-
source_size_mtl.height * destination_bytes_per_row;
124+
destination_region.GetWidth() * destination_bytes_per_pixel;
128125

129126
[encoder copyFromBuffer:source_mtl
130127
sourceOffset:source.range.offset
131128
sourceBytesPerRow:destination_bytes_per_row
132-
sourceBytesPerImage:destination_bytes_per_image
129+
sourceBytesPerImage:
130+
0 // 0 for 2D textures according to
131+
// https://developer.apple.com/documentation/metal/mtlblitcommandencoder/1400752-copyfrombuffer
133132
sourceSize:source_size_mtl
134133
toTexture:destination_mtl
135134
destinationSlice:slice

‎impeller/renderer/backend/metal/blit_pass_mtl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class BlitPassMTL final : public BlitPass {
5555
// |BlitPass|
5656
bool OnCopyBufferToTextureCommand(BufferView source,
5757
std::shared_ptr<Texture> destination,
58-
IPoint destination_origin,
58+
IRect destination_region,
5959
std::string label,
6060
uint32_t slice) override;
6161

‎impeller/renderer/backend/metal/blit_pass_mtl.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,14 @@
127127
bool BlitPassMTL::OnCopyBufferToTextureCommand(
128128
BufferView source,
129129
std::shared_ptr<Texture> destination,
130-
IPoint destination_origin,
130+
IRect destination_region,
131131
std::string label,
132132
uint32_t slice) {
133133
auto command = std::make_unique<BlitCopyBufferToTextureCommandMTL>();
134134
command->label = std::move(label);
135135
command->source = std::move(source);
136136
command->destination = std::move(destination);
137-
command->destination_origin = destination_origin;
137+
command->destination_region = destination_region;
138138
command->slice = slice;
139139

140140
commands_.emplace_back(std::move(command));

‎impeller/renderer/backend/vulkan/blit_command_vk.cc

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,12 @@ bool BlitCopyBufferToTextureCommandVK::Encode(CommandEncoderVK& encoder) const {
246246
image_copy.setBufferImageHeight(0);
247247
image_copy.setImageSubresource(
248248
vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1));
249-
image_copy.setImageOffset(
250-
vk::Offset3D(destination_origin.x, destination_origin.y, 0));
251-
image_copy.setImageExtent(vk::Extent3D(destination->GetSize().width,
252-
destination->GetSize().height, 1));
249+
image_copy.imageOffset.x = destination_region.GetX();
250+
image_copy.imageOffset.y = destination_region.GetY();
251+
image_copy.imageOffset.z = 0u;
252+
image_copy.imageExtent.width = destination_region.GetWidth();
253+
image_copy.imageExtent.height = destination_region.GetHeight();
254+
image_copy.imageExtent.depth = 1u;
253255

254256
if (!dst.SetLayout(dst_barrier)) {
255257
VALIDATION_LOG << "Could not encode layout transition.";

‎impeller/renderer/backend/vulkan/blit_pass_vk.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,14 @@ bool BlitPassVK::OnCopyTextureToBufferCommand(
9696
bool BlitPassVK::OnCopyBufferToTextureCommand(
9797
BufferView source,
9898
std::shared_ptr<Texture> destination,
99-
IPoint destination_origin,
99+
IRect destination_region,
100100
std::string label,
101101
uint32_t slice) {
102102
auto command = std::make_unique<BlitCopyBufferToTextureCommandVK>();
103103

104104
command->source = std::move(source);
105105
command->destination = std::move(destination);
106-
command->destination_origin = destination_origin;
106+
command->destination_region = destination_region;
107107
command->label = std::move(label);
108108
command->slice = slice;
109109

‎impeller/renderer/backend/vulkan/blit_pass_vk.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BLIT_PASS_VK_H_
77

88
#include "flutter/impeller/base/config.h"
9+
#include "impeller/geometry/rect.h"
910
#include "impeller/renderer/backend/vulkan/blit_command_vk.h"
1011
#include "impeller/renderer/blit_pass.h"
1112

@@ -55,7 +56,7 @@ class BlitPassVK final : public BlitPass {
5556
// |BlitPass|
5657
bool OnCopyBufferToTextureCommand(BufferView source,
5758
std::shared_ptr<Texture> destination,
58-
IPoint destination_origin,
59+
IRect destination_region,
5960
std::string label,
6061
uint32_t slice) override;
6162
// |BlitPass|

‎impeller/renderer/blit_command.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ struct BlitCopyTextureToBufferCommand : public BlitCommand {
3232
struct BlitCopyBufferToTextureCommand : public BlitCommand {
3333
BufferView source;
3434
std::shared_ptr<Texture> destination;
35-
IPoint destination_origin;
35+
IRect destination_region;
3636
uint32_t slice = 0;
3737
};
3838

‎impeller/renderer/blit_pass.cc

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,28 +122,41 @@ bool BlitPass::AddCopy(std::shared_ptr<Texture> source,
122122

123123
bool BlitPass::AddCopy(BufferView source,
124124
std::shared_ptr<Texture> destination,
125-
IPoint destination_origin,
125+
std::optional<IRect> destination_region,
126126
std::string label,
127127
uint32_t slice) {
128128
if (!destination) {
129129
VALIDATION_LOG << "Attempted to add a texture blit with no destination.";
130130
return false;
131131
}
132+
ISize destination_size = destination->GetSize();
133+
IRect destination_region_value =
134+
destination_region.value_or(IRect::MakeSize(destination_size));
135+
if (destination_region_value.GetX() < 0 ||
136+
destination_region_value.GetY() < 0 ||
137+
destination_region_value.GetRight() > destination_size.width ||
138+
destination_region_value.GetBottom() > destination_size.height) {
139+
VALIDATION_LOG << "Blit region cannot be larger than destination texture.";
140+
return false;
141+
}
132142

133143
auto bytes_per_pixel =
134144
BytesPerPixelForPixelFormat(destination->GetTextureDescriptor().format);
135-
auto bytes_per_image =
136-
destination->GetTextureDescriptor().size.Area() * bytes_per_pixel;
145+
auto bytes_per_image = destination_region_value.Area() * bytes_per_pixel;
137146

138147
if (source.range.length != bytes_per_image) {
139148
VALIDATION_LOG
140149
<< "Attempted to add a texture blit with out of bounds access.";
141150
return false;
142151
}
152+
if (slice > 5) {
153+
VALIDATION_LOG << "Invalid value for slice: " << slice;
154+
return false;
155+
}
143156

144157
return OnCopyBufferToTextureCommand(std::move(source), std::move(destination),
145-
destination_origin, std::move(label),
146-
slice);
158+
destination_region_value,
159+
std::move(label), slice);
147160
}
148161

149162
bool BlitPass::GenerateMipmap(std::shared_ptr<Texture> texture,

0 commit comments

Comments
 (0)