Reland "Untangle dependency cycle in sksl dehydration"
Explanation: The sksl standalone compiler is used to convert the raw
(text) SkSL pre-includes into a "dehydrated" binary format. It also
(previously) depended on those files, as they were #included and used,
unless a special #define was changed. This created a dependency cycle
that we hid from GN (by lying about the outputs of the dehydrate step).
As a result, builds would never reach steady-state, because the compiler
would be rebuilt (due to the newer dehydrated files), and then the
dehydrated files would be rebuilt (due to the newer compiler).
This CL changes the logic so that the standalone compiler always uses
the textual pre-includes, and no longer depends on the dehydrated binary
files. Thus, to make any kind of change to the dehydrated files (whether
due to pre-include changes, or the encoding format itself), you just
need skia_compile_processors enabled. The dependencies are now honestly
communicated to GN, and we reach steady state after one build.
The NOTE above is because GN/ninja cache the dependencies of each
target, and will still think that the SkSLCompiler.obj linked into the
standalone compiler depends on the dehydrated files, at least until one
successful build, when it will realize that's no longer true.
Reland notes:
The bots originally rejected this CL, because SkSLCompiler was
hard-coded to load the text files from a relative path that assumed the
executable was in "<skia_checkout>/out/<some_dir>". That's not true for
bots, and it was fragile, even for users. Now, we use GN to directly
generate sksl_fp.sksl, and copy all of the other pre-includes to the
root out dir (working directory when running skslc). This means we
no longer need to generate the sksl_fp.sksl file into the src tree, and
the compiler can more safely assume that the files will be in the
working directory.
Bug: skia:10571
Change-Id: Id7837a9aba7ee0c3f7fa82eb84f7761e24b9c705
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308896
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/BUILD.gn b/BUILD.gn
index 61f7beb..106a5be 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -572,21 +572,6 @@
]
}
- action("create_sksl_fp") {
- script = "gn/create_sksl_fp.py"
- sources = [
- "include/private/GrSharedEnums.h",
- "src/sksl/sksl_fp_raw.sksl",
- ]
- outputs = [ "$target_out_dir/" +
- rebase_path("src/sksl/generated/sksl_fp.sksl", target_out_dir) ]
- args = [
- rebase_path(sources[0]),
- rebase_path(sources[1]),
- rebase_path(outputs[0]),
- ]
- }
-
skslc_path = "$root_out_dir/"
if (host_toolchain != default_toolchain_name) {
skslc_path += "$host_toolchain/"
@@ -596,27 +581,67 @@
skslc_path += ".exe"
}
+ action("create_sksl_fp") {
+ script = "gn/create_sksl_fp.py"
+ sources = [
+ "include/private/GrSharedEnums.h",
+ "src/sksl/sksl_fp_raw.sksl",
+ ]
+ outputs = [ "$root_out_dir/sksl_fp.sksl" ]
+ args = [
+ rebase_path(sources[0]),
+ rebase_path(sources[1]),
+ rebase_path(outputs[0]),
+ ]
+ }
+
+ copy("sksl_pre_includes") {
+ sources = [
+ "src/sksl/sksl_frag.sksl",
+ "src/sksl/sksl_geom.sksl",
+ "src/sksl/sksl_gpu.sksl",
+ "src/sksl/sksl_interp.sksl",
+ "src/sksl/sksl_pipeline.sksl",
+ "src/sksl/sksl_vert.sksl",
+ ]
+ outputs = [ "$root_out_dir/{{source_file_part}}" ]
+ }
+
+ dehydrate_sksl_sources = get_target_outputs(":sksl_pre_includes")
+ dehydrate_sksl_sources += get_target_outputs(":create_sksl_fp")
+
+ dehydrate_sksl_outputs = []
+ foreach(src, dehydrate_sksl_sources) {
+ name = get_path_info(src, "name")
+
+ # GN insists its outputs should go somewhere underneath target_out_dir, so we trick it with a
+ # path that starts with target_out_dir and then uses ".." to back up into the src dir.
+ dehydrate_sksl_outputs += [ "$target_out_dir/" + rebase_path(
+ "src/sksl/generated/$name.dehydrated.sksl",
+ target_out_dir) ]
+ }
+
action("dehydrate_sksl") {
script = "gn/dehydrate_sksl.py"
- deps = [ ":skslc(//gn/toolchain:$host_toolchain)" ]
- sources = skia_dehydrate_sksl_sources
-
- # we can't list the true outputs because it would cause a circular dependency, so we create a
- # fake file just to satisfy gn
- output = "$target_out_dir/" +
- rebase_path("src/sksl/generated/fake.output", target_out_dir)
- outputs = [ output ]
+ deps = [
+ ":create_sksl_fp",
+ ":sksl_pre_includes",
+ ":skslc(//gn/toolchain:$host_toolchain)",
+ ]
+ sources = dehydrate_sksl_sources
+ outputs = dehydrate_sksl_outputs
args = [
rebase_path(skslc_path),
- rebase_path(output),
+ rebase_path("src/sksl/generated"),
]
- args += rebase_path(skia_dehydrate_sksl_sources)
+ args += rebase_path(dehydrate_sksl_sources)
}
action("compile_processors") {
script = "gn/compile_processors.py"
deps = [
":create_sksl_fp",
+ ":sksl_pre_includes",
":skslc(//gn/toolchain:$host_toolchain)",
]
sources = skia_gpu_processor_sources
diff --git a/gn/dehydrate_sksl.py b/gn/dehydrate_sksl.py
index 0a762d0..ca34a91 100644
--- a/gn/dehydrate_sksl.py
+++ b/gn/dehydrate_sksl.py
@@ -10,7 +10,7 @@
import sys
skslc = sys.argv[1]
-output = sys.argv[2]
+targetDir = sys.argv[2]
includes = sys.argv[3:]
for inc in includes:
@@ -18,10 +18,6 @@
try:
noExt, _ = os.path.splitext(inc)
head, tail = os.path.split(noExt)
- if head.endswith("generated"):
- targetDir = head
- else:
- targetDir = os.path.join(head, "generated")
if not os.path.isdir(targetDir):
os.mkdir(targetDir)
target = os.path.join(targetDir, tail)
@@ -30,4 +26,3 @@
print("### Error compiling " + inc + ":")
print(err.output)
exit(1)
-open(output, 'w')
diff --git a/gn/sksl.gni b/gn/sksl.gni
index dc54608..ac6d7a6 100644
--- a/gn/sksl.gni
+++ b/gn/sksl.gni
@@ -112,13 +112,3 @@
"$_src/gpu/gradients/GrTwoPointConicalGradientLayout.fp",
"$_src/gpu/gradients/GrUnrolledBinaryGradientColorizer.fp",
]
-
-skia_dehydrate_sksl_sources = [
- "$_src/sksl/generated/sksl_fp.sksl",
- "$_src/sksl/sksl_frag.sksl",
- "$_src/sksl/sksl_geom.sksl",
- "$_src/sksl/sksl_gpu.sksl",
- "$_src/sksl/sksl_interp.sksl",
- "$_src/sksl/sksl_pipeline.sksl",
- "$_src/sksl/sksl_vert.sksl",
-]
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 9422240..66e66d9 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -43,14 +43,7 @@
#include "spirv-tools/libspirv.hpp"
#endif
-// If true, we use a compact binary IR representation of the core include files; otherwise we parse
-// the actual source code for the include files at runtime. The main reason you would need to change
-// this is to make format changes easier: set it to 0, change the encoder and decoder as needed,
-// build Skia to regenerate the encoded files, then set this back to 1 to actually use the
-// newly-generated files.
-#define REHYDRATE 1
-
-#if REHYDRATE
+#if !SKSL_STANDALONE
#include "src/sksl/generated/sksl_fp.dehydrated.sksl"
#include "src/sksl/generated/sksl_frag.dehydrated.sksl"
@@ -62,15 +55,14 @@
#else
-#warning SkSL rehydrator is disabled
-
-static const char SKSL_GPU_INCLUDE[] = "../../src/sksl/sksl_gpu.sksl";
-static const char SKSL_INTERP_INCLUDE[] = "../../src/sksl/sksl_interp.sksl";
-static const char SKSL_VERT_INCLUDE[] = "../../src/sksl/sksl_vert.sksl";
-static const char SKSL_FRAG_INCLUDE[] = "../../src/sksl/sksl_frag.sksl";
-static const char SKSL_GEOM_INCLUDE[] = "../../src/sksl/sksl_geom.sksl";
-static const char SKSL_FP_INCLUDE[] = "../../src/sksl/generated/sksl_fp.sksl";
-static const char SKSL_PIPELINE_INCLUDE[] = "../../src/sksl/sksl_pipeline.sksl";
+// GN generates or copies all of these files to the skslc executable directory
+static const char SKSL_GPU_INCLUDE[] = "sksl_gpu.sksl";
+static const char SKSL_INTERP_INCLUDE[] = "sksl_interp.sksl";
+static const char SKSL_VERT_INCLUDE[] = "sksl_vert.sksl";
+static const char SKSL_FRAG_INCLUDE[] = "sksl_frag.sksl";
+static const char SKSL_GEOM_INCLUDE[] = "sksl_geom.sksl";
+static const char SKSL_FP_INCLUDE[] = "sksl_fp.sksl";
+static const char SKSL_PIPELINE_INCLUDE[] = "sksl_pipeline.sksl";
#endif
@@ -245,7 +237,7 @@
fIRGenerator->fIntrinsics = &fGPUIntrinsics;
std::vector<std::unique_ptr<ProgramElement>> gpuIntrinsics;
std::vector<std::unique_ptr<ProgramElement>> interpIntrinsics;
-#if !REHYDRATE
+#if SKSL_STANDALONE
this->processIncludeFile(Program::kFragment_Kind, SKSL_GPU_INCLUDE, symbols, &gpuIntrinsics,
&fGpuSymbolTable);
this->processIncludeFile(Program::kVertex_Kind, SKSL_VERT_INCLUDE, fGpuSymbolTable,
@@ -284,7 +276,7 @@
if (fGeometrySymbolTable) {
return;
}
- #if REHYDRATE
+ #if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fGpuSymbolTable, this, SKSL_INCLUDE_sksl_geom,
SKSL_INCLUDE_sksl_geom_LENGTH);
@@ -301,7 +293,7 @@
if (fPipelineSymbolTable) {
return;
}
- #if REHYDRATE
+ #if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fGpuSymbolTable, this,
SKSL_INCLUDE_sksl_pipeline,
@@ -320,7 +312,7 @@
return;
}
this->loadPipelineIntrinsics();
- #if REHYDRATE
+ #if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fPipelineSymbolTable, this,
SKSL_INCLUDE_sksl_interp,
@@ -1558,7 +1550,7 @@
fIRGenerator->start(&settings, inherited);
break;
case Program::kFragmentProcessor_Kind: {
-#if REHYDRATE
+#if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fGpuSymbolTable, this,
SKSL_INCLUDE_sksl_fp,
diff --git a/src/sksl/generated/sksl_fp.sksl b/src/sksl/generated/sksl_fp.sksl
deleted file mode 100644
index 19b04b4..0000000
--- a/src/sksl/generated/sksl_fp.sksl
+++ /dev/null
@@ -1,71 +0,0 @@
-// *********************************************************
-// *** AUTOGENERATED BY create_sksl_fp.py, DO NOT EDIT ***
-// *********************************************************
-
-
-/*
- * Copyright 2017 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-
-/*************************************************************************************************/
-/* This file is used from both C++ and SkSL, so we need to stick to syntax compatible with both. */
-/*************************************************************************************************/
-
-/**
- * We have coverage effects that clip rendering to the edge of some geometric primitive.
- * This enum specifies how that clipping is performed. Not all factories that take a
- * GrProcessorEdgeType will succeed with all values and it is up to the caller to verify success.
- */
-enum class GrClipEdgeType {
- kFillBW,
- kFillAA,
- kInverseFillBW,
- kInverseFillAA,
-
- kLast = kInverseFillAA
-};
-
-enum class PMConversion {
- kToPremul = 0,
- kToUnpremul = 1,
- kPMConversionCnt = 2
-};
-
-
-// defines built-in interfaces supported by SkiaSL fragment shaders
-
-layout(builtin=15) in float4 sk_FragCoord;
-layout(builtin=3) float sk_ClipDistance[1];
-layout(builtin=20) out int sk_SampleMask[1];
-
-// 9999 is a temporary value that causes us to ignore these declarations beyond
-// adding them to the symbol table. This works fine in GLSL (where they do not
-// require any further handling) but will fail in SPIR-V. We'll have a better
-// solution for this soon.
-layout(builtin=9999) float4 gl_LastFragData[1];
-layout(builtin=9999) half4 gl_LastFragColor;
-layout(builtin=9999) half4 gl_LastFragColorARM;
-layout(builtin=9999) half4 gl_SecondaryFragColorEXT;
-
-layout(builtin=10003) half4 sk_InColor;
-layout(builtin=10004) out half4 sk_OutColor;
-layout(builtin=10006) sampler2D[] sk_TextureSamplers;
-layout(builtin=10011) half sk_Width;
-layout(builtin=10012) half sk_Height;
-
-half4 sample(fragmentProcessor fp);
-half4 sample(fragmentProcessor fp, float3x3 transform);
-half4 sample(fragmentProcessor fp, float2 coords);
-half4 sample(fragmentProcessor fp, half4 input);
-half4 sample(fragmentProcessor fp, half4 input, float3x3 transform);
-half4 sample(fragmentProcessor fp, half4 input, float2 coords);
-half4 sample(fragmentProcessor? fp);
-half4 sample(fragmentProcessor? fp, float3x3 transform);
-half4 sample(fragmentProcessor? fp, float2 coords);
-half4 sample(fragmentProcessor? fp, half4 input);
-half4 sample(fragmentProcessor? fp, half4 input, float3x3 transfrom);
-half4 sample(fragmentProcessor? fp, half4 input, float2 coords);
