Reland "Cleanup program building a bit"
This reverts commit 77fdf66946d2a498945394fe0b7cb06e3895aa5f.
Reason for revert: Skia-Dawn breakage should not be a tree closer.
Original change's description:
> Revert "Cleanup program building a bit"
>
> This reverts commit 4777e3addec478786dcbb68035c5e11c82479b9f.
>
> Reason for revert: This CL is breaking the build on Linux FYI SkiaRenderer Dawn Release
>
> Original change's description:
> > Cleanup program building a bit
> >
> > This CL:
> > now passes the GrProgramDesc as a const&
> > returns GrGLProgram as an sk_sp
> > makes the parameter ordering more consistent
> > makes GrVkPipelineState no longer ref-counted
> >
> > This is pulled out of the DDL pre-compile CL which touches this portion of the code.
> >
> > Bug: skia:9455
> > Change-Id: Id4d06f93450e276de5a2662be330ae9523026244
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268777
> > Reviewed-by: Greg Daniel <egdaniel@google.com>
> > Commit-Queue: Robert Phillips <robertphillips@google.com>
>
> TBR=egdaniel@google.com,robertphillips@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: skia:9455
> Change-Id: I7019d9876b68576274e87c3b2e6bbbf9695522ba
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269261
> Reviewed-by: Austin Eng <enga@google.com>
> Reviewed-by: Kenneth Russell <kbr@google.com>
> Reviewed-by: Stephen White <senorblanco@chromium.org>
> Commit-Queue: Stephen White <senorblanco@chromium.org>
> Auto-Submit: Austin Eng <enga@google.com>
TBR=egdaniel@google.com,robertphillips@google.com,senorblanco@chromium.org,kbr@google.com,enga@google.com
Change-Id: I62f6d38a8ac351e411f4605425caec3b4783fd70
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:9455
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269358
Reviewed-by: Stephen White <senorblanco@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index aa8498c..2f033f8 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -1817,7 +1817,7 @@
bool GrGLGpu::flushGLState(GrRenderTarget* renderTarget, const GrProgramInfo& programInfo) {
- sk_sp<GrGLProgram> program(fProgramCache->refProgram(this, renderTarget, programInfo));
+ sk_sp<GrGLProgram> program(fProgramCache->findOrCreateProgram(renderTarget, programInfo));
if (!program) {
GrCapsDebugf(this->caps(), "Failed to create program!\n");
return false;
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 748eb34..3f63fb6 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -316,7 +316,7 @@
void abandon();
void reset();
- GrGLProgram* refProgram(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&);
+ sk_sp<GrGLProgram> findOrCreateProgram(GrRenderTarget*, const GrProgramInfo&);
bool precompileShader(const SkData& key, const SkData& data);
private:
diff --git a/src/gpu/gl/GrGLGpuProgramCache.cpp b/src/gpu/gl/GrGLGpuProgramCache.cpp
index c5f9f2a..6453bde 100644
--- a/src/gpu/gl/GrGLGpuProgramCache.cpp
+++ b/src/gpu/gl/GrGLGpuProgramCache.cpp
@@ -45,14 +45,13 @@
fMap.reset();
}
-GrGLProgram* GrGLGpu::ProgramCache::refProgram(GrGLGpu* gpu,
- GrRenderTarget* renderTarget,
- const GrProgramInfo& programInfo) {
- const GrCaps& caps = *gpu->caps();
+sk_sp<GrGLProgram> GrGLGpu::ProgramCache::findOrCreateProgram(GrRenderTarget* renderTarget,
+ const GrProgramInfo& programInfo) {
+ const GrCaps& caps = *fGpu->caps();
GrProgramDesc desc = caps.makeDesc(renderTarget, programInfo);
if (!desc.isValid()) {
- GrCapsDebugf(gpu->caps(), "Failed to gl program descriptor!\n");
+ GrCapsDebugf(fGpu->caps(), "Failed to gl program descriptor!\n");
return nullptr;
}
@@ -61,26 +60,24 @@
// We've pre-compiled the GL program, but don't have the GrGLProgram scaffolding
const GrGLPrecompiledProgram* precompiledProgram = &((*entry)->fPrecompiledProgram);
SkASSERT(precompiledProgram->fProgramID != 0);
- GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
- &desc, fGpu,
- precompiledProgram);
- if (nullptr == program) {
+ (*entry)->fProgram = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget, desc,
+ programInfo, precompiledProgram);
+ if (!(*entry)->fProgram) {
// Should we purge the program ID from the cache at this point?
SkDEBUGFAIL("Couldn't create program from precompiled program");
return nullptr;
}
- (*entry)->fProgram.reset(program);
} else if (!entry) {
// We have a cache miss
- GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
- &desc, fGpu);
- if (nullptr == program) {
+ sk_sp<GrGLProgram> program = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget,
+ desc, programInfo);
+ if (!program) {
return nullptr;
}
- entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(sk_sp<GrGLProgram>(program))));
+ entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(std::move(program))));
}
- return SkRef((*entry)->fProgram.get());
+ return (*entry)->fProgram;
}
bool GrGLGpu::ProgramCache::precompileShader(const SkData& key, const SkData& data) {
diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.cpp b/src/gpu/gl/builders/GrGLProgramBuilder.cpp
index 6c12794..012140f 100644
--- a/src/gpu/gl/builders/GrGLProgramBuilder.cpp
+++ b/src/gpu/gl/builders/GrGLProgramBuilder.cpp
@@ -45,21 +45,22 @@
cleanup_shaders(gpu, shaderIDs);
}
-GrGLProgram* GrGLProgramBuilder::CreateProgram(GrRenderTarget* renderTarget,
- const GrProgramInfo& programInfo,
- GrProgramDesc* desc,
+sk_sp<GrGLProgram> GrGLProgramBuilder::CreateProgram(
GrGLGpu* gpu,
+ GrRenderTarget* renderTarget,
+ const GrProgramDesc& desc,
+ const GrProgramInfo& programInfo,
const GrGLPrecompiledProgram* precompiledProgram) {
ATRACE_ANDROID_FRAMEWORK_ALWAYS("shader_compile");
GrAutoLocaleSetter als("C");
// create a builder. This will be handed off to effects so they can use it to add
// uniforms, varyings, textures, etc
- GrGLProgramBuilder builder(gpu, renderTarget, programInfo, desc);
+ GrGLProgramBuilder builder(gpu, renderTarget, desc, programInfo);
auto persistentCache = gpu->getContext()->priv().getPersistentCache();
if (persistentCache && !precompiledProgram) {
- sk_sp<SkData> key = SkData::MakeWithoutCopy(desc->asKey(), desc->keyLength());
+ sk_sp<SkData> key = SkData::MakeWithoutCopy(desc.asKey(), desc.keyLength());
builder.fCached = persistentCache->load(*key);
// the eventual end goal is to completely skip emitAndInstallProcs on a cache hit, but it's
// doing necessary setup in addition to generating the SkSL code. Currently we are only able
@@ -75,9 +76,9 @@
GrGLProgramBuilder::GrGLProgramBuilder(GrGLGpu* gpu,
GrRenderTarget* renderTarget,
- const GrProgramInfo& programInfo,
- GrProgramDesc* desc)
- : INHERITED(renderTarget, programInfo, desc)
+ const GrProgramDesc& desc,
+ const GrProgramInfo& programInfo)
+ : INHERITED(renderTarget, desc, programInfo)
, fGpu(gpu)
, fVaryingHandler(this)
, fUniformHandler(this)
@@ -159,7 +160,7 @@
if (!this->gpu()->getContext()->priv().getPersistentCache()) {
return;
}
- sk_sp<SkData> key = SkData::MakeWithoutCopy(this->desc()->asKey(), this->desc()->keyLength());
+ sk_sp<SkData> key = SkData::MakeWithoutCopy(this->desc().asKey(), this->desc().keyLength());
if (fGpu->glCaps().programBinarySupport()) {
// binary cache
GrGLsizei length = 0;
@@ -198,7 +199,7 @@
}
}
-GrGLProgram* GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
+sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
TRACE_EVENT0("skia.gpu", TRACE_FUNC);
// verify we can get a program id
@@ -541,22 +542,22 @@
}
}
-GrGLProgram* GrGLProgramBuilder::createProgram(GrGLuint programID) {
- return new GrGLProgram(fGpu,
- fUniformHandles,
- programID,
- fUniformHandler.fUniforms,
- fUniformHandler.fSamplers,
- fVaryingHandler.fPathProcVaryingInfos,
- std::move(fGeometryProcessor),
- std::move(fXferProcessor),
- std::move(fFragmentProcessors),
- fFragmentProcessorCnt,
- std::move(fAttributes),
- fVertexAttributeCnt,
- fInstanceAttributeCnt,
- fVertexStride,
- fInstanceStride);
+sk_sp<GrGLProgram> GrGLProgramBuilder::createProgram(GrGLuint programID) {
+ return sk_sp<GrGLProgram>(new GrGLProgram(fGpu,
+ fUniformHandles,
+ programID,
+ fUniformHandler.fUniforms,
+ fUniformHandler.fSamplers,
+ fVaryingHandler.fPathProcVaryingInfos,
+ std::move(fGeometryProcessor),
+ std::move(fXferProcessor),
+ std::move(fFragmentProcessors),
+ fFragmentProcessorCnt,
+ std::move(fAttributes),
+ fVertexAttributeCnt,
+ fInstanceAttributeCnt,
+ fVertexStride,
+ fInstanceStride));
}
bool GrGLProgramBuilder::PrecompileProgram(GrGLPrecompiledProgram* precompiledProgram,
diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.h b/src/gpu/gl/builders/GrGLProgramBuilder.h
index 800c64a..baf319d 100644
--- a/src/gpu/gl/builders/GrGLProgramBuilder.h
+++ b/src/gpu/gl/builders/GrGLProgramBuilder.h
@@ -40,18 +40,15 @@
* The program implements what is specified in the stages given as input.
* After successful generation, the builder result objects are available
* to be used.
- * This function may modify the GrProgramDesc by setting the surface origin
- * key to 0 (unspecified) if it turns out the program does not care about
- * the surface origin.
* If a GL program has already been created, the program ID and inputs can
* be supplied to skip the shader compilation.
- * @return true if generation was successful.
+ * @return the created program if generation was successful.
*/
- static GrGLProgram* CreateProgram(GrRenderTarget*,
- const GrProgramInfo&,
- GrProgramDesc*,
- GrGLGpu*,
- const GrGLPrecompiledProgram* = nullptr);
+ static sk_sp<GrGLProgram> CreateProgram(GrGLGpu*,
+ GrRenderTarget*,
+ const GrProgramDesc&,
+ const GrProgramInfo&,
+ const GrGLPrecompiledProgram* = nullptr);
static bool PrecompileProgram(GrGLPrecompiledProgram*, GrGLGpu*, const SkData&);
@@ -60,7 +57,7 @@
GrGLGpu* gpu() const { return fGpu; }
private:
- GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);
+ GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
void addInputVars(const SkSL::Program::Inputs& inputs);
bool compileAndAttachShaders(const SkSL::String& glsl,
@@ -74,14 +71,14 @@
void storeShaderInCache(const SkSL::Program::Inputs& inputs, GrGLuint programID,
const SkSL::String shaders[], bool isSkSL,
SkSL::Program::Settings* settings);
- GrGLProgram* finalize(const GrGLPrecompiledProgram*);
+ sk_sp<GrGLProgram> finalize(const GrGLPrecompiledProgram*);
void bindProgramResourceLocations(GrGLuint programID);
bool checkLinkStatus(GrGLuint programID, GrContextOptions::ShaderErrorHandler* errorHandler,
SkSL::String* sksl[], const SkSL::String glsl[]);
void resolveProgramResourceLocations(GrGLuint programID, bool force);
// Subclasses create different programs
- GrGLProgram* createProgram(GrGLuint programID);
+ sk_sp<GrGLProgram> createProgram(GrGLuint programID);
GrGLSLUniformHandler* uniformHandler() override { return &fUniformHandler; }
const GrGLSLUniformHandler* uniformHandler() const override { return &fUniformHandler; }