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

[Impeller] null check samplers #37489

Merged
merged 26 commits into from
Nov 11, 2022
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
6 changes: 5 additions & 1 deletion display_list/display_list_color_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,11 @@ class DlRuntimeEffectColorSource final : public DlColorSource {
}
std::vector<sk_sp<SkShader>> sk_samplers(samplers_.size());
for (size_t i = 0; i < samplers_.size(); i++) {
sk_samplers[i] = samplers_[i]->skia_object();
auto sampler = samplers_[i];
if (sampler == nullptr) {
return nullptr;
}
sk_samplers[i] = sampler->skia_object();
}

auto ref = new std::shared_ptr<std::vector<uint8_t>>(uniform_data_);
Expand Down
9 changes: 9 additions & 0 deletions display_list/display_list_color_source_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -974,5 +974,14 @@ TEST(DisplayListColorSource, RuntimeEffect) {
TestNotEquals(source2, source3, "SkRuntimeEffect differs");
}

TEST(DisplayListColorSource, RuntimeEffectWithNullSampler) {
std::shared_ptr<DlRuntimeEffectColorSource> source1 =
DlColorSource::MakeRuntimeEffect(
kTestRuntimeEffect1, {nullptr},
std::make_shared<std::vector<uint8_t>>());

ASSERT_EQ(source1->skia_object(), nullptr);
}

} // namespace testing
} // namespace flutter
3 changes: 3 additions & 0 deletions impeller/display_list/display_list_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,9 @@ void DisplayListDispatcher::setColorSource(
std::vector<RuntimeEffectContents::TextureInput> texture_inputs;

for (auto& sampler : samplers) {
if (sampler == nullptr) {
return;
}
auto* image = sampler->asImage();
if (!sampler->asImage()) {
UNIMPLEMENTED;
Expand Down
1 change: 1 addition & 0 deletions lib/ui/dart_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ typedef CanvasPath Path;
V(FragmentProgram, initFromAsset, 2) \
V(ReusableFragmentShader, Dispose, 1) \
V(ReusableFragmentShader, SetSampler, 3) \
V(ReusableFragmentShader, ValidateSamplers, 1) \
V(Gradient, initLinear, 6) \
V(Gradient, initRadial, 8) \
V(Gradient, initSweep, 9) \
Expand Down
28 changes: 23 additions & 5 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,14 @@ class Paint {
);
return true;
}());
assert(() {
if (value is FragmentShader) {
if (!value._validateSamplers()) {
throw Exception('Invalid FragmentShader ${value._debugName ?? ''}: missing sampler');
}
}
return true;
}());
_ensureObjectsInitialized()[_kShaderIndex] = value;
}

Expand Down Expand Up @@ -4151,10 +4159,16 @@ class FragmentProgram extends NativeFieldWrapperClass1 {
_constructor();
final String result = _initFromAsset(assetKey);
if (result.isNotEmpty) {
throw result; // ignore: only_throw_errors
throw Exception(result);
}
assert(() {
_debugName = assetKey;
return true;
}());
}

String? _debugName;

// TODO(zra): Document custom shaders on the website and add a link to it
// here. https://github.com/flutter/flutter/issues/107929.
/// Creates a fragment program from the asset with key [assetKey].
Expand Down Expand Up @@ -4222,7 +4236,7 @@ class FragmentProgram extends NativeFieldWrapperClass1 {
external String _initFromAsset(String assetKey);

/// Returns a fresh instance of [FragmentShader].
FragmentShader fragmentShader() => FragmentShader._(this);
FragmentShader fragmentShader() => FragmentShader._(this, debugName: _debugName);
}

/// A [Shader] generated from a [FragmentProgram].
Expand All @@ -4239,17 +4253,18 @@ class FragmentProgram extends NativeFieldWrapperClass1 {
/// are required to exist simultaneously, they must be obtained from two
/// different calls to [FragmentProgram.fragmentShader].
class FragmentShader extends Shader {
FragmentShader._(FragmentProgram program) : super._() {
FragmentShader._(FragmentProgram program, { String? debugName }) : _debugName = debugName, super._() {
_floats = _constructor(
program,
program._uniformFloatCount,
program._samplerCount,
);
}

static final Float32List _kEmptyFloat32List = Float32List(0);
final String? _debugName;

late Float32List _floats;
static final Float32List _kEmptyFloat32List = Float32List(0);
Float32List _floats = _kEmptyFloat32List;

/// Sets the float uniform at [index] to [value].
void setFloat(int index, double value) {
Expand Down Expand Up @@ -4284,6 +4299,9 @@ class FragmentShader extends Shader {
@FfiNative<Void Function(Pointer<Void>, Handle, Handle)>('ReusableFragmentShader::SetSampler')
external void _setSampler(int index, ImageShader sampler);

@FfiNative<Bool Function(Pointer<Void>)>('ReusableFragmentShader::ValidateSamplers')
external bool _validateSamplers();

@FfiNative<Void Function(Pointer<Void>)>('ReusableFragmentShader::Dispose')
external void _dispose();
}
Expand Down
2 changes: 0 additions & 2 deletions lib/ui/painting/fragment_program.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <iostream>
#include <memory>
#include <sstream>

Expand Down Expand Up @@ -78,7 +77,6 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) {
if (Dart_IsError(ths)) {
Dart_PropagateError(ths);
}

Dart_Handle result = Dart_SetField(ths, tonic::ToDart("_samplerCount"),
Dart_NewInteger(sampled_image_count));
if (Dart_IsError(result)) {
Expand Down
10 changes: 9 additions & 1 deletion lib/ui/painting/fragment_shader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <iostream>
#include <memory>
#include <utility>

Expand Down Expand Up @@ -53,6 +52,15 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper,
float_count);
}

bool ReusableFragmentShader::ValidateSamplers() {
for (auto i = 0u; i < samplers_.size(); i += 1) {
if (samplers_[i] == nullptr) {
return false;
}
}
return true;
}

void ReusableFragmentShader::SetSampler(Dart_Handle index_handle,
Dart_Handle sampler_handle) {
uint64_t index = tonic::DartConverter<uint64_t>::FromDart(index_handle);
Expand Down
2 changes: 2 additions & 0 deletions lib/ui/painting/fragment_shader.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class ReusableFragmentShader : public Shader {

void SetSampler(Dart_Handle index, Dart_Handle sampler);

bool ValidateSamplers();

void Dispose();

// |Shader|
Expand Down
19 changes: 19 additions & 0 deletions testing/dart/fragment_shader_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,25 @@ void main() async {
}
});

test('FragmentShader with sampler asserts if sampler is missing when assigned to paint', () async {
if (!assertsEnabled) {
return;
}
final FragmentProgram program = await FragmentProgram.fromAsset(
'blue_green_sampler.frag.iplr',
);
final FragmentShader fragmentShader = program.fragmentShader();

try {
Paint().shader = fragmentShader;
fail('Expected to throw');
} catch (err) {
expect(err.toString(), contains('Invalid FragmentShader blue_green_sampler.frag.iplr'));
} finally {
fragmentShader.dispose();
}
});

test('Disposed FragmentShader on Paint', () async {
final FragmentProgram program = await FragmentProgram.fromAsset(
'blue_green_sampler.frag.iplr',
Expand Down