From ee049b088fca4737d9b2ae3bfbd74bc31e86d9f9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Nov 2022 21:14:11 -0800 Subject: [PATCH 01/25] [Impeller] null check sampler and assert in Paint --- .../display_list/display_list_dispatcher.cc | 4 +++ lib/ui/dart_ui.cc | 1 + lib/ui/painting.dart | 31 +++++++++++++++++-- lib/ui/painting/fragment_shader.cc | 9 ++++++ lib/ui/painting/fragment_shader.h | 2 ++ testing/dart/fragment_shader_test.dart | 11 +++++++ 6 files changed, 55 insertions(+), 3 deletions(-) diff --git a/impeller/display_list/display_list_dispatcher.cc b/impeller/display_list/display_list_dispatcher.cc index 3e555aeaf418f..7143ef7a2d27b 100644 --- a/impeller/display_list/display_list_dispatcher.cc +++ b/impeller/display_list/display_list_dispatcher.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include "display_list/display_list_blend_mode.h" #include "display_list/display_list_color_filter.h" @@ -442,6 +443,9 @@ void DisplayListDispatcher::setColorSource( std::vector texture_inputs; for (auto& sampler : samplers) { + if (sampler == nullptr) { + return; + } auto* image = sampler->asImage(); if (!sampler->asImage()) { UNIMPLEMENTED; diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 595988a19bb66..5199821389311 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -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) \ diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 7dbb9b62d6beb..c6da131c87acf 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1409,6 +1409,10 @@ class Paint { ); return true; }()); + assert(() { + value?._validate(); + return true; + }()); _ensureObjectsInitialized()[_kShaderIndex] = value; } @@ -3732,6 +3736,9 @@ class Shader extends NativeFieldWrapperClass1 { return disposed; } + // Validate the shader correctness. + void _validate() { } + /// Release the resources used by this object. The object is no longer usable /// after this method is called. /// @@ -4151,10 +4158,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]. @@ -4222,7 +4235,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]. @@ -4239,7 +4252,7 @@ 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, @@ -4247,6 +4260,8 @@ class FragmentShader extends Shader { ); } + String? _debugName; + static final Float32List _kEmptyFloat32List = Float32List(0); late Float32List _floats; @@ -4278,12 +4293,22 @@ class FragmentShader extends Shader { _dispose(); } + @override + void _validate() { + if (!_validateSamplers()) { + throw Exception('Invalid FragmentShader ${_debugName ?? ''}: missing sampler'); + } + } + @FfiNative('ReusableFragmentShader::Create') external Float32List _constructor(FragmentProgram program, int floatUniforms, int samplerUniforms); @FfiNative, Handle, Handle)>('ReusableFragmentShader::SetSampler') external void _setSampler(int index, ImageShader sampler); + @FfiNative('ReusableFragmentShader::ValidateSamplers') + external bool _validateSamplers(); + @FfiNative)>('ReusableFragmentShader::Dispose') external void _dispose(); } diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index f1243ea971d6d..090b854675193 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -53,6 +53,15 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, float_count); } +bool ReusableFragmentShader::ValidateSamplers(Dart_Handle wrapper) { + 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::FromDart(index_handle); diff --git a/lib/ui/painting/fragment_shader.h b/lib/ui/painting/fragment_shader.h index da7af7dfb1ea6..5c8c876c8c4cb 100644 --- a/lib/ui/painting/fragment_shader.h +++ b/lib/ui/painting/fragment_shader.h @@ -36,6 +36,8 @@ class ReusableFragmentShader : public Shader { void SetSampler(Dart_Handle index, Dart_Handle sampler); + bool ValidateSamplers(Dart_Handle wrapper); + void Dispose(); // |Shader| diff --git a/testing/dart/fragment_shader_test.dart b/testing/dart/fragment_shader_test.dart index ed12e9939ed9e..cf39e4acde979 100644 --- a/testing/dart/fragment_shader_test.dart +++ b/testing/dart/fragment_shader_test.dart @@ -62,6 +62,17 @@ void main() async { } }); + test('FragmentShader with sampler asserts if sampler is missing when assigned to paint', () async { + final FragmentProgram program = await FragmentProgram.fromAsset( + 'blue_green_sampler.frag.iplr', + ); + final FragmentShader fragmentShader = program.fragmentShader(); + + expect(() => Paint()..shader = fragmentShader, throwsA(isInstanceOf())); + + fragmentShader.dispose(); + }); + test('Disposed FragmentShader on Paint', () async { final FragmentProgram program = await FragmentProgram.fromAsset( 'blue_green_sampler.frag.iplr', From 6246d781bb0466b866162f0262f0704a75afbaac Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Nov 2022 21:15:26 -0800 Subject: [PATCH 02/25] ++ --- impeller/display_list/display_list_dispatcher.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/impeller/display_list/display_list_dispatcher.cc b/impeller/display_list/display_list_dispatcher.cc index 7143ef7a2d27b..80678248e858b 100644 --- a/impeller/display_list/display_list_dispatcher.cc +++ b/impeller/display_list/display_list_dispatcher.cc @@ -11,7 +11,6 @@ #include #include #include -#include #include "display_list/display_list_blend_mode.h" #include "display_list/display_list_color_filter.h" From 777967565f08c0c65f65c70ba246a0dde3f392d0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Nov 2022 21:33:59 -0800 Subject: [PATCH 03/25] ++ --- display_list/display_list_color_source.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/display_list/display_list_color_source.h b/display_list/display_list_color_source.h index 95892f41c053d..1e5f545554a72 100644 --- a/display_list/display_list_color_source.h +++ b/display_list/display_list_color_source.h @@ -705,7 +705,11 @@ class DlRuntimeEffectColorSource final : public DlColorSource { } std::vector> 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>(uniform_data_); From 0da465a5ef45e85eda89b6ed3743aad64f80c5e9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Nov 2022 21:37:03 -0800 Subject: [PATCH 04/25] ++ --- display_list/display_list_color_source_unittests.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/display_list/display_list_color_source_unittests.cc b/display_list/display_list_color_source_unittests.cc index 8e9044b3223b2..432cb09e52b9e 100644 --- a/display_list/display_list_color_source_unittests.cc +++ b/display_list/display_list_color_source_unittests.cc @@ -974,5 +974,14 @@ TEST(DisplayListColorSource, RuntimeEffect) { TestNotEquals(source2, source3, "SkRuntimeEffect differs"); } +TEST(DisplayListColorSource, RuntimeEffectWithNullSampler) { + std::shared_ptr source1 = + DlColorSource::MakeRuntimeEffect( + kTestRuntimeEffect1, {nullptr}, + std::make_shared>()); + + ASSERT_EQ(source3->skia_object(), nullptr); +} + } // namespace testing } // namespace flutter From 4646b10afc29eaacc922f34b99e4b3379e27c335 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Nov 2022 22:14:22 -0800 Subject: [PATCH 05/25] ++ --- display_list/display_list_color_source_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/display_list/display_list_color_source_unittests.cc b/display_list/display_list_color_source_unittests.cc index 432cb09e52b9e..5c9a62abd8697 100644 --- a/display_list/display_list_color_source_unittests.cc +++ b/display_list/display_list_color_source_unittests.cc @@ -980,7 +980,7 @@ TEST(DisplayListColorSource, RuntimeEffectWithNullSampler) { kTestRuntimeEffect1, {nullptr}, std::make_shared>()); - ASSERT_EQ(source3->skia_object(), nullptr); + ASSERT_EQ(source1->skia_object(), nullptr); } } // namespace testing From 4b03d10edc87ab462ef79ef52cade60fc7057b7b Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 9 Nov 2022 22:25:26 -0800 Subject: [PATCH 06/25] Update fragment_shader_test.dart --- testing/dart/fragment_shader_test.dart | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/testing/dart/fragment_shader_test.dart b/testing/dart/fragment_shader_test.dart index cf39e4acde979..cae5c94ca7e82 100644 --- a/testing/dart/fragment_shader_test.dart +++ b/testing/dart/fragment_shader_test.dart @@ -68,9 +68,14 @@ void main() async { ); final FragmentShader fragmentShader = program.fragmentShader(); - expect(() => Paint()..shader = fragmentShader, throwsA(isInstanceOf())); - - fragmentShader.dispose(); + try { + Paint()..shader = fragmentShader; + fail('Expected to throw'); + } catch (err) { + expect(err, contains('Invalid FragmentShader blue_green_sampler.frag.iplr')); + } finally { + fragmentShader.dispose(); + } }); test('Disposed FragmentShader on Paint', () async { From a54ccc81c3896af488e7bc1fae6c5e964da440ec Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Nov 2022 22:45:20 -0800 Subject: [PATCH 07/25] ++ --- testing/dart/fragment_shader_test.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/testing/dart/fragment_shader_test.dart b/testing/dart/fragment_shader_test.dart index cae5c94ca7e82..eca105ec9d394 100644 --- a/testing/dart/fragment_shader_test.dart +++ b/testing/dart/fragment_shader_test.dart @@ -63,16 +63,19 @@ 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; + Paint().shader = fragmentShader; fail('Expected to throw'); } catch (err) { - expect(err, contains('Invalid FragmentShader blue_green_sampler.frag.iplr')); + expect(err.toString(), contains('Invalid FragmentShader blue_green_sampler.frag.iplr')); } finally { fragmentShader.dispose(); } From ff35ea497b3a2eef34ba82f800dbe8f0a951a66b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 09:25:06 -0800 Subject: [PATCH 08/25] TESTING --- lib/ui/painting.dart | 2 +- lib/ui/painting/fragment_shader.cc | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index c6da131c87acf..bc16ca3e3b030 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4260,7 +4260,7 @@ class FragmentShader extends Shader { ); } - String? _debugName; + final String? _debugName; static final Float32List _kEmptyFloat32List = Float32List(0); diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 090b854675193..b4ff8259daecd 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include "flutter/lib/ui/painting/fragment_shader.h" @@ -56,9 +57,11 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, bool ReusableFragmentShader::ValidateSamplers(Dart_Handle wrapper) { for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { + std::cerr << "Missing Sampler!" << std::endl; return false; } } + std::cerr << "No Missing Samplers" << std::endl; return true; } From 53ddf6122df5f0d721507d32f054648b8e7a2f6d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 09:39:37 -0800 Subject: [PATCH 09/25] ++ --- lib/ui/painting/fragment_shader.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index b4ff8259daecd..e6d2548081f72 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -5,7 +5,6 @@ #include #include #include -#include #include "flutter/lib/ui/painting/fragment_shader.h" From bbf92cb83653c114e4cf613f2182d723ee7e1fde Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 09:57:54 -0800 Subject: [PATCH 10/25] TESTING --- lib/ui/painting/fragment_program.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ui/painting/fragment_program.cc b/lib/ui/painting/fragment_program.cc index 3e814e1753dfe..c0cffbba09775 100644 --- a/lib/ui/painting/fragment_program.cc +++ b/lib/ui/painting/fragment_program.cc @@ -67,6 +67,8 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { // SkString makes a copy. SkRuntimeEffect::Result result = SkRuntimeEffect::MakeForShader(SkString(sksl, code_size)); + std::cerr << "Compiled asset " << asset_name << std::endl; + std::cerr << sksl << std::endl; if (result.effect == nullptr) { return std::string("Invalid SkSL:\n") + sksl + std::string("\nSkSL Error:\n") + result.errorText.c_str(); From 2b304e462f37752f70182e90f5512d00d8b5bf55 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 10:52:15 -0800 Subject: [PATCH 11/25] ++ --- lib/ui/painting/fragment_program.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/fragment_program.cc b/lib/ui/painting/fragment_program.cc index c0cffbba09775..132d3b29059bd 100644 --- a/lib/ui/painting/fragment_program.cc +++ b/lib/ui/painting/fragment_program.cc @@ -67,8 +67,6 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { // SkString makes a copy. SkRuntimeEffect::Result result = SkRuntimeEffect::MakeForShader(SkString(sksl, code_size)); - std::cerr << "Compiled asset " << asset_name << std::endl; - std::cerr << sksl << std::endl; if (result.effect == nullptr) { return std::string("Invalid SkSL:\n") + sksl + std::string("\nSkSL Error:\n") + result.errorText.c_str(); @@ -80,7 +78,8 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { if (Dart_IsError(ths)) { Dart_PropagateError(ths); } - + std::cerr << asset_name << std::endl; + std::cerr << "Setting sampled_image_count " << sampled_image_count << std::endl; Dart_Handle result = Dart_SetField(ths, tonic::ToDart("_samplerCount"), Dart_NewInteger(sampled_image_count)); if (Dart_IsError(result)) { From ce8daa578d19c23839407c641a1755b002a54449 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 11:03:39 -0800 Subject: [PATCH 12/25] ++ --- lib/ui/painting/fragment_program.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ui/painting/fragment_program.cc b/lib/ui/painting/fragment_program.cc index 132d3b29059bd..6807c9c5fcebe 100644 --- a/lib/ui/painting/fragment_program.cc +++ b/lib/ui/painting/fragment_program.cc @@ -79,7 +79,8 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { Dart_PropagateError(ths); } std::cerr << asset_name << std::endl; - std::cerr << "Setting sampled_image_count " << sampled_image_count << std::endl; + std::cerr << "Setting sampled_image_count " << sampled_image_count + << std::endl; Dart_Handle result = Dart_SetField(ths, tonic::ToDart("_samplerCount"), Dart_NewInteger(sampled_image_count)); if (Dart_IsError(result)) { From ca9fdf32cda737ba1f1f21246426f30e182f6e82 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 11:16:00 -0800 Subject: [PATCH 13/25] ++ --- lib/ui/painting/fragment_shader.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index e6d2548081f72..70973608fbb4a 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -54,6 +54,7 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, } bool ReusableFragmentShader::ValidateSamplers(Dart_Handle wrapper) { + std::cerr << "sampler_count: " << sampler_count << std::endl; for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { std::cerr << "Missing Sampler!" << std::endl; From 83190233ca208404cfb0f71ca89e06b000e690fb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 11:19:19 -0800 Subject: [PATCH 14/25] ++ --- lib/ui/painting/fragment_shader.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 70973608fbb4a..4ab8c6830d522 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -30,7 +30,9 @@ ReusableFragmentShader::ReusableFragmentShader( uniform_data_(SkData::MakeUninitialized( (float_count + 2 * sampler_count) * sizeof(float))), samplers_(sampler_count), - float_count_(float_count) {} + float_count_(float_count) { + std::cerr << "sampler_count: " << sampler_count << std::endl; +} Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, Dart_Handle program, @@ -54,7 +56,6 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, } bool ReusableFragmentShader::ValidateSamplers(Dart_Handle wrapper) { - std::cerr << "sampler_count: " << sampler_count << std::endl; for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { std::cerr << "Missing Sampler!" << std::endl; From 9189308b2a7e458f2c073991a73937f2f90e44fb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 11:39:19 -0800 Subject: [PATCH 15/25] ++ --- lib/ui/painting/fragment_shader.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 4ab8c6830d522..27329f8d0800f 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -31,6 +31,7 @@ ReusableFragmentShader::ReusableFragmentShader( (float_count + 2 * sampler_count) * sizeof(float))), samplers_(sampler_count), float_count_(float_count) { + std::cerr << ((void*) this) << std::endl; std::cerr << "sampler_count: " << sampler_count << std::endl; } @@ -56,6 +57,7 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, } bool ReusableFragmentShader::ValidateSamplers(Dart_Handle wrapper) { + std::cerr << ((void*) this) << std::endl; for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { std::cerr << "Missing Sampler!" << std::endl; From ff9a0fc18b3a1e5df8d1c17ed9a25c00e48850b4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 11:42:42 -0800 Subject: [PATCH 16/25] ++ --- lib/ui/painting/fragment_shader.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 27329f8d0800f..f0d37a7f9447e 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -31,7 +31,7 @@ ReusableFragmentShader::ReusableFragmentShader( (float_count + 2 * sampler_count) * sizeof(float))), samplers_(sampler_count), float_count_(float_count) { - std::cerr << ((void*) this) << std::endl; + std::cerr << ((void*)this) << std::endl; std::cerr << "sampler_count: " << sampler_count << std::endl; } @@ -57,7 +57,7 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, } bool ReusableFragmentShader::ValidateSamplers(Dart_Handle wrapper) { - std::cerr << ((void*) this) << std::endl; + std::cerr << ((void*)this) << std::endl; for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { std::cerr << "Missing Sampler!" << std::endl; From 909daf3d274b09de2aca6e180f2ab8d28ae1314d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 12:27:07 -0800 Subject: [PATCH 17/25] ++ --- lib/ui/dart_ui.cc | 2 +- lib/ui/painting/fragment_shader.cc | 2 +- lib/ui/painting/fragment_shader.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 5199821389311..a04ad7de25d15 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -175,7 +175,7 @@ typedef CanvasPath Path; V(FragmentProgram, initFromAsset, 2) \ V(ReusableFragmentShader, Dispose, 1) \ V(ReusableFragmentShader, SetSampler, 3) \ - V(ReusableFragmentShader, ValidateSamplers, 1) \ + V(ReusableFragmentShader, ValidateSamplers, 0) \ V(Gradient, initLinear, 6) \ V(Gradient, initRadial, 8) \ V(Gradient, initSweep, 9) \ diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index f0d37a7f9447e..653c041913344 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -56,7 +56,7 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, float_count); } -bool ReusableFragmentShader::ValidateSamplers(Dart_Handle wrapper) { +bool ReusableFragmentShader::ValidateSamplers() { std::cerr << ((void*)this) << std::endl; for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { diff --git a/lib/ui/painting/fragment_shader.h b/lib/ui/painting/fragment_shader.h index 5c8c876c8c4cb..5b948e80dea0c 100644 --- a/lib/ui/painting/fragment_shader.h +++ b/lib/ui/painting/fragment_shader.h @@ -36,7 +36,7 @@ class ReusableFragmentShader : public Shader { void SetSampler(Dart_Handle index, Dart_Handle sampler); - bool ValidateSamplers(Dart_Handle wrapper); + bool ValidateSamplers(); void Dispose(); From 3bff2a9d9a93f07fb547124d8ce6de35b678e80d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 12:41:35 -0800 Subject: [PATCH 18/25] ++ --- lib/ui/dart_ui.cc | 2 +- lib/ui/painting.dart | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index a04ad7de25d15..5199821389311 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -175,7 +175,7 @@ typedef CanvasPath Path; V(FragmentProgram, initFromAsset, 2) \ V(ReusableFragmentShader, Dispose, 1) \ V(ReusableFragmentShader, SetSampler, 3) \ - V(ReusableFragmentShader, ValidateSamplers, 0) \ + V(ReusableFragmentShader, ValidateSamplers, 1) \ V(Gradient, initLinear, 6) \ V(Gradient, initRadial, 8) \ V(Gradient, initSweep, 9) \ diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index bc16ca3e3b030..00c0cd4b2072c 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4263,8 +4263,7 @@ class FragmentShader extends Shader { final String? _debugName; static final Float32List _kEmptyFloat32List = Float32List(0); - - late Float32List _floats; + Float32List _floats = _kEmptyFloat32List; /// Sets the float uniform at [index] to [value]. void setFloat(int index, double value) { @@ -4295,6 +4294,7 @@ class FragmentShader extends Shader { @override void _validate() { + assert(!debugDisposed, 'Tried to accesss uniforms on a disposed Shader: $this'); if (!_validateSamplers()) { throw Exception('Invalid FragmentShader ${_debugName ?? ''}: missing sampler'); } From ce218a1e2e9f404e523e454da5fa08e15eafd1ff Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 13:03:28 -0800 Subject: [PATCH 19/25] ++ --- lib/ui/dart_ui.cc | 2 +- lib/ui/painting.dart | 5 +++-- lib/ui/painting/fragment_program.cc | 4 ---- lib/ui/painting/fragment_shader.cc | 20 +++++++++----------- lib/ui/painting/fragment_shader.h | 7 +++++-- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 5199821389311..1385dcdd5d5b2 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -68,7 +68,7 @@ typedef CanvasPath Path; V(Canvas::Create, 6) \ V(ColorFilter::Create, 1) \ V(FragmentProgram::Create, 1) \ - V(ReusableFragmentShader::Create, 4) \ + V(ReusableFragmentShader::Create, 5) \ V(Gradient::Create, 1) \ V(ImageFilter::Create, 1) \ V(ImageShader::Create, 1) \ diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 00c0cd4b2072c..c02d58eb18bc1 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4257,6 +4257,7 @@ class FragmentShader extends Shader { program, program._uniformFloatCount, program._samplerCount, + debugName ?? '', ); } @@ -4300,8 +4301,8 @@ class FragmentShader extends Shader { } } - @FfiNative('ReusableFragmentShader::Create') - external Float32List _constructor(FragmentProgram program, int floatUniforms, int samplerUniforms); + @FfiNative('ReusableFragmentShader::Create') + external Float32List _constructor(FragmentProgram program, int floatUniforms, int samplerUniforms, String? debugName); @FfiNative, Handle, Handle)>('ReusableFragmentShader::SetSampler') external void _setSampler(int index, ImageShader sampler); diff --git a/lib/ui/painting/fragment_program.cc b/lib/ui/painting/fragment_program.cc index 6807c9c5fcebe..97536caf75215 100644 --- a/lib/ui/painting/fragment_program.cc +++ b/lib/ui/painting/fragment_program.cc @@ -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 #include #include @@ -78,9 +77,6 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { if (Dart_IsError(ths)) { Dart_PropagateError(ths); } - std::cerr << asset_name << std::endl; - std::cerr << "Setting sampled_image_count " << sampled_image_count - << std::endl; Dart_Handle result = Dart_SetField(ths, tonic::ToDart("_samplerCount"), Dart_NewInteger(sampled_image_count)); if (Dart_IsError(result)) { diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 653c041913344..2fb1046fc289a 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include #include #include +#include #include "flutter/lib/ui/painting/fragment_shader.h" @@ -25,20 +25,20 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, ReusableFragmentShader); ReusableFragmentShader::ReusableFragmentShader( fml::RefPtr program, uint64_t float_count, - uint64_t sampler_count) + uint64_t sampler_count, + const std::string& debug_name) : program_(std::move(program)), uniform_data_(SkData::MakeUninitialized( (float_count + 2 * sampler_count) * sizeof(float))), samplers_(sampler_count), - float_count_(float_count) { - std::cerr << ((void*)this) << std::endl; - std::cerr << "sampler_count: " << sampler_count << std::endl; -} + float_count_(float_count), + debug_name_(std::move(debug_name)) {} Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, Dart_Handle program, Dart_Handle float_count_handle, - Dart_Handle sampler_count_handle) { + Dart_Handle sampler_count_handle, + const std::string& debug_name) { auto* fragment_program = tonic::DartConverter::FromDart(program); uint64_t float_count = @@ -47,7 +47,7 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, tonic::DartConverter::FromDart(sampler_count_handle); auto res = fml::MakeRefCounted( - fml::Ref(fragment_program), float_count, sampler_count); + fml::Ref(fragment_program), float_count, sampler_count, debug_name); res->AssociateWithDartWrapper(wrapper); void* raw_uniform_data = @@ -57,14 +57,12 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, } bool ReusableFragmentShader::ValidateSamplers() { - std::cerr << ((void*)this) << std::endl; for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { - std::cerr << "Missing Sampler!" << std::endl; + std::cerr << "Missing Sampler!: " << debug_name_ << std::endl; return false; } } - std::cerr << "No Missing Samplers" << std::endl; return true; } diff --git a/lib/ui/painting/fragment_shader.h b/lib/ui/painting/fragment_shader.h index 5b948e80dea0c..d574f525c827a 100644 --- a/lib/ui/painting/fragment_shader.h +++ b/lib/ui/painting/fragment_shader.h @@ -32,7 +32,8 @@ class ReusableFragmentShader : public Shader { static Dart_Handle Create(Dart_Handle wrapper, Dart_Handle program, Dart_Handle float_count, - Dart_Handle sampler_count); + Dart_Handle sampler_count, + const std::string& debug_handle); void SetSampler(Dart_Handle index, Dart_Handle sampler); @@ -46,12 +47,14 @@ class ReusableFragmentShader : public Shader { private: ReusableFragmentShader(fml::RefPtr program, uint64_t float_count, - uint64_t sampler_count); + uint64_t sampler_count, + const std::string& debug_name); fml::RefPtr program_; sk_sp uniform_data_; std::vector> samplers_; size_t float_count_; + std::string debug_name_; }; } // namespace flutter From 8531bd80a96e95fc45d0573fa79d354a61597598 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 13:04:33 -0800 Subject: [PATCH 20/25] ++ --- lib/ui/painting/fragment_shader.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 2fb1046fc289a..3f51df34f7b03 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include #include -#include #include "flutter/lib/ui/painting/fragment_shader.h" From 621f696d8831ba88d597d12869bcbc0090aec59a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 13:44:03 -0800 Subject: [PATCH 21/25] ++ --- lib/ui/painting.dart | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index c02d58eb18bc1..3227030a4e4b6 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1410,7 +1410,11 @@ class Paint { return true; }()); assert(() { - value?._validate(); + if (value is FragmentShader) { + if (!value._validateSamplers()) { + throw Exception('Invalid FragmentShader ${value._debugName ?? ''}: missing sampler'); + } + } return true; }()); _ensureObjectsInitialized()[_kShaderIndex] = value; @@ -3736,9 +3740,6 @@ class Shader extends NativeFieldWrapperClass1 { return disposed; } - // Validate the shader correctness. - void _validate() { } - /// Release the resources used by this object. The object is no longer usable /// after this method is called. /// @@ -4293,14 +4294,6 @@ class FragmentShader extends Shader { _dispose(); } - @override - void _validate() { - assert(!debugDisposed, 'Tried to accesss uniforms on a disposed Shader: $this'); - if (!_validateSamplers()) { - throw Exception('Invalid FragmentShader ${_debugName ?? ''}: missing sampler'); - } - } - @FfiNative('ReusableFragmentShader::Create') external Float32List _constructor(FragmentProgram program, int floatUniforms, int samplerUniforms, String? debugName); From f8268458fc79602a4736b97da1940db9a577116d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 13:48:54 -0800 Subject: [PATCH 22/25] ++ --- lib/ui/painting.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 3227030a4e4b6..bacd37e1bf851 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4300,7 +4300,7 @@ class FragmentShader extends Shader { @FfiNative, Handle, Handle)>('ReusableFragmentShader::SetSampler') external void _setSampler(int index, ImageShader sampler); - @FfiNative('ReusableFragmentShader::ValidateSamplers') + @FfiNative)>('ReusableFragmentShader::ValidateSamplers') external bool _validateSamplers(); @FfiNative)>('ReusableFragmentShader::Dispose') From ed83735f3f397af9634ba8193082319996051375 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 14:57:19 -0800 Subject: [PATCH 23/25] remove debugging code --- lib/ui/dart_ui.cc | 2 +- lib/ui/painting.dart | 5 ++--- lib/ui/painting/fragment_shader.cc | 13 ++++--------- lib/ui/painting/fragment_shader.h | 4 +--- 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 1385dcdd5d5b2..5199821389311 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -68,7 +68,7 @@ typedef CanvasPath Path; V(Canvas::Create, 6) \ V(ColorFilter::Create, 1) \ V(FragmentProgram::Create, 1) \ - V(ReusableFragmentShader::Create, 5) \ + V(ReusableFragmentShader::Create, 4) \ V(Gradient::Create, 1) \ V(ImageFilter::Create, 1) \ V(ImageShader::Create, 1) \ diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index bacd37e1bf851..5e96751c289ff 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4258,7 +4258,6 @@ class FragmentShader extends Shader { program, program._uniformFloatCount, program._samplerCount, - debugName ?? '', ); } @@ -4294,8 +4293,8 @@ class FragmentShader extends Shader { _dispose(); } - @FfiNative('ReusableFragmentShader::Create') - external Float32List _constructor(FragmentProgram program, int floatUniforms, int samplerUniforms, String? debugName); + @FfiNative('ReusableFragmentShader::Create') + external Float32List _constructor(FragmentProgram program, int floatUniforms, int samplerUniforms); @FfiNative, Handle, Handle)>('ReusableFragmentShader::SetSampler') external void _setSampler(int index, ImageShader sampler); diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 3f51df34f7b03..4021c51658676 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -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 #include #include @@ -25,20 +24,17 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, ReusableFragmentShader); ReusableFragmentShader::ReusableFragmentShader( fml::RefPtr program, uint64_t float_count, - uint64_t sampler_count, - const std::string& debug_name) + uint64_t sampler_count : program_(std::move(program)), uniform_data_(SkData::MakeUninitialized( (float_count + 2 * sampler_count) * sizeof(float))), samplers_(sampler_count), - float_count_(float_count), - debug_name_(std::move(debug_name)) {} + float_count_(float_count) {} Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, Dart_Handle program, Dart_Handle float_count_handle, - Dart_Handle sampler_count_handle, - const std::string& debug_name) { + Dart_Handle sampler_count_handle) { auto* fragment_program = tonic::DartConverter::FromDart(program); uint64_t float_count = @@ -47,7 +43,7 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, tonic::DartConverter::FromDart(sampler_count_handle); auto res = fml::MakeRefCounted( - fml::Ref(fragment_program), float_count, sampler_count, debug_name); + fml::Ref(fragment_program), float_count, sampler_count); res->AssociateWithDartWrapper(wrapper); void* raw_uniform_data = @@ -59,7 +55,6 @@ Dart_Handle ReusableFragmentShader::Create(Dart_Handle wrapper, bool ReusableFragmentShader::ValidateSamplers() { for (auto i = 0u; i < samplers_.size(); i += 1) { if (samplers_[i] == nullptr) { - std::cerr << "Missing Sampler!: " << debug_name_ << std::endl; return false; } } diff --git a/lib/ui/painting/fragment_shader.h b/lib/ui/painting/fragment_shader.h index d574f525c827a..2dc6f1fb451e6 100644 --- a/lib/ui/painting/fragment_shader.h +++ b/lib/ui/painting/fragment_shader.h @@ -32,8 +32,7 @@ class ReusableFragmentShader : public Shader { static Dart_Handle Create(Dart_Handle wrapper, Dart_Handle program, Dart_Handle float_count, - Dart_Handle sampler_count, - const std::string& debug_handle); + Dart_Handle sampler_count); void SetSampler(Dart_Handle index, Dart_Handle sampler); @@ -54,7 +53,6 @@ class ReusableFragmentShader : public Shader { sk_sp uniform_data_; std::vector> samplers_; size_t float_count_; - std::string debug_name_; }; } // namespace flutter From fb56d59f9f2cad695d0a189d84d2dd94653214f8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 15:29:43 -0800 Subject: [PATCH 24/25] ++ --- lib/ui/painting/fragment_shader.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting/fragment_shader.cc b/lib/ui/painting/fragment_shader.cc index 4021c51658676..38f62ef3d507e 100644 --- a/lib/ui/painting/fragment_shader.cc +++ b/lib/ui/painting/fragment_shader.cc @@ -24,7 +24,7 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, ReusableFragmentShader); ReusableFragmentShader::ReusableFragmentShader( fml::RefPtr program, uint64_t float_count, - uint64_t sampler_count + uint64_t sampler_count) : program_(std::move(program)), uniform_data_(SkData::MakeUninitialized( (float_count + 2 * sampler_count) * sizeof(float))), From f70bac37b8ebf0a30b585d7f8043c973de7e38bc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Nov 2022 16:28:19 -0800 Subject: [PATCH 25/25] ++ --- lib/ui/painting/fragment_shader.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ui/painting/fragment_shader.h b/lib/ui/painting/fragment_shader.h index 2dc6f1fb451e6..5b948e80dea0c 100644 --- a/lib/ui/painting/fragment_shader.h +++ b/lib/ui/painting/fragment_shader.h @@ -46,8 +46,7 @@ class ReusableFragmentShader : public Shader { private: ReusableFragmentShader(fml::RefPtr program, uint64_t float_count, - uint64_t sampler_count, - const std::string& debug_name); + uint64_t sampler_count); fml::RefPtr program_; sk_sp uniform_data_;