diff --git a/include/sksl/DSLCore.h b/include/sksl/DSLCore.h index 833bfd390a..162007b320 100644 --- a/include/sksl/DSLCore.h +++ b/include/sksl/DSLCore.h @@ -23,6 +23,7 @@ namespace SkSL { class Compiler; +struct Program; struct ProgramSettings; namespace dsl { @@ -47,6 +48,11 @@ void Start(SkSL::Compiler* compiler, SkSL::ProgramKind kind, const SkSL::Program */ void End(); +/** + * Returns all global elements (functions and global variables) as a self-contained Program. + */ +std::unique_ptr ReleaseProgram(); + /** * Installs an ErrorHandler which will be notified of any errors that occur during DSL calls. If * no ErrorHandler is installed, any errors will be fatal. @@ -68,7 +74,7 @@ DSLStatement Break(); DSLStatement Continue(); /** - * Creates a variable declaration statement. + * Creates a local variable declaration statement. */ DSLStatement Declare(DSLVar& var, PositionInfo pos = PositionInfo()); diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index ecdcd10f6e..a4b9297f87 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -487,8 +487,8 @@ std::unique_ptr Compiler::convertProgram( dsl::Start(this, kind, settings); IRGenerator::IRBundle ir = fIRGenerator->convertProgram(baseModule, /*isBuiltinCode=*/false, textPtr->c_str(), textPtr->size()); - // Ideally, we would just use DSLWriter::ReleaseProgram and not have to do any manual mucking - // about with the memory pool, but we've got some impedance mismatches to solve first + // Ideally, we would just use dsl::ReleaseProgram and not have to do any manual mucking about + // with the memory pool, but we've got some impedance mismatches to solve first Pool* memoryPool = dsl::DSLWriter::MemoryPool().get(); auto program = std::make_unique(std::move(textPtr), std::move(dsl::DSLWriter::GetProgramConfig()), diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h index 565c7a68f4..bc5b14c4be 100644 --- a/src/sksl/SkSLCompiler.h +++ b/src/sksl/SkSLCompiler.h @@ -44,6 +44,7 @@ class SkSLCompileBench; namespace SkSL { namespace dsl { + class DSLCore; class DSLWriter; } @@ -265,6 +266,7 @@ private: friend class AutoSource; friend class ::SkSLCompileBench; + friend class dsl::DSLCore; friend class dsl::DSLWriter; }; diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp index 24fee26f03..239bca16a3 100644 --- a/src/sksl/dsl/DSLCore.cpp +++ b/src/sksl/dsl/DSLCore.cpp @@ -53,6 +53,41 @@ void SetErrorHandler(ErrorHandler* errorHandler) { class DSLCore { public: + static std::unique_ptr ReleaseProgram() { + DSLWriter& instance = DSLWriter::Instance(); + SkSL::IRGenerator& ir = DSLWriter::IRGenerator(); + IRGenerator::IRBundle bundle = ir.finish(); + Pool* pool = DSLWriter::Instance().fPool.get(); + auto result = std::make_unique(/*source=*/nullptr, + std::move(instance.fConfig), + DSLWriter::Instance().fCompiler->fContext, + std::move(bundle.fElements), + std::move(bundle.fSharedElements), + std::move(instance.fModifiersPool), + std::move(bundle.fSymbolTable), + std::move(instance.fPool), + bundle.fInputs); + bool success = false; + if (DSLWriter::Compiler().errorCount() || DSLWriter::Instance().fEncounteredErrors) { + // Make sure that if we encountered any compiler errors, we reported them through the + // DSL error handling side of things. + SkASSERT(!DSLWriter::Compiler().errorCount() || + DSLWriter::Instance().fEncounteredErrors); + // Do not return programs that failed to compile. + } else if (!DSLWriter::Compiler().optimize(*result)) { + // Do not return programs that failed to optimize. + } else { + // We have a successful program! + success = true; + } + if (pool) { + pool->detachFromThread(); + } + SkASSERT(DSLWriter::ProgramElements().empty()); + SkASSERT(!DSLWriter::SymbolTable()); + return success ? std::move(result) : nullptr; + } + static DSLVar sk_FragColor() { return DSLVar("sk_FragColor"); } @@ -208,6 +243,10 @@ public: } }; +std::unique_ptr ReleaseProgram() { + return DSLCore::ReleaseProgram(); +} + DSLVar sk_FragColor() { return DSLCore::sk_FragColor(); } diff --git a/src/sksl/dsl/DSLRuntimeEffects.cpp b/src/sksl/dsl/DSLRuntimeEffects.cpp index 59424fb71b..a731427246 100644 --- a/src/sksl/dsl/DSLRuntimeEffects.cpp +++ b/src/sksl/dsl/DSLRuntimeEffects.cpp @@ -29,13 +29,13 @@ void StartRuntimeShader(SkSL::Compiler* compiler) { } sk_sp EndRuntimeShader() { - std::unique_ptr program = DSLWriter::ReleaseProgram(); - auto result = SkRuntimeEffect::MakeForShader(std::move(program)); - // TODO(skbug.com/11862): propagate errors properly - SkASSERTF(result.effect, "%s\n", result.errorText.c_str()); - SkSL::ProgramSettings& settings = DSLWriter::IRGenerator().fContext.fConfig->fSettings; - settings.fInlineThreshold = SkSL::kDefaultInlineThreshold; - settings.fAllowNarrowingConversions = false; + std::unique_ptr program = ReleaseProgram(); + SkRuntimeEffect::Result result; + if (program) { + result = SkRuntimeEffect::MakeForShader(std::move(program)); + // TODO(skbug.com/11862): propagate errors properly + SkASSERTF(result.effect, "%s\n", result.errorText.c_str()); + } End(); return result.effect; } diff --git a/src/sksl/dsl/priv/DSLWriter.cpp b/src/sksl/dsl/priv/DSLWriter.cpp index 7a6e0a91e2..a2e0394273 100644 --- a/src/sksl/dsl/priv/DSLWriter.cpp +++ b/src/sksl/dsl/priv/DSLWriter.cpp @@ -208,6 +208,7 @@ DSLPossibleStatement DSLWriter::ConvertSwitch(std::unique_ptr value, } void DSLWriter::ReportError(const char* msg, PositionInfo* info) { + Instance().fEncounteredErrors = true; if (info && !info->file_name()) { info = nullptr; } @@ -264,28 +265,6 @@ void DSLWriter::MarkDeclared(DSLVar& var) { var.fDeclared = true; } -std::unique_ptr DSLWriter::ReleaseProgram() { - DSLWriter& instance = Instance(); - SkSL::IRGenerator& ir = IRGenerator(); - IRGenerator::IRBundle bundle = ir.finish(); - Pool* pool = Instance().fPool.get(); - auto result = std::make_unique(/*source=*/nullptr, - std::move(instance.fConfig), - Compiler().fContext, - std::move(bundle.fElements), - std::move(bundle.fSharedElements), - std::move(instance.fModifiersPool), - std::move(bundle.fSymbolTable), - std::move(instance.fPool), - bundle.fInputs); - if (pool) { - pool->detachFromThread(); - } - SkASSERT(ProgramElements().empty()); - SkASSERT(!SymbolTable()); - return result; -} - #if SKSL_USE_THREAD_LOCAL thread_local DSLWriter* instance = nullptr; diff --git a/src/sksl/dsl/priv/DSLWriter.h b/src/sksl/dsl/priv/DSLWriter.h index 728309524f..398cfcea0d 100644 --- a/src/sksl/dsl/priv/DSLWriter.h +++ b/src/sksl/dsl/priv/DSLWriter.h @@ -256,8 +256,6 @@ public: return Instance().fSettings.fDSLMarkVarsDeclared; } - static std::unique_ptr ReleaseProgram(); - static DSLWriter& Instance(); static void SetInstance(std::unique_ptr instance); @@ -274,6 +272,7 @@ private: ErrorHandler* fErrorHandler = nullptr; ProgramSettings fSettings; Mangler fMangler; + bool fEncounteredErrors = false; #if !defined(SKSL_STANDALONE) && SK_SUPPORT_GPU struct StackFrame { GrGLSLFragmentProcessor* fProcessor; diff --git a/tests/SkDSLRuntimeEffectTest.cpp b/tests/SkDSLRuntimeEffectTest.cpp index 997ac8bc04..42bfaf6df7 100644 --- a/tests/SkDSLRuntimeEffectTest.cpp +++ b/tests/SkDSLRuntimeEffectTest.cpp @@ -36,10 +36,12 @@ public: StartRuntimeShader(fCompiler.get()); } - void end() { + void end(bool expectSuccess = true) { sk_sp effect = EndRuntimeShader(); - SkASSERT(effect); - fBuilder.init(std::move(effect)); + REPORTER_ASSERT(fReporter, effect ? expectSuccess : !expectSuccess); + if (effect) { + fBuilder.init(std::move(effect)); + } } SkRuntimeShaderBuilder::BuilderUniform uniform(const char* name) { @@ -191,6 +193,27 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext effect.test(0xFF000000, 0xFF0000FF, 0xFF00FF00, 0xFF00FFFF); } + // Test error reporting. We put this before a couple of successful tests to ensure that a + // failure doesn't leave us in a broken state. + { + class SimpleErrorHandler : public ErrorHandler { + public: + void handleError(const char* msg, PositionInfo* pos) override { + fMsg = msg; + } + + SkSL::String fMsg; + } errorHandler; + effect.start(); + SetErrorHandler(&errorHandler); + Var p(kFloat2_Type, "p"); + Function(kHalf4_Type, "main", p).define( + Return(1) // Error, type mismatch + ); + effect.end(false); + REPORTER_ASSERT(r, errorHandler.fMsg == "error: expected 'half4', but found 'int'\n"); + } + // Mutating coords should work. (skbug.com/10918) { effect.start();