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

[Windows] Improve texture format logic #54329

Merged
auto-submit merged 1 commit into from
Aug 12, 2024
Merged

[Windows] Improve texture format logic #54329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 27 additions & 2 deletions shell/platform/embedder/embedder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,19 @@ static const SkIRect FlutterRectToSkIRect(FlutterRect flutter_rect) {
#define GL_BGRA8_EXT 0x93A1
#endif

static std::optional<SkColorType> FlutterFormatToSkColorType(uint32_t format) {
switch (format) {
case GL_BGRA8_EXT:
return kBGRA_8888_SkColorType;
case GL_RGBA8:
return kRGBA_8888_SkColorType;
default:
FML_LOG(ERROR) << "Cannot convert format " << format
<< " to SkColorType.";
return std::nullopt;
}
}

#endif

static inline flutter::Shell::CreateCallback<flutter::PlatformView>
Expand Down Expand Up @@ -769,12 +782,18 @@ static sk_sp<SkSurface> MakeSkSurfaceFromBackingStore(

SkSurfaceProps surface_properties(0, kUnknown_SkPixelGeometry);

std::optional<SkColorType> color_type =
FlutterFormatToSkColorType(texture->format);
if (!color_type) {
return nullptr;
}

auto surface = SkSurfaces::WrapBackendTexture(
context, // context
backend_texture, // back-end texture
kBottomLeft_GrSurfaceOrigin, // surface origin
1, // sample count
kN32_SkColorType, // color type
Copy link
Member Author

@loic-sharma loic-sharma Aug 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

kN32_SkColorType is either BGRA8 or RGBA8 depending on the machine that builds the engine. This caused Skia to reject RGBA8 textures if the engine was built on a machine where BGRA8 is the native color format.

Copy link
Member

@cbracken cbracken Aug 2, 2024

Choose a reason for hiding this comment

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

Oh wow - nice catch.

color_type.value(), // color type
SkColorSpace::MakeSRGB(), // color space
&surface_properties, // surface properties
static_cast<SkSurfaces::TextureReleaseProc>(
Expand Down Expand Up @@ -812,11 +831,17 @@ static sk_sp<SkSurface> MakeSkSurfaceFromBackingStore(

SkSurfaceProps surface_properties(0, kUnknown_SkPixelGeometry);

std::optional<SkColorType> color_type =
FlutterFormatToSkColorType(framebuffer->target);
if (!color_type) {
return nullptr;
}

auto surface = SkSurfaces::WrapBackendRenderTarget(
context, // context
backend_render_target, // backend render target
kBottomLeft_GrSurfaceOrigin, // surface origin
kN32_SkColorType, // color type
color_type.value(), // color type
SkColorSpace::MakeSRGB(), // color space
&surface_properties, // surface properties
static_cast<SkSurfaces::RenderTargetReleaseProc>(
Expand Down
33 changes: 14 additions & 19 deletions shell/platform/windows/compositor_opengl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,6 @@ struct FramebufferBackingStore {
uint32_t texture_id;
};

// Based off Skia's logic:
// https://github.com/google/skia/blob/4738ed711e03212aceec3cd502a4adb545f38e63/src/gpu/ganesh/gl/GrGLCaps.cpp#L1963-L2116
int GetSupportedTextureFormat(const impeller::DescriptionGLES* description) {
if (description->HasExtension("GL_EXT_texture_format_BGRA8888")) {
return GL_BGRA8_EXT;
} else if (description->HasExtension("GL_APPLE_texture_format_BGRA8888") &&
description->GetGlVersion().IsAtLeast(impeller::Version(3, 0))) {
return GL_BGRA8_EXT;
} else {
return GL_RGBA8;
}
}

} // namespace

CompositorOpenGL::CompositorOpenGL(FlutterWindowsEngine* engine,
Expand All @@ -58,17 +45,18 @@ bool CompositorOpenGL::CreateBackingStore(
gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
gl_->TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, config.size.width,
config.size.height, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
gl_->TexImage2D(GL_TEXTURE_2D, 0, format_.general_format, config.size.width,
config.size.height, 0, format_.general_format,
GL_UNSIGNED_BYTE, nullptr);
gl_->BindTexture(GL_TEXTURE_2D, 0);

gl_->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0_EXT,
Copy link
Member Author

@loic-sharma loic-sharma Aug 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

GL_COLOR_ATTACHMENT0 is the same value as GL_COLOR_ATTACHMENT0_EXT but available in OpenGL ES 2.0.

GL_TEXTURE_2D, store->texture_id, 0);
gl_->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
store->texture_id, 0);

result->type = kFlutterBackingStoreTypeOpenGL;
result->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;
result->open_gl.framebuffer.name = store->framebuffer_id;
result->open_gl.framebuffer.target = format_;
result->open_gl.framebuffer.target = format_.sized_format;
result->open_gl.framebuffer.user_data = store.release();
result->open_gl.framebuffer.destruction_callback = [](void* user_data) {
// Backing store destroyed in `CompositorOpenGL::CollectBackingStore`, set
Expand Down Expand Up @@ -185,7 +173,14 @@ bool CompositorOpenGL::Initialize() {
return false;
}

format_ = GetSupportedTextureFormat(gl_->GetDescription());
if (gl_->GetDescription()->HasExtension("GL_EXT_texture_format_BGRA8888")) {
format_.sized_format = GL_BGRA8_EXT;
format_.general_format = GL_BGRA_EXT;
} else {
format_.sized_format = GL_RGBA8;
format_.general_format = GL_RGBA;
}

is_initialized_ = true;
return true;
}
Expand Down
11 changes: 9 additions & 2 deletions shell/platform/windows/compositor_opengl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ class CompositorOpenGL : public Compositor {
FlutterWindowsEngine* engine_;

private:
struct TextureFormat {
// The format passed to the engine using `FlutterOpenGLFramebuffer.target`.
uint32_t sized_format = 0;
// The format used to create textures. Passed to `glTexImage2D`.
uint32_t general_format = 0;
};

// The compositor initializes itself lazily once |CreateBackingStore| is
// called. True if initialization completed successfully.
bool is_initialized_ = false;
Expand All @@ -48,9 +55,9 @@ class CompositorOpenGL : public Compositor {
// Table of resolved GLES functions. Null until the compositor is initialized.
std::unique_ptr<impeller::ProcTableGLES> gl_ = nullptr;

// The OpenGL texture target format for backing stores. Invalid value until
// The OpenGL texture format for backing stores. Invalid value until
// the compositor is initialized.
uint32_t format_ = 0;
TextureFormat format_;

// Initialize the compositor. This must run on the raster thread.
bool Initialize();
Expand Down