Revert "Rearrange SkSL pre-include modules to hide things"

This reverts commit bea0dc6.

No-Try: true
Change-Id: I7b000de199dc23bd3719a4ee835b2a8d3c93fefd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332747
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

Author: brianosman

BUILD.gn

@@ -604,7 +604,6 @@ if (skia_compile_processors || skia_compile_sksl_tests) {
       "src/sksl/sksl_gpu.sksl",
       "src/sksl/sksl_interp.sksl",
       "src/sksl/sksl_pipeline.sksl",
-      "src/sksl/sksl_public.sksl",
       "src/sksl/sksl_vert.sksl",
     ]
     outputs = [ "$root_out_dir/{{source_file_part}}" ]

RELEASE_NOTES.txt

@@ -9,8 +9,9 @@ Milestone 88
 
   * <insert new release notes here>
 
-  * Limit the types and intrinsics supported in SkRuntimeEffect to GLSL ES 1.00
-    https://review.skia.org/332597
+  * Image filters with perspective, for saveLayer and draws, are now drawn correctly. Performance
+    and quality is highest if user bounds are provided to saveLayer.
+    https://review.skia.org/328376
 
   * Add AVIF support to SkHeifCodec.
 

src/sksl/SkSLByteCodeGenerator.cpp

@@ -43,9 +43,9 @@ ByteCodeGenerator::ByteCodeGenerator(const Context* context, const Program* prog
     : INHERITED(program, errors, nullptr)
     , fContext(*context)
     , fOutput(output)
-    // If you're adding new intrinsics here, ensure that they're declared in sksl_interp.sksl or
-    // sksl_public.sksl, so they're available to "generic" interpreter programs (eg particles).
-    // You can probably copy the declarations from sksl_gpu.sksl.
+    // If you're adding new intrinsics here, ensure that they're declared in sksl_interp.inc, so
+    // they're available to "generic" interpreter programs (eg particles).
+    // You can probably copy the declarations from sksl_gpu.inc.
     , fIntrinsics {
         { "abs",       ByteCodeInstruction::kAbs },
         { "atan",      ByteCodeInstruction::kATan },

src/sksl/SkSLCompiler.cpp

@@ -61,7 +61,6 @@
 #include "src/sksl/generated/sksl_gpu.dehydrated.sksl"
 #include "src/sksl/generated/sksl_interp.dehydrated.sksl"
 #include "src/sksl/generated/sksl_pipeline.dehydrated.sksl"
-#include "src/sksl/generated/sksl_public.dehydrated.sksl"
 #include "src/sksl/generated/sksl_vert.dehydrated.sksl"
 
 #define MODULE_DATA(name) MakeModuleData(SKSL_INCLUDE_sksl_##name,\
@@ -90,93 +89,142 @@ Compiler::Compiler(const ShaderCapsClass* caps, Flags flags)
 , fContext(std::make_shared<Context>())
 , fErrorCount(0) {
     fRootSymbolTable = std::make_shared<SymbolTable>(this, /*builtin=*/true);
-    fPrivateSymbolTable = std::make_shared<SymbolTable>(fRootSymbolTable, /*builtin=*/true);
     fIRGenerator = std::make_unique<IRGenerator>(fContext.get(), &fInliner, *this);
-
-#define TYPE(t) fContext->f##t##_Type.get()
-
-    const SkSL::Symbol* rootTypes[] = {
-        TYPE(Void),
-
-        TYPE( Float), TYPE( Float2), TYPE( Float3), TYPE( Float4),
-        TYPE(  Half), TYPE(  Half2), TYPE(  Half3), TYPE(  Half4),
-        TYPE(   Int), TYPE(   Int2), TYPE(   Int3), TYPE(   Int4),
-        TYPE(  UInt), TYPE(  UInt2), TYPE(  UInt3), TYPE(  UInt4),
-        TYPE( Short), TYPE( Short2), TYPE( Short3), TYPE( Short4),
-        TYPE(UShort), TYPE(UShort2), TYPE(UShort3), TYPE(UShort4),
-        TYPE(  Byte), TYPE(  Byte2), TYPE(  Byte3), TYPE(  Byte4),
-        TYPE( UByte), TYPE( UByte2), TYPE( UByte3), TYPE( UByte4),
-        TYPE(  Bool), TYPE(  Bool2), TYPE(  Bool3), TYPE(  Bool4),
-
-        TYPE(Float2x2), TYPE(Float2x3), TYPE(Float2x4),
-        TYPE(Float3x2), TYPE(Float3x3), TYPE(Float3x4),
-        TYPE(Float4x2), TYPE(Float4x3), TYPE(Float4x4),
-
-        TYPE(Half2x2),  TYPE(Half2x3),  TYPE(Half2x4),
-        TYPE(Half3x2),  TYPE(Half3x3),  TYPE(Half3x4),
-        TYPE(Half4x2),  TYPE(Half4x3),  TYPE(Half4x4),
-
-        TYPE(GenType), TYPE(GenHType), TYPE(GenIType), TYPE(GenUType), TYPE(GenBType),
-        TYPE(Mat), TYPE(Vec),
-        TYPE(GVec), TYPE(GVec2), TYPE(GVec3), TYPE(GVec4),
-        TYPE(HVec), TYPE(IVec), TYPE(UVec), TYPE(SVec), TYPE(USVec),
-        TYPE(ByteVec), TYPE(UByteVec), TYPE(BVec),
-
-        TYPE(FragmentProcessor),
-    };
-
-    const SkSL::Symbol* privateTypes[] = {
-        TYPE(Sampler1D), TYPE(Sampler2D), TYPE(Sampler3D),
-        TYPE(SamplerExternalOES),
-        TYPE(SamplerCube),
-        TYPE(Sampler2DRect),
-        TYPE(Sampler1DArray), TYPE(Sampler2DArray), TYPE(SamplerCubeArray),
-        TYPE(SamplerBuffer),
-        TYPE(Sampler2DMS), TYPE(Sampler2DMSArray),
-
-        TYPE(ISampler2D),
-        TYPE(Image2D), TYPE(IImage2D),
-        TYPE(SubpassInput), TYPE(SubpassInputMS),
-
-        TYPE(GSampler1D), TYPE(GSampler2D), TYPE(GSampler3D),
-        TYPE(GSamplerCube),
-        TYPE(GSampler2DRect),
-        TYPE(GSampler1DArray), TYPE(GSampler2DArray), TYPE(GSamplerCubeArray),
-        TYPE(GSamplerBuffer),
-        TYPE(GSampler2DMS), TYPE(GSampler2DMSArray),
-
-        TYPE(Sampler1DShadow), TYPE(Sampler2DShadow), TYPE(SamplerCubeShadow),
-        TYPE(Sampler2DRectShadow),
-        TYPE(Sampler1DArrayShadow), TYPE(Sampler2DArrayShadow), TYPE(SamplerCubeArrayShadow),
-
-        TYPE(GSampler2DArrayShadow), TYPE(GSamplerCubeArrayShadow),
-        TYPE(Sampler),
-        TYPE(Texture2D),
-    };
-
-    for (const SkSL::Symbol* type : rootTypes) {
-        fRootSymbolTable->addWithoutOwnership(type);
-    }
-    for (const SkSL::Symbol* type : privateTypes) {
-        fPrivateSymbolTable->addWithoutOwnership(type);
-    }
-
-#undef TYPE
+#define ADD_TYPE(t) fRootSymbolTable->addWithoutOwnership(fContext->f##t##_Type.get())
+    ADD_TYPE(Void);
+    ADD_TYPE(Float);
+    ADD_TYPE(Float2);
+    ADD_TYPE(Float3);
+    ADD_TYPE(Float4);
+    ADD_TYPE(Half);
+    ADD_TYPE(Half2);
+    ADD_TYPE(Half3);
+    ADD_TYPE(Half4);
+    ADD_TYPE(Int);
+    ADD_TYPE(Int2);
+    ADD_TYPE(Int3);
+    ADD_TYPE(Int4);
+    ADD_TYPE(UInt);
+    ADD_TYPE(UInt2);
+    ADD_TYPE(UInt3);
+    ADD_TYPE(UInt4);
+    ADD_TYPE(Short);
+    ADD_TYPE(Short2);
+    ADD_TYPE(Short3);
+    ADD_TYPE(Short4);
+    ADD_TYPE(UShort);
+    ADD_TYPE(UShort2);
+    ADD_TYPE(UShort3);
+    ADD_TYPE(UShort4);
+    ADD_TYPE(Byte);
+    ADD_TYPE(Byte2);
+    ADD_TYPE(Byte3);
+    ADD_TYPE(Byte4);
+    ADD_TYPE(UByte);
+    ADD_TYPE(UByte2);
+    ADD_TYPE(UByte3);
+    ADD_TYPE(UByte4);
+    ADD_TYPE(Bool);
+    ADD_TYPE(Bool2);
+    ADD_TYPE(Bool3);
+    ADD_TYPE(Bool4);
+    ADD_TYPE(Float2x2);
+    ADD_TYPE(Float2x3);
+    ADD_TYPE(Float2x4);
+    ADD_TYPE(Float3x2);
+    ADD_TYPE(Float3x3);
+    ADD_TYPE(Float3x4);
+    ADD_TYPE(Float4x2);
+    ADD_TYPE(Float4x3);
+    ADD_TYPE(Float4x4);
+    ADD_TYPE(Half2x2);
+    ADD_TYPE(Half2x3);
+    ADD_TYPE(Half2x4);
+    ADD_TYPE(Half3x2);
+    ADD_TYPE(Half3x3);
+    ADD_TYPE(Half3x4);
+    ADD_TYPE(Half4x2);
+    ADD_TYPE(Half4x3);
+    ADD_TYPE(Half4x4);
+    ADD_TYPE(GenType);
+    ADD_TYPE(GenHType);
+    ADD_TYPE(GenIType);
+    ADD_TYPE(GenUType);
+    ADD_TYPE(GenBType);
+    ADD_TYPE(Mat);
+    ADD_TYPE(Vec);
+    ADD_TYPE(GVec);
+    ADD_TYPE(GVec2);
+    ADD_TYPE(GVec3);
+    ADD_TYPE(GVec4);
+    ADD_TYPE(HVec);
+    ADD_TYPE(IVec);
+    ADD_TYPE(UVec);
+    ADD_TYPE(SVec);
+    ADD_TYPE(USVec);
+    ADD_TYPE(ByteVec);
+    ADD_TYPE(UByteVec);
+    ADD_TYPE(BVec);
+
+    ADD_TYPE(Sampler1D);
+    ADD_TYPE(Sampler2D);
+    ADD_TYPE(Sampler3D);
+    ADD_TYPE(SamplerExternalOES);
+    ADD_TYPE(SamplerCube);
+    ADD_TYPE(Sampler2DRect);
+    ADD_TYPE(Sampler1DArray);
+    ADD_TYPE(Sampler2DArray);
+    ADD_TYPE(SamplerCubeArray);
+    ADD_TYPE(SamplerBuffer);
+    ADD_TYPE(Sampler2DMS);
+    ADD_TYPE(Sampler2DMSArray);
+
+    ADD_TYPE(ISampler2D);
+
+    ADD_TYPE(Image2D);
+    ADD_TYPE(IImage2D);
+
+    ADD_TYPE(SubpassInput);
+    ADD_TYPE(SubpassInputMS);
+
+    ADD_TYPE(GSampler1D);
+    ADD_TYPE(GSampler2D);
+    ADD_TYPE(GSampler3D);
+    ADD_TYPE(GSamplerCube);
+    ADD_TYPE(GSampler2DRect);
+    ADD_TYPE(GSampler1DArray);
+    ADD_TYPE(GSampler2DArray);
+    ADD_TYPE(GSamplerCubeArray);
+    ADD_TYPE(GSamplerBuffer);
+    ADD_TYPE(GSampler2DMS);
+    ADD_TYPE(GSampler2DMSArray);
+
+    ADD_TYPE(Sampler1DShadow);
+    ADD_TYPE(Sampler2DShadow);
+    ADD_TYPE(SamplerCubeShadow);
+    ADD_TYPE(Sampler2DRectShadow);
+    ADD_TYPE(Sampler1DArrayShadow);
+    ADD_TYPE(Sampler2DArrayShadow);
+    ADD_TYPE(SamplerCubeArrayShadow);
+    ADD_TYPE(GSampler2DArrayShadow);
+    ADD_TYPE(GSamplerCubeArrayShadow);
+    ADD_TYPE(FragmentProcessor);
+    ADD_TYPE(Sampler);
+    ADD_TYPE(Texture2D);
 
     // sk_Caps is "builtin", but all references to it are resolved to Settings, so we don't need to
     // treat it as builtin (ie, no need to clone it into the Program).
-    fPrivateSymbolTable->add(
-            std::make_unique<Variable>(/*offset=*/-1,
-                                       fIRGenerator->fModifiers->handle(Modifiers()),
-                                       "sk_Caps",
-                                       fContext->fSkCaps_Type.get(),
-                                       /*builtin=*/false,
-                                       Variable::Storage::kGlobal));
+    StringFragment skCapsName("sk_Caps");
+    fRootSymbolTable->add(std::make_unique<Variable>(/*offset=*/-1,
+                                                     fIRGenerator->fModifiers->handle(Modifiers()),
+                                                     skCapsName,
+                                                     fContext->fSkCaps_Type.get(),
+                                                     /*builtin=*/false,
+                                                     Variable::Storage::kGlobal));
 
     fRootModule = {fRootSymbolTable, /*fIntrinsics=*/nullptr};
-    fPrivateModule = {fPrivateSymbolTable, /*fIntrinsics=*/nullptr};
 
-    fGPUModule = this->parseModule(Program::kFragment_Kind, MODULE_DATA(gpu), fPrivateModule);
+    fGPUModule = this->parseModule(Program::kFragment_Kind, MODULE_DATA(gpu), fRootModule);
     fVertexModule = this->parseModule(Program::kVertex_Kind, MODULE_DATA(vert), fGPUModule);
     fFragmentModule = this->parseModule(Program::kFragment_Kind, MODULE_DATA(frag), fGPUModule);
 }
@@ -198,17 +246,10 @@ const ParsedModule& Compiler::loadFPModule() {
     return fFPModule;
 }
 
-const ParsedModule& Compiler::loadPublicModule() {
-    if (!fPublicModule.fSymbols) {
-        fPublicModule = this->parseModule(Program::kGeneric_Kind, MODULE_DATA(public), fRootModule);
-    }
-    return fPublicModule;
-}
-
 const ParsedModule& Compiler::loadPipelineModule() {
     if (!fPipelineModule.fSymbols) {
-        fPipelineModule = this->parseModule(Program::kPipelineStage_Kind, MODULE_DATA(pipeline),
-                                            this->loadPublicModule());
+        fPipelineModule =
+                this->parseModule(Program::kPipelineStage_Kind, MODULE_DATA(pipeline), fGPUModule);
 
         // Add some aliases to the pipeline module so that it's friendlier, and more like GLSL
         fPipelineModule.fSymbols->addAlias("shader", fContext->fFragmentProcessor_Type.get());
@@ -242,8 +283,8 @@ const ParsedModule& Compiler::loadPipelineModule() {
 
 const ParsedModule& Compiler::loadInterpreterModule() {
     if (!fInterpreterModule.fSymbols) {
-        fInterpreterModule = this->parseModule(Program::kGeneric_Kind, MODULE_DATA(interp),
-                                               this->loadPublicModule());
+        fInterpreterModule =
+                this->parseModule(Program::kGeneric_Kind, MODULE_DATA(interp), fRootModule);
     }
     return fInterpreterModule;
 }
@@ -264,13 +305,7 @@ LoadedModule Compiler::loadModule(Program::Kind kind,
                                   ModuleData data,
                                   std::shared_ptr<SymbolTable> base) {
     if (!base) {
-        // NOTE: This is a workaround. The only time 'base' is null is when dehydrating includes.
-        // In that case, skslc doesn't know which module it's preparing, nor what the correct base
-        // module is. We can't use 'Root', because many GPU intrinsics reference private types,
-        // like samplers or textures. Today, 'Private' does contain the union of all known types,
-        // so this is safe. If we ever have types that only exist in 'Public' (for example), this
-        // logic needs to be smarter (by choosing the correct base for the module we're compiling).
-        base = fPrivateSymbolTable;
+        base = fRootSymbolTable;
     }
 
 #if defined(SKSL_STANDALONE)

src/sksl/SkSLCompiler.h

@@ -228,7 +228,6 @@ class SK_API Compiler : public ErrorReporter {
 private:
     const ParsedModule& loadFPModule();
     const ParsedModule& loadGeometryModule();
-    const ParsedModule& loadPublicModule();
     const ParsedModule& loadInterpreterModule();
     const ParsedModule& loadPipelineModule();
 
@@ -272,20 +271,15 @@ class SK_API Compiler : public ErrorReporter {
     const ShaderCapsClass* fCaps = nullptr;
 
     std::shared_ptr<SymbolTable> fRootSymbolTable;
-    std::shared_ptr<SymbolTable> fPrivateSymbolTable;
 
-    ParsedModule fRootModule;         // Core types
-
-    ParsedModule fPrivateModule;      // [Root] + Internal types
-    ParsedModule fGPUModule;          // [Private] + GPU intrinsics, helper functions
-    ParsedModule fVertexModule;       // [GPU] + Vertex stage decls
-    ParsedModule fFragmentModule;     // [GPU] + Fragment stage decls
-    ParsedModule fGeometryModule;     // [GPU] + Geometry stage decls
-    ParsedModule fFPModule;           // [GPU] + FP features
-
-    ParsedModule fPublicModule;       // [Root] + Public features
-    ParsedModule fInterpreterModule;  // [Public] + Interpreter-only decls
-    ParsedModule fPipelineModule;     // [Public] + Runtime effect decls
+    ParsedModule fRootModule;
+    ParsedModule fGPUModule;
+    ParsedModule fInterpreterModule;
+    ParsedModule fVertexModule;
+    ParsedModule fFragmentModule;
+    ParsedModule fGeometryModule;
+    ParsedModule fPipelineModule;
+    ParsedModule fFPModule;
 
     // holds ModifiersPools belonging to the core includes for lifetime purposes
     std::vector<std::unique_ptr<ModifiersPool>> fModifiers;