Skip to content

Commit 831a7a1

Browse files
bderobdero
authored
[Flutter GPU] Fix Android/GLES crashers. (flutter#172588)
Continuation of flutter#165630. Resolves flutter#164278. Resolves bdero/flutter_scene#35. Partly fixes: flutter#160948. * Fixes OpenGLES crashers. * Threadsafe GLES pipeline creation. * Threadsafe GLES texture creation. * Fix GLES internal float texture formats. * Encode GLES passes on the raster thread. * Overwrite GLES textures on the raster thread. * Fixes vulkan memory leak (flutter#164278).
1 parent 24fc562 commit 831a7a1

30 files changed

+190
-85
lines changed

‎engine/src/flutter/impeller/core/allocator.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ std::shared_ptr<DeviceBuffer> Allocator::CreateBuffer(
4646
return OnCreateBuffer(desc);
4747
}
4848

49-
std::shared_ptr<Texture> Allocator::CreateTexture(
50-
const TextureDescriptor& desc) {
49+
std::shared_ptr<Texture> Allocator::CreateTexture(const TextureDescriptor& desc,
50+
bool threadsafe) {
5151
const auto max_size = GetMaxTextureSizeSupported();
5252
if (desc.size.width > max_size.width || desc.size.height > max_size.height) {
5353
VALIDATION_LOG << "Requested texture size " << desc.size
@@ -60,10 +60,10 @@ std::shared_ptr<Texture> Allocator::CreateTexture(
6060
<< " exceeds maximum supported for size " << desc.size;
6161
TextureDescriptor corrected_desc = desc;
6262
corrected_desc.mip_count = desc.size.MipCount();
63-
return OnCreateTexture(corrected_desc);
63+
return OnCreateTexture(corrected_desc, threadsafe);
6464
}
6565

66-
return OnCreateTexture(desc);
66+
return OnCreateTexture(desc, threadsafe);
6767
}
6868

6969
uint16_t Allocator::MinimumBytesPerRow(PixelFormat format) const {

‎engine/src/flutter/impeller/core/allocator.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,23 @@ class Allocator {
3030
std::shared_ptr<DeviceBuffer> CreateBuffer(
3131
const DeviceBufferDescriptor& desc);
3232

33-
std::shared_ptr<Texture> CreateTexture(const TextureDescriptor& desc);
33+
//------------------------------------------------------------------------------
34+
/// @brief Creates a new texture.
35+
///
36+
/// @param[in] desc The descriptor of the texture to create.
37+
/// @param[in] threadsafe Whether mutations to this texture should be
38+
/// protected with a threadsafe barrier.
39+
///
40+
/// This parameter only affects the OpenGLES rendering
41+
/// backend.
42+
///
43+
/// If any interaction with this texture (including
44+
/// creation) will be done on a thread other than
45+
/// where the OpenGLES context resides, then
46+
/// `threadsafe`, must be set to `true`.
47+
///
48+
std::shared_ptr<Texture> CreateTexture(const TextureDescriptor& desc,
49+
bool threadsafe = false);
3450

3551
//------------------------------------------------------------------------------
3652
/// @brief Minimum value for `row_bytes` on a Texture. The row
@@ -62,7 +78,8 @@ class Allocator {
6278
const DeviceBufferDescriptor& desc) = 0;
6379

6480
virtual std::shared_ptr<Texture> OnCreateTexture(
65-
const TextureDescriptor& desc) = 0;
81+
const TextureDescriptor& desc,
82+
bool threadsafe = false) = 0;
6683

6784
private:
6885
Allocator(const Allocator&) = delete;

‎engine/src/flutter/impeller/entity/contents/host_buffer_unittests.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,8 @@ class FailingAllocator : public Allocator {
206206
return delegate_->CreateBuffer(desc);
207207
}
208208

209-
std::shared_ptr<Texture> OnCreateTexture(const TextureDescriptor& desc) {
209+
std::shared_ptr<Texture> OnCreateTexture(const TextureDescriptor& desc,
210+
bool threadsafe) {
210211
return delegate_->CreateTexture(desc);
211212
}
212213

‎engine/src/flutter/impeller/entity/render_target_cache_unittests.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ class TestAllocator : public Allocator {
3939
};
4040

4141
virtual std::shared_ptr<Texture> OnCreateTexture(
42-
const TextureDescriptor& desc) override {
42+
const TextureDescriptor& desc,
43+
bool threadsafe) override {
4344
if (should_fail) {
4445
return nullptr;
4546
}

‎engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ std::shared_ptr<DeviceBuffer> AllocatorGLES::OnCreateBuffer(
3939

4040
// |Allocator|
4141
std::shared_ptr<Texture> AllocatorGLES::OnCreateTexture(
42-
const TextureDescriptor& desc) {
43-
return std::make_shared<TextureGLES>(reactor_, desc);
42+
const TextureDescriptor& desc,
43+
bool threadsafe) {
44+
return std::make_shared<TextureGLES>(reactor_, desc, threadsafe);
4445
}
4546

4647
// |Allocator|

‎engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ class AllocatorGLES final : public Allocator {
3131
const DeviceBufferDescriptor& desc) override;
3232

3333
// |Allocator|
34-
std::shared_ptr<Texture> OnCreateTexture(
35-
const TextureDescriptor& desc) override;
34+
std::shared_ptr<Texture> OnCreateTexture(const TextureDescriptor& desc,
35+
bool threadsafe) override;
3636

3737
// |Allocator|
3838
ISize GetMaxTextureSizeSupported() const override;

‎engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.cc

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,8 @@ std::shared_ptr<PipelineGLES> PipelineLibraryGLES::CreatePipeline(
185185
const std::weak_ptr<PipelineLibrary>& weak_library,
186186
const PipelineDescriptor& desc,
187187
const std::shared_ptr<const ShaderFunction>& vert_function,
188-
const std::shared_ptr<const ShaderFunction>& frag_function) {
188+
const std::shared_ptr<const ShaderFunction>& frag_function,
189+
bool threadsafe) {
189190
auto strong_library = weak_library.lock();
190191

191192
if (!strong_library) {
@@ -209,14 +210,22 @@ std::shared_ptr<PipelineGLES> PipelineLibraryGLES::CreatePipeline(
209210

210211
const auto has_cached_program = !!cached_program;
211212

212-
auto pipeline = std::shared_ptr<PipelineGLES>(new PipelineGLES(
213-
reactor, //
214-
weak_library, //
215-
desc, //
216-
has_cached_program
217-
? std::move(cached_program)
218-
: std::make_shared<UniqueHandleGLES>(UniqueHandleGLES::MakeUntracked(
219-
reactor, HandleType::kProgram))));
213+
std::shared_ptr<UniqueHandleGLES> program_handle = nullptr;
214+
if (has_cached_program) {
215+
program_handle = std::move(cached_program);
216+
} else {
217+
program_handle = threadsafe ? std::make_shared<UniqueHandleGLES>(
218+
reactor, HandleType::kProgram)
219+
: std::make_shared<UniqueHandleGLES>(
220+
UniqueHandleGLES::MakeUntracked(
221+
reactor, HandleType::kProgram));
222+
}
223+
224+
auto pipeline = std::shared_ptr<PipelineGLES>(
225+
new PipelineGLES(reactor, //
226+
weak_library, //
227+
desc, //
228+
std::move(program_handle)));
220229

221230
auto program = reactor->GetGLHandle(pipeline->GetProgramHandle());
222231

@@ -258,7 +267,8 @@ std::shared_ptr<PipelineGLES> PipelineLibraryGLES::CreatePipeline(
258267
// |PipelineLibrary|
259268
PipelineFuture<PipelineDescriptor> PipelineLibraryGLES::GetPipeline(
260269
PipelineDescriptor descriptor,
261-
bool async) {
270+
bool async,
271+
bool threadsafe) {
262272
if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) {
263273
return found->second;
264274
}
@@ -290,10 +300,11 @@ PipelineFuture<PipelineDescriptor> PipelineLibraryGLES::GetPipeline(
290300
weak_this = weak_from_this(), //
291301
descriptor, //
292302
vert_function, //
293-
frag_function //
303+
frag_function, //
304+
threadsafe //
294305
](const ReactorGLES& reactor) {
295-
promise->set_value(
296-
CreatePipeline(weak_this, descriptor, vert_function, frag_function));
306+
promise->set_value(CreatePipeline(weak_this, descriptor, vert_function,
307+
frag_function, threadsafe));
297308
});
298309
FML_CHECK(result);
299310

‎engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ class PipelineLibraryGLES final
9999

100100
// |PipelineLibrary|
101101
PipelineFuture<PipelineDescriptor> GetPipeline(PipelineDescriptor descriptor,
102-
bool async) override;
102+
bool async,
103+
bool threadsafe) override;
103104

104105
// |PipelineLibrary|
105106
PipelineFuture<ComputePipelineDescriptor> GetPipeline(
@@ -119,7 +120,8 @@ class PipelineLibraryGLES final
119120
const std::weak_ptr<PipelineLibrary>& weak_library,
120121
const PipelineDescriptor& desc,
121122
const std::shared_ptr<const ShaderFunction>& vert_shader,
122-
const std::shared_ptr<const ShaderFunction>& frag_shader);
123+
const std::shared_ptr<const ShaderFunction>& frag_shader,
124+
bool threadsafe);
123125

124126
std::shared_ptr<UniqueHandleGLES> GetProgramForKey(const ProgramKey& key);
125127

‎engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,12 @@ struct TexImage2DData {
8888
type = GL_UNSIGNED_BYTE;
8989
break;
9090
case PixelFormat::kR32G32B32A32Float:
91-
internal_format = GL_RGBA;
91+
internal_format = GL_RGBA32F;
9292
external_format = GL_RGBA;
9393
type = GL_FLOAT;
9494
break;
9595
case PixelFormat::kR16G16B16A16Float:
96-
internal_format = GL_RGBA;
96+
internal_format = GL_RGBA16F;
9797
external_format = GL_RGBA;
9898
type = GL_HALF_FLOAT;
9999
break;
@@ -149,7 +149,7 @@ std::shared_ptr<TextureGLES> TextureGLES::WrapFBO(
149149
TextureDescriptor desc,
150150
GLuint fbo) {
151151
auto texture = std::shared_ptr<TextureGLES>(
152-
new TextureGLES(std::move(reactor), desc, fbo, std::nullopt));
152+
new TextureGLES(std::move(reactor), desc, false, fbo, std::nullopt));
153153
if (!texture->IsValid()) {
154154
return nullptr;
155155
}
@@ -168,8 +168,8 @@ std::shared_ptr<TextureGLES> TextureGLES::WrapTexture(
168168
VALIDATION_LOG << "Cannot wrap a non-texture handle.";
169169
return nullptr;
170170
}
171-
auto texture = std::shared_ptr<TextureGLES>(
172-
new TextureGLES(std::move(reactor), desc, std::nullopt, external_handle));
171+
auto texture = std::shared_ptr<TextureGLES>(new TextureGLES(
172+
std::move(reactor), desc, false, std::nullopt, external_handle));
173173
if (!texture->IsValid()) {
174174
return nullptr;
175175
}
@@ -183,15 +183,18 @@ std::shared_ptr<TextureGLES> TextureGLES::CreatePlaceholder(
183183
}
184184

185185
TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
186-
TextureDescriptor desc)
186+
TextureDescriptor desc,
187+
bool threadsafe)
187188
: TextureGLES(std::move(reactor), //
188189
desc, //
190+
threadsafe, //
189191
std::nullopt, //
190192
std::nullopt //
191193
) {}
192194

193195
TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
194196
TextureDescriptor desc,
197+
bool threadsafe,
195198
std::optional<GLuint> fbo,
196199
std::optional<HandleGLES> external_handle)
197200
: Texture(desc),
@@ -201,7 +204,9 @@ TextureGLES::TextureGLES(std::shared_ptr<ReactorGLES> reactor,
201204
reactor_->GetProcTable().GetCapabilities())),
202205
handle_(external_handle.has_value()
203206
? external_handle.value()
204-
: reactor_->CreateUntrackedHandle(ToHandleType(type_))),
207+
: (threadsafe ? reactor_->CreateHandle(ToHandleType(type_))
208+
: reactor_->CreateUntrackedHandle(
209+
ToHandleType(type_)))),
205210
is_wrapped_(fbo.has_value() || external_handle.has_value()),
206211
wrapped_fbo_(fbo) {
207212
// Ensure the texture descriptor itself is valid.

‎engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ class TextureGLES final : public Texture,
7575
std::shared_ptr<ReactorGLES> reactor,
7676
TextureDescriptor desc);
7777

78-
TextureGLES(std::shared_ptr<ReactorGLES> reactor, TextureDescriptor desc);
78+
TextureGLES(std::shared_ptr<ReactorGLES> reactor,
79+
TextureDescriptor desc,
80+
bool threadsafe = false);
7981

8082
// |Texture|
8183
~TextureGLES() override;
@@ -164,6 +166,7 @@ class TextureGLES final : public Texture,
164166

165167
TextureGLES(std::shared_ptr<ReactorGLES> reactor,
166168
TextureDescriptor desc,
169+
bool threadsafe,
167170
std::optional<GLuint> fbo,
168171
std::optional<HandleGLES> external_handle);
169172

0 commit comments

Comments
 (0)