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

Commit 0721c86

Browse files
[Impeller] null check samplers (#37489)
* [Impeller] null check sampler and assert in Paint * ++ * ++ * ++ * ++ * Update fragment_shader_test.dart * ++ * TESTING * ++ * TESTING * ++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ * remove debugging code * ++ * ++
1 parent 53cfb94 commit 0721c86

9 files changed

+71
-9
lines changed

display_list/display_list_color_source.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,11 @@ class DlRuntimeEffectColorSource final : public DlColorSource {
705705
}
706706
std::vector<sk_sp<SkShader>> sk_samplers(samplers_.size());
707707
for (size_t i = 0; i < samplers_.size(); i++) {
708-
sk_samplers[i] = samplers_[i]->skia_object();
708+
auto sampler = samplers_[i];
709+
if (sampler == nullptr) {
710+
return nullptr;
711+
}
712+
sk_samplers[i] = sampler->skia_object();
709713
}
710714

711715
auto ref = new std::shared_ptr<std::vector<uint8_t>>(uniform_data_);

display_list/display_list_color_source_unittests.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,5 +974,14 @@ TEST(DisplayListColorSource, RuntimeEffect) {
974974
TestNotEquals(source2, source3, "SkRuntimeEffect differs");
975975
}
976976

977+
TEST(DisplayListColorSource, RuntimeEffectWithNullSampler) {
978+
std::shared_ptr<DlRuntimeEffectColorSource> source1 =
979+
DlColorSource::MakeRuntimeEffect(
980+
kTestRuntimeEffect1, {nullptr},
981+
std::make_shared<std::vector<uint8_t>>());
982+
983+
ASSERT_EQ(source1->skia_object(), nullptr);
984+
}
985+
977986
} // namespace testing
978987
} // namespace flutter

impeller/display_list/display_list_dispatcher.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,9 @@ void DisplayListDispatcher::setColorSource(
442442
std::vector<RuntimeEffectContents::TextureInput> texture_inputs;
443443

444444
for (auto& sampler : samplers) {
445+
if (sampler == nullptr) {
446+
return;
447+
}
445448
auto* image = sampler->asImage();
446449
if (!sampler->asImage()) {
447450
UNIMPLEMENTED;

lib/ui/dart_ui.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ typedef CanvasPath Path;
175175
V(FragmentProgram, initFromAsset, 2) \
176176
V(ReusableFragmentShader, Dispose, 1) \
177177
V(ReusableFragmentShader, SetSampler, 3) \
178+
V(ReusableFragmentShader, ValidateSamplers, 1) \
178179
V(Gradient, initLinear, 6) \
179180
V(Gradient, initRadial, 8) \
180181
V(Gradient, initSweep, 9) \

lib/ui/painting.dart

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,14 @@ class Paint {
14091409
);
14101410
return true;
14111411
}());
1412+
assert(() {
1413+
if (value is FragmentShader) {
1414+
if (!value._validateSamplers()) {
1415+
throw Exception('Invalid FragmentShader ${value._debugName ?? ''}: missing sampler');
1416+
}
1417+
}
1418+
return true;
1419+
}());
14121420
_ensureObjectsInitialized()[_kShaderIndex] = value;
14131421
}
14141422

@@ -4151,10 +4159,16 @@ class FragmentProgram extends NativeFieldWrapperClass1 {
41514159
_constructor();
41524160
final String result = _initFromAsset(assetKey);
41534161
if (result.isNotEmpty) {
4154-
throw result; // ignore: only_throw_errors
4162+
throw Exception(result);
41554163
}
4164+
assert(() {
4165+
_debugName = assetKey;
4166+
return true;
4167+
}());
41564168
}
41574169

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

42244238
/// Returns a fresh instance of [FragmentShader].
4225-
FragmentShader fragmentShader() => FragmentShader._(this);
4239+
FragmentShader fragmentShader() => FragmentShader._(this, debugName: _debugName);
42264240
}
42274241

