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

[Impeller] Update BlitPass::AddCopy to use destination_region instead of origin for buffer to texture copies. #52555

Merged
merged 8 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "impeller/aiks/image_filter.h"
#include "impeller/aiks/paint_pass_delegate.h"
#include "impeller/aiks/testing/context_spy.h"
#include "impeller/core/device_buffer.h"
#include "impeller/entity/contents/solid_color_contents.h"
#include "impeller/geometry/color.h"
#include "impeller/geometry/constants.h"
Expand Down Expand Up @@ -3151,6 +3152,39 @@ TEST_P(AiksTest, CanRenderTextWithLargePerspectiveTransform) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, SetContentsWithRegion) {
auto bridge = CreateTextureForFixture("bay_bridge.jpg");

// Replace part of the texture with a red rectangle.
std::vector<uint8_t> bytes(100 * 100 * 4);
for (auto i = 0u; i < bytes.size(); i += 4) {
bytes[i] = 255;
bytes[i + 1] = 0;
bytes[i + 2] = 0;
bytes[i + 3] = 255;
}
auto mapping =
std::make_shared<fml::NonOwnedMapping>(bytes.data(), bytes.size());
auto device_buffer =
GetContext()->GetResourceAllocator()->CreateBufferWithCopy(*mapping);
auto cmd_buffer = GetContext()->CreateCommandBuffer();
auto blit_pass = cmd_buffer->CreateBlitPass();
blit_pass->AddCopy(DeviceBuffer::AsBufferView(device_buffer), bridge,
IRect::MakeLTRB(50, 50, 150, 150));

auto did_submit =
blit_pass->EncodeCommands(GetContext()->GetResourceAllocator()) &&
GetContext()->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok();
ASSERT_TRUE(did_submit);

auto image = std::make_shared<Image>(bridge);

Canvas canvas;
canvas.DrawImage(image, {0, 0}, {});

ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

} // namespace testing
} // namespace impeller

Expand Down
28 changes: 23 additions & 5 deletions impeller/renderer/backend/gles/blit_command_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ bool BlitCopyBufferToTextureCommandGLES::Encode(
}

if (!tex_descriptor.IsValid() ||
source.range.length < tex_descriptor.GetByteSizeOfBaseMipLevel()) {
source.range.length !=
BytesPerPixelForPixelFormat(tex_descriptor.format) *
destination_region.Area()) {
return false;
}

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

{
TRACE_EVENT1("impeller", "TexImage2DUpload", "Bytes",
std::to_string(data.buffer_view.range.length).c_str());
// GL_INVALID_OPERATION if the texture array has not been
// defined by a previous glTexImage2D operation.
if (!texture_gles.IsSliceInitialized(slice)) {
gl.TexImage2D(texture_target, // target
0u, // LOD level
data.internal_format, // internal format
Expand All @@ -276,8 +278,24 @@ bool BlitCopyBufferToTextureCommandGLES::Encode(
data.type, // type
tex_data // data
);
texture_gles.MarkSliceInitialized(slice);
}

{
TRACE_EVENT1("impeller", "TexImage2DUpload", "Bytes",
std::to_string(data.buffer_view.range.length).c_str());
gl.TexSubImage2D(texture_target, // target
0u, // LOD level
destination_region.GetX(), // xoffset
destination_region.GetY(), // yoffset
destination_region.GetWidth(), // width
destination_region.GetHeight(), // height
data.external_format, // external format
data.type, // type
tex_data // data

);
}
texture_gles.MarkContentsInitialized();
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions impeller/renderer/backend/gles/blit_pass_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ bool BlitPassGLES::OnCopyTextureToBufferCommand(
bool BlitPassGLES::OnCopyBufferToTextureCommand(
BufferView source,
std::shared_ptr<Texture> destination,
IPoint destination_origin,
IRect destination_region,
std::string label,
uint32_t slice) {
auto command = std::make_unique<BlitCopyBufferToTextureCommandGLES>();
command->label = label;
command->source = std::move(source);
command->destination = std::move(destination);
command->destination_origin = destination_origin;
command->destination_region = destination_region;
command->label = label;
command->slice = slice;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere, we should check that slices are from 0 to 5. And, if not, either return false or clamp to the values. I prefer the former.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this check it blit_pass.cc and a test case.


Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/gles/blit_pass_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class BlitPassGLES final : public BlitPass,
// |BlitPass|
bool OnCopyBufferToTextureCommand(BufferView source,
std::shared_ptr<Texture> destination,
IPoint destination_origin,
IRect destination_region,
std::string label,
uint32_t slice) override;

Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/gles/proc_table_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ struct GLProc {
PROC(StencilMaskSeparate); \
PROC(StencilOpSeparate); \
PROC(TexImage2D); \
PROC(TexSubImage2D); \
PROC(TexParameteri); \
PROC(TexParameterfv); \
PROC(Uniform1fv); \
Expand Down
19 changes: 10 additions & 9 deletions impeller/renderer/backend/gles/texture_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,8 @@ bool TextureGLES::OnSetContents(std::shared_ptr<const fml::Mapping> mapping,
}
};

contents_initialized_ = reactor_->AddOperation(texture_upload);
return contents_initialized_;
slices_initialized_ = reactor_->AddOperation(texture_upload);
return slices_initialized_[0];
}

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

void TextureGLES::InitializeContentsIfNecessary() const {
if (!IsValid()) {
return;
}
if (contents_initialized_) {
if (!IsValid() || slices_initialized_[0]) {
return;
}
contents_initialized_ = true;
slices_initialized_[0] = true;

if (is_wrapped_) {
return;
Expand Down Expand Up @@ -455,8 +452,12 @@ bool TextureGLES::Bind() const {
return true;
}

void TextureGLES::MarkContentsInitialized() const {
contents_initialized_ = true;
void TextureGLES::MarkSliceInitialized(size_t slice) const {
slices_initialized_[slice] = true;
}

bool TextureGLES::IsSliceInitialized(size_t slice) const {
return slices_initialized_[slice];
}

bool TextureGLES::GenerateMipmap() {
Expand Down
11 changes: 9 additions & 2 deletions impeller/renderer/backend/gles/texture_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_TEXTURE_GLES_H_
#define FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_TEXTURE_GLES_H_

#include <bitset>

#include "impeller/base/backend_cast.h"
#include "impeller/core/texture.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
Expand Down Expand Up @@ -60,13 +62,18 @@ class TextureGLES final : public Texture,

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

void MarkContentsInitialized() const;
// For non cubemap textures, 0 indicates uninitialized and 1 indicates
// initialized. For cubemap textures, each face is initialized separately with
// each bit tracking the initialization of the corresponding slice.
void MarkSliceInitialized(size_t slice) const;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I was missing last time. For the GLES cube map textures we need to tracking initialization per slice


bool IsSliceInitialized(size_t slice) const;

private:
ReactorGLES::Ref reactor_;
const Type type_;
HandleGLES handle_;
mutable bool contents_initialized_ = false;
mutable std::bitset<6> slices_initialized_ = 0;
const bool is_wrapped_;
const std::optional<GLuint> wrapped_fbo_;
bool is_valid_ = false;
Expand Down
15 changes: 7 additions & 8 deletions impeller/renderer/backend/metal/blit_command_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,21 @@
}

auto destination_origin_mtl =
MTLOriginMake(destination_origin.x, destination_origin.y, 0);

auto image_size = destination->GetTextureDescriptor().size;
auto source_size_mtl = MTLSizeMake(image_size.width, image_size.height, 1);
MTLOriginMake(destination_region.GetX(), destination_region.GetY(), 0);
auto source_size_mtl = MTLSizeMake(destination_region.GetWidth(),
destination_region.GetHeight(), 1);

auto destination_bytes_per_pixel =
BytesPerPixelForPixelFormat(destination->GetTextureDescriptor().format);
auto destination_bytes_per_row =
source_size_mtl.width * destination_bytes_per_pixel;
auto destination_bytes_per_image =
source_size_mtl.height * destination_bytes_per_row;
destination_region.GetWidth() * destination_bytes_per_pixel;

[encoder copyFromBuffer:source_mtl
sourceOffset:source.range.offset
sourceBytesPerRow:destination_bytes_per_row
sourceBytesPerImage:destination_bytes_per_image
sourceBytesPerImage:
0 // 0 for 2D textures according to
// https://developer.apple.com/documentation/metal/mtlblitcommandencoder/1400752-copyfrombuffer
sourceSize:source_size_mtl
toTexture:destination_mtl
destinationSlice:slice
Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/metal/blit_pass_mtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class BlitPassMTL final : public BlitPass {
// |BlitPass|
bool OnCopyBufferToTextureCommand(BufferView source,
std::shared_ptr<Texture> destination,
IPoint destination_origin,
IRect destination_region,
std::string label,
uint32_t slice) override;

Expand Down
4 changes: 2 additions & 2 deletions impeller/renderer/backend/metal/blit_pass_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,14 @@
bool BlitPassMTL::OnCopyBufferToTextureCommand(
BufferView source,
std::shared_ptr<Texture> destination,
IPoint destination_origin,
IRect destination_region,
std::string label,
uint32_t slice) {
auto command = std::make_unique<BlitCopyBufferToTextureCommandMTL>();
command->label = std::move(label);
command->source = std::move(source);
command->destination = std::move(destination);
command->destination_origin = destination_origin;
command->destination_region = destination_region;
command->slice = slice;

commands_.emplace_back(std::move(command));
Expand Down
10 changes: 6 additions & 4 deletions impeller/renderer/backend/vulkan/blit_command_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,12 @@ bool BlitCopyBufferToTextureCommandVK::Encode(CommandEncoderVK& encoder) const {
image_copy.setBufferImageHeight(0);
image_copy.setImageSubresource(
vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1));
image_copy.setImageOffset(
vk::Offset3D(destination_origin.x, destination_origin.y, 0));
image_copy.setImageExtent(vk::Extent3D(destination->GetSize().width,
destination->GetSize().height, 1));
image_copy.imageOffset.x = destination_region.GetX();
image_copy.imageOffset.y = destination_region.GetY();
image_copy.imageOffset.z = 0u;
image_copy.imageExtent.width = destination_region.GetWidth();
image_copy.imageExtent.height = destination_region.GetHeight();
image_copy.imageExtent.depth = 1u;

if (!dst.SetLayout(dst_barrier)) {
VALIDATION_LOG << "Could not encode layout transition.";
Expand Down
4 changes: 2 additions & 2 deletions impeller/renderer/backend/vulkan/blit_pass_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,14 @@ bool BlitPassVK::OnCopyTextureToBufferCommand(
bool BlitPassVK::OnCopyBufferToTextureCommand(
BufferView source,
std::shared_ptr<Texture> destination,
IPoint destination_origin,
IRect destination_region,
std::string label,
uint32_t slice) {
auto command = std::make_unique<BlitCopyBufferToTextureCommandVK>();

command->source = std::move(source);
command->destination = std::move(destination);
command->destination_origin = destination_origin;
command->destination_region = destination_region;
command->label = std::move(label);
command->slice = slice;

Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/vulkan/blit_pass_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BLIT_PASS_VK_H_

#include "flutter/impeller/base/config.h"
#include "impeller/geometry/rect.h"
#include "impeller/renderer/backend/vulkan/blit_command_vk.h"
#include "impeller/renderer/blit_pass.h"

Expand Down Expand Up @@ -55,7 +56,7 @@ class BlitPassVK final : public BlitPass {
// |BlitPass|
bool OnCopyBufferToTextureCommand(BufferView source,
std::shared_ptr<Texture> destination,
IPoint destination_origin,
IRect destination_region,
std::string label,
uint32_t slice) override;
// |BlitPass|
Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/blit_command.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ struct BlitCopyTextureToBufferCommand : public BlitCommand {
struct BlitCopyBufferToTextureCommand : public BlitCommand {
BufferView source;
std::shared_ptr<Texture> destination;
IPoint destination_origin;
IRect destination_region;
uint32_t slice = 0;
};

Expand Down
23 changes: 18 additions & 5 deletions impeller/renderer/blit_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,28 +122,41 @@ bool BlitPass::AddCopy(std::shared_ptr<Texture> source,

bool BlitPass::AddCopy(BufferView source,
std::shared_ptr<Texture> destination,
IPoint destination_origin,
std::optional<IRect> destination_region,
std::string label,
uint32_t slice) {
if (!destination) {
VALIDATION_LOG << "Attempted to add a texture blit with no destination.";
return false;
}
ISize destination_size = destination->GetSize();
IRect destination_region_value =
destination_region.value_or(IRect::MakeSize(destination_size));
if (destination_region_value.GetX() < 0 ||
destination_region_value.GetY() < 0 ||
destination_region_value.GetRight() > destination_size.width ||
destination_region_value.GetBottom() > destination_size.height) {
VALIDATION_LOG << "Blit region cannot be larger than destination texture.";
return false;
}

auto bytes_per_pixel =
BytesPerPixelForPixelFormat(destination->GetTextureDescriptor().format);
auto bytes_per_image =
destination->GetTextureDescriptor().size.Area() * bytes_per_pixel;
auto bytes_per_image = destination_region_value.Area() * bytes_per_pixel;

if (source.range.length != bytes_per_image) {
VALIDATION_LOG
<< "Attempted to add a texture blit with out of bounds access.";
return false;
}
if (slice > 5) {
VALIDATION_LOG << "Invalid value for slice: " << slice;
return false;
}

return OnCopyBufferToTextureCommand(std::move(source), std::move(destination),
destination_origin, std::move(label),
slice);
destination_region_value,
std::move(label), slice);
}

bool BlitPass::GenerateMipmap(std::shared_ptr<Texture> texture,
Expand Down
Loading