When precompiling SkSL, avoid the need to re-link
Adds metadata to the SkSL blobs about attributes (and other resources)
so that we can do all necessary work during precompile.
Change-Id: I1846c6c96946d5a43a48112d062853717a6571a0
Bug: skia:9402
Bug: b/140174804
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/243739
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrPersistentCacheUtils.h b/src/gpu/GrPersistentCacheUtils.h
index e96f5a0..b460184 100644
--- a/src/gpu/GrPersistentCacheUtils.h
+++ b/src/gpu/GrPersistentCacheUtils.h
@@ -20,11 +20,18 @@
// put the serialization logic here, to be shared by the backend code and the tool code.
namespace GrPersistentCacheUtils {
+struct ShaderMetadata {
+ SkSL::Program::Settings* fSettings = nullptr;
+ SkTArray<SkSL::String> fAttributeNames;
+ bool fHasCustomColorOutput = false;
+ bool fHasSecondaryColorOutput = false;
+};
+
static inline sk_sp<SkData> PackCachedShaders(SkFourByteTag shaderType,
const SkSL::String shaders[],
const SkSL::Program::Inputs inputs[],
int numInputs,
- const SkSL::Program::Settings* settings) {
+ const ShaderMetadata* meta = nullptr) {
// For consistency (so tools can blindly pack and unpack cached shaders), we always write
// kGrShaderTypeCount inputs. If the backend gives us fewer, we just replicate the last one.
SkASSERT(numInputs >= 1 && numInputs <= kGrShaderTypeCount);
@@ -35,11 +42,22 @@
writer.writeString(shaders[i].c_str(), shaders[i].size());
writer.writePad(&inputs[SkTMin(i, numInputs - 1)], sizeof(SkSL::Program::Inputs));
}
- writer.writeBool(SkToBool(settings));
- if (settings) {
- writer.writeBool(settings->fFlipY);
- writer.writeBool(settings->fFragColorIsInOut);
- writer.writeBool(settings->fForceHighPrecision);
+ writer.writeBool(SkToBool(meta));
+ if (meta) {
+ writer.writeBool(SkToBool(meta->fSettings));
+ if (meta->fSettings) {
+ writer.writeBool(meta->fSettings->fFlipY);
+ writer.writeBool(meta->fSettings->fFragColorIsInOut);
+ writer.writeBool(meta->fSettings->fForceHighPrecision);
+ }
+
+ writer.writeInt(meta->fAttributeNames.count());
+ for (const auto& attr : meta->fAttributeNames) {
+ writer.writeString(attr.c_str(), attr.size());
+ }
+
+ writer.writeBool(meta->fHasCustomColorOutput);
+ writer.writeBool(meta->fHasSecondaryColorOutput);
}
return writer.snapshotAsData();
}
@@ -48,7 +66,7 @@
SkSL::String shaders[],
SkSL::Program::Inputs inputs[],
int numInputs,
- SkSL::Program::Settings* settings = nullptr) {
+ ShaderMetadata* meta = nullptr) {
for (int i = 0; i < kGrShaderTypeCount; ++i) {
size_t stringLen = 0;
const char* string = reader->readString(&stringLen);
@@ -61,10 +79,24 @@
reader->skip(sizeof(SkSL::Program::Inputs));
}
}
- if (reader->readBool() && settings) {
- settings->fFlipY = reader->readBool();
- settings->fFragColorIsInOut = reader->readBool();
- settings->fForceHighPrecision = reader->readBool();
+ if (reader->readBool() && meta) {
+ SkASSERT(meta->fSettings != nullptr);
+
+ if (reader->readBool()) {
+ meta->fSettings->fFlipY = reader->readBool();
+ meta->fSettings->fFragColorIsInOut = reader->readBool();
+ meta->fSettings->fForceHighPrecision = reader->readBool();
+ }
+
+ meta->fAttributeNames.resize(reader->readInt());
+ for (int i = 0; i < meta->fAttributeNames.count(); ++i) {
+ size_t stringLen = 0;
+ const char* string = reader->readString(&stringLen);
+ meta->fAttributeNames[i] = SkSL::String(string, stringLen);
+ }
+
+ meta->fHasCustomColorOutput = reader->readBool();
+ meta->fHasSecondaryColorOutput = reader->readBool();
}
}
diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.cpp b/src/gpu/gl/builders/GrGLProgramBuilder.cpp
index 38c15ff..c521c31 100644
--- a/src/gpu/gl/builders/GrGLProgramBuilder.cpp
+++ b/src/gpu/gl/builders/GrGLProgramBuilder.cpp
@@ -163,7 +163,7 @@
void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs, GrGLuint programID,
const SkSL::String shaders[], bool isSkSL,
- const SkSL::Program::Settings& settings) {
+ SkSL::Program::Settings* settings) {
if (!this->gpu()->getContext()->priv().getPersistentCache()) {
return;
}
@@ -188,9 +188,20 @@
this->gpu()->getContext()->priv().getPersistentCache()->store(*key, *data);
}
} else {
- // source cache
+ // source cache, plus metadata to allow for a complete precompile
+ GrPersistentCacheUtils::ShaderMetadata meta;
+ meta.fSettings = settings;
+ meta.fHasCustomColorOutput = fFS.hasCustomColorOutput();
+ meta.fHasSecondaryColorOutput = fFS.hasSecondaryOutput();
+ for (const auto& attr : this->primitiveProcessor().vertexAttributes()) {
+ meta.fAttributeNames.emplace_back(attr.name());
+ }
+ for (const auto& attr : this->primitiveProcessor().instanceAttributes()) {
+ meta.fAttributeNames.emplace_back(attr.name());
+ }
+
auto data = GrPersistentCacheUtils::PackCachedShaders(isSkSL ? kSKSL_Tag : kGLSL_Tag,
- shaders, &inputs, 1, &settings);
+ shaders, &inputs, 1, &meta);
this->gpu()->getContext()->priv().getPersistentCache()->store(*key, *data);
}
}
@@ -247,12 +258,8 @@
if (precompiledProgram) {
// This is very similar to when we get program binaries. We even set that flag, as it's
// used to prevent other compile work later, and to force re-querying uniform locations.
- // We couldn't bind attribute or program resource locations during the pre-compile, so do
- // that now. Those APIs don't take effect until the next link, so re-link the program.
this->addInputVars(precompiledProgram->fInputs);
- this->computeCountsAndStrides(programID, primProc, true);
- this->bindProgramResourceLocations(programID);
- GL_CALL(LinkProgram(programID));
+ this->computeCountsAndStrides(programID, primProc, false);
usedProgramBinaries = true;
} else if (cached) {
SkReader32 reader(fCached->data(), fCached->size());
@@ -416,7 +423,7 @@
}
isSkSL = true;
}
- this->storeShaderInCache(inputs, programID, glsl, isSkSL, settings);
+ this->storeShaderInCache(inputs, programID, glsl, isSkSL, &settings);
}
return this->createProgram(programID);
}
@@ -540,12 +547,15 @@
SkTDArray<GrGLuint> shadersToDelete;
SkSL::Program::Settings settings;
- settings.fCaps = gpu->glCaps().shaderCaps();
+ const GrGLCaps& caps = gpu->glCaps();
+ settings.fCaps = caps.shaderCaps();
settings.fSharpenTextures = gpu->getContext()->priv().options().fSharpenMipmappedTextures;
+ GrPersistentCacheUtils::ShaderMetadata meta;
+ meta.fSettings = &settings;
SkSL::String shaders[kGrShaderTypeCount];
SkSL::Program::Inputs inputs;
- GrPersistentCacheUtils::UnpackCachedShaders(&reader, shaders, &inputs, 1, &settings);
+ GrPersistentCacheUtils::UnpackCachedShaders(&reader, shaders, &inputs, 1, &meta);
auto compileShader = [&](SkSL::Program::Kind kind, const SkSL::String& sksl, GrGLenum type) {
SkSL::String glsl;
@@ -577,6 +587,20 @@
return false;
}
+ for (int i = 0; i < meta.fAttributeNames.count(); ++i) {
+ GR_GL_CALL(gpu->glInterface(), BindAttribLocation(programID, i,
+ meta.fAttributeNames[i].c_str()));
+ }
+
+ if (meta.fHasCustomColorOutput && caps.bindFragDataLocationSupport()) {
+ GR_GL_CALL(gpu->glInterface(), BindFragDataLocation(programID, 0,
+ GrGLSLFragmentShaderBuilder::DeclaredColorOutputName()));
+ }
+ if (meta.fHasSecondaryColorOutput && caps.shaderCaps()->mustDeclareFragmentShaderOutput()) {
+ GR_GL_CALL(gpu->glInterface(), BindFragDataLocationIndexed(programID, 0, 1,
+ GrGLSLFragmentShaderBuilder::DeclaredSecondaryColorOutputName()));
+ }
+
GR_GL_CALL(gpu->glInterface(), LinkProgram(programID));
GrGLint linked = GR_GL_INIT_ZERO;
GR_GL_CALL(gpu->glInterface(), GetProgramiv(programID, GR_GL_LINK_STATUS, &linked));
diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.h b/src/gpu/gl/builders/GrGLProgramBuilder.h
index de97d42..95c7c99 100644
--- a/src/gpu/gl/builders/GrGLProgramBuilder.h
+++ b/src/gpu/gl/builders/GrGLProgramBuilder.h
@@ -77,7 +77,7 @@
bool bindAttribLocations);
void storeShaderInCache(const SkSL::Program::Inputs& inputs, GrGLuint programID,
const SkSL::String shaders[], bool isSkSL,
- const SkSL::Program::Settings& settings);
+ SkSL::Program::Settings* settings);
GrGLProgram* finalize(const GrGLPrecompiledProgram*);
void bindProgramResourceLocations(GrGLuint programID);
bool checkLinkStatus(GrGLuint programID, GrContextOptions::ShaderErrorHandler* errorHandler,
diff --git a/src/gpu/vk/GrVkPipelineStateBuilder.cpp b/src/gpu/vk/GrVkPipelineStateBuilder.cpp
index 0919827..8428924 100644
--- a/src/gpu/vk/GrVkPipelineStateBuilder.cpp
+++ b/src/gpu/vk/GrVkPipelineStateBuilder.cpp
@@ -138,14 +138,12 @@
void GrVkPipelineStateBuilder::storeShadersInCache(const SkSL::String shaders[],
const SkSL::Program::Inputs inputs[],
- bool isSkSL,
- const SkSL::Program::Settings& settings) {
+ bool isSkSL) {
Desc* desc = static_cast<Desc*>(this->desc());
sk_sp<SkData> key = SkData::MakeWithoutCopy(desc->asKey(), desc->shaderKeyLength());
sk_sp<SkData> data = GrPersistentCacheUtils::PackCachedShaders(isSkSL ? kSKSL_Tag : kSPIRV_Tag,
shaders,
- inputs, kGrShaderTypeCount,
- &settings);
+ inputs, kGrShaderTypeCount);
this->gpu()->getContext()->priv().getPersistentCache()->store(*key, *data);
}
@@ -290,7 +288,7 @@
}
isSkSL = true;
}
- this->storeShadersInCache(shaders, inputs, isSkSL, settings);
+ this->storeShadersInCache(shaders, inputs, isSkSL);
}
}
GrVkPipeline* pipeline = resourceProvider.createPipeline(
diff --git a/src/gpu/vk/GrVkPipelineStateBuilder.h b/src/gpu/vk/GrVkPipelineStateBuilder.h
index 4cb22d0..980e6ac 100644
--- a/src/gpu/vk/GrVkPipelineStateBuilder.h
+++ b/src/gpu/vk/GrVkPipelineStateBuilder.h
@@ -96,7 +96,7 @@
VkPipelineShaderStageCreateInfo* outStageInfo);
void storeShadersInCache(const SkSL::String shaders[], const SkSL::Program::Inputs inputs[],
- bool isSkSL, const SkSL::Program::Settings& settings);
+ bool isSkSL);
bool createVkShaderModule(VkShaderStageFlagBits stage,
const SkSL::String& sksl,