42284242
/// A [Shader] generated from a [FragmentProgram].
@@ -4239,17 +4253,18 @@ class FragmentProgram extends NativeFieldWrapperClass1 {
42394253
/// are required to exist simultaneously, they must be obtained from two
42404254
/// different calls to [FragmentProgram.fragmentShader].
42414255
class FragmentShader extends Shader {
4242-
FragmentShader._(FragmentProgram program) : super._() {
4256+
FragmentShader._(FragmentProgram program, { String? debugName }) : _debugName = debugName, super._() {
42434257
_floats = _constructor(
42444258
program,
42454259
program._uniformFloatCount,
42464260
program._samplerCount,
42474261
);
42484262
}
42494263

4250-
static final Float32List _kEmptyFloat32List = Float32List(0);
4264+
final String? _debugName;
42514265

4252-
late Float32List _floats;
4266+
static final Float32List _kEmptyFloat32List = Float32List(0);
4267+
Float32List _floats = _kEmptyFloat32List;
42534268

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

4302+
@FfiNative<Bool Function(Pointer<Void>)>('ReusableFragmentShader::ValidateSamplers')
4303+
external bool _validateSamplers();
4304+
42874305
@FfiNative<Void Function(Pointer<Void>)>('ReusableFragmentShader::Dispose')
42884306
external void _dispose();
42894307
}

lib/ui/painting/fragment_program.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#include <iostream>
65
#include <memory>
76
#include <sstream>
87

@@ -78,7 +77,6 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) {
7877
if (Dart_IsError(ths)) {
7978
Dart_PropagateError(ths);
8079
}
81-
8280
Dart_Handle result = Dart_SetField(ths, tonic::ToDart("_samplerCount"),
8381
Dart_NewInteger(sampled_image_count));
8482
if (Dart_IsError(result)) {

lib/ui/painting/fragment_shader.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#include <iostream>
65
#include <memory>
76
#include <utility>
87

@@ -53,6 +52,15 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper,
5352
float_count);
5453
}
5554

55+
bool ReusableFragmentShader::ValidateSamplers() {
56+
for (auto i = 0u; i < samplers_.size(); i += 1) {
57+
if (samplers_[i] == nullptr) {
58+
return false;
59+
}
60+
}
61+
return true;
62+
}
63+
5664
void ReusableFragmentShader::SetSampler(Dart_Handle index_handle,
5765
Dart_Handle sampler_handle) {
5866
uint64_t index = tonic::DartConverter<uint64_t>::FromDart(index_handle);

lib/ui/painting/fragment_shader.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ class ReusableFragmentShader : public Shader {
3636

3737
void SetSampler(Dart_Handle index, Dart_Handle sampler);
3838

39+
bool ValidateSamplers();
40+
3941
void Dispose();
4042

4143
// |Shader|

testing/dart/fragment_shader_test.dart

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,25 @@ void main() async {
6262
}
6363
});
6464

65+
test('FragmentShader with sampler asserts if sampler is missing when assigned to paint', () async {
66+
if (!assertsEnabled) {
67+
return;
68+
}
69+
final FragmentProgram program = await FragmentProgram.fromAsset(
70+
'blue_green_sampler.frag.iplr',
71+
);
72+
final FragmentShader fragmentShader = program.fragmentShader();
73+
74+
try {
75+
Paint().shader = fragmentShader;
76+
fail('Expected to throw');
77+
} catch (err) {
78+
expect(err.toString(), contains('Invalid FragmentShader blue_green_sampler.frag.iplr'));
79+
} finally {
80+
fragmentShader.dispose();
81+
}
82+
});
83+
6584
test('Disposed FragmentShader on Paint', () async {
6685
final FragmentProgram program = await FragmentProgram.fromAsset(
6786
'blue_green_sampler.frag.iplr',

0 commit comments

Comments
 (0)