Skip to content

Commit

Permalink
Revert "Cleanup program building a bit"
Browse files Browse the repository at this point in the history
This reverts commit 4777e3a.

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>
  • Loading branch information
austinEng authored and Skia Commit-Bot committed Feb 7, 2020
1 parent f3560b6 commit 77fdf66
Show file tree
Hide file tree
Showing 16 changed files with 117 additions and 102 deletions.
2 changes: 1 addition & 1 deletion src/gpu/gl/GrGLGpu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1817,7 +1817,7 @@ void GrGLGpu::disableWindowRectangles() {

bool GrGLGpu::flushGLState(GrRenderTarget* renderTarget, const GrProgramInfo& programInfo) {

sk_sp<GrGLProgram> program(fProgramCache->findOrCreateProgram(renderTarget, programInfo));
sk_sp<GrGLProgram> program(fProgramCache->refProgram(this, renderTarget, programInfo));
if (!program) {
GrCapsDebugf(this->caps(), "Failed to create program!\n");
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/gpu/gl/GrGLGpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ class GrGLGpu final : public GrGpu, private GrMesh::SendToGpuImpl {

void abandon();
void reset();
sk_sp<GrGLProgram> findOrCreateProgram(GrRenderTarget*, const GrProgramInfo&);
GrGLProgram* refProgram(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&);
bool precompileShader(const SkData& key, const SkData& data);

private:
Expand Down
27 changes: 15 additions & 12 deletions src/gpu/gl/GrGLGpuProgramCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,14 @@ void GrGLGpu::ProgramCache::reset() {
fMap.reset();
}

sk_sp<GrGLProgram> GrGLGpu::ProgramCache::findOrCreateProgram(GrRenderTarget* renderTarget,
const GrProgramInfo& programInfo) {
const GrCaps& caps = *fGpu->caps();
GrGLProgram* GrGLGpu::ProgramCache::refProgram(GrGLGpu* gpu,
GrRenderTarget* renderTarget,
const GrProgramInfo& programInfo) {
const GrCaps& caps = *gpu->caps();

GrProgramDesc desc = caps.makeDesc(renderTarget, programInfo);
if (!desc.isValid()) {
GrCapsDebugf(fGpu->caps(), "Failed to gl program descriptor!\n");
GrCapsDebugf(gpu->caps(), "Failed to gl program descriptor!\n");
return nullptr;
}

Expand All @@ -60,24 +61,26 @@ sk_sp<GrGLProgram> GrGLGpu::ProgramCache::findOrCreateProgram(GrRenderTarget* re
// We've pre-compiled the GL program, but don't have the GrGLProgram scaffolding
const GrGLPrecompiledProgram* precompiledProgram = &((*entry)->fPrecompiledProgram);
SkASSERT(precompiledProgram->fProgramID != 0);
(*entry)->fProgram = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget, desc,
programInfo, precompiledProgram);
if (!(*entry)->fProgram) {
GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
&desc, fGpu,
precompiledProgram);
if (nullptr == program) {
// 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
sk_sp<GrGLProgram> program = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget,
desc, programInfo);
if (!program) {
GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
&desc, fGpu);
if (nullptr == program) {
return nullptr;
}
entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(std::move(program))));
entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(sk_sp<GrGLProgram>(program))));
}

return (*entry)->fProgram;
return SkRef((*entry)->fProgram.get());
}

bool GrGLGpu::ProgramCache::precompileShader(const SkData& key, const SkData& data) {
Expand Down
53 changes: 26 additions & 27 deletions src/gpu/gl/builders/GrGLProgramBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,21 @@ static void cleanup_program(GrGLGpu* gpu, GrGLuint programID,
cleanup_shaders(gpu, shaderIDs);
}

sk_sp<GrGLProgram> GrGLProgramBuilder::CreateProgram(
GrGLGpu* gpu,
GrRenderTarget* renderTarget,
const GrProgramDesc& desc,
GrGLProgram* GrGLProgramBuilder::CreateProgram(GrRenderTarget* renderTarget,
const GrProgramInfo& programInfo,
GrProgramDesc* desc,
GrGLGpu* gpu,
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, desc, programInfo);
GrGLProgramBuilder builder(gpu, renderTarget, programInfo, desc);

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
Expand All @@ -76,9 +75,9 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::CreateProgram(

GrGLProgramBuilder::GrGLProgramBuilder(GrGLGpu* gpu,
GrRenderTarget* renderTarget,
const GrProgramDesc& desc,
const GrProgramInfo& programInfo)
: INHERITED(renderTarget, desc, programInfo)
const GrProgramInfo& programInfo,
GrProgramDesc* desc)
: INHERITED(renderTarget, programInfo, desc)
, fGpu(gpu)
, fVaryingHandler(this)
, fUniformHandler(this)
Expand Down Expand Up @@ -160,7 +159,7 @@ void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs,
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;
Expand Down Expand Up @@ -199,7 +198,7 @@ void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs,
}
}

sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
GrGLProgram* GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
TRACE_EVENT0("skia.gpu", TRACE_FUNC);

// verify we can get a program id
Expand Down Expand Up @@ -542,22 +541,22 @@ void GrGLProgramBuilder::resolveProgramResourceLocations(GrGLuint programID, boo
}
}

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));
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);
}

bool GrGLProgramBuilder::PrecompileProgram(GrGLPrecompiledProgram* precompiledProgram,
Expand Down
21 changes: 12 additions & 9 deletions src/gpu/gl/builders/GrGLProgramBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,18 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
* 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 the created program if generation was successful.
* @return true if generation was successful.
*/
static sk_sp<GrGLProgram> CreateProgram(GrGLGpu*,
GrRenderTarget*,
const GrProgramDesc&,
const GrProgramInfo&,
const GrGLPrecompiledProgram* = nullptr);
static GrGLProgram* CreateProgram(GrRenderTarget*,
const GrProgramInfo&,
GrProgramDesc*,
GrGLGpu*,
const GrGLPrecompiledProgram* = nullptr);

static bool PrecompileProgram(GrGLPrecompiledProgram*, GrGLGpu*, const SkData&);

Expand All @@ -57,7 +60,7 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
GrGLGpu* gpu() const { return fGpu; }

private:
GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);

void addInputVars(const SkSL::Program::Inputs& inputs);
bool compileAndAttachShaders(const SkSL::String& glsl,
Expand All @@ -71,14 +74,14 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
void storeShaderInCache(const SkSL::Program::Inputs& inputs, GrGLuint programID,
const SkSL::String shaders[], bool isSkSL,
SkSL::Program::Settings* settings);
sk_sp<GrGLProgram> finalize(const GrGLPrecompiledProgram*);
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
sk_sp<GrGLProgram> createProgram(GrGLuint programID);
GrGLProgram* createProgram(GrGLuint programID);

GrGLSLUniformHandler* uniformHandler() override { return &fUniformHandler; }
const GrGLSLUniformHandler* uniformHandler() const override { return &fUniformHandler; }
Expand Down
6 changes: 3 additions & 3 deletions src/gpu/glsl/GrGLSLProgramBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
const int GrGLSLProgramBuilder::kVarsPerBlock = 8;

GrGLSLProgramBuilder::GrGLSLProgramBuilder(GrRenderTarget* renderTarget,
const GrProgramDesc& desc,
const GrProgramInfo& programInfo)
const GrProgramInfo& programInfo,
const GrProgramDesc* desc)
: fVS(this)
, fGS(this)
, fFS(this)
, fStageIndex(-1)
, fRenderTarget(renderTarget)
, fDesc(desc)
, fProgramInfo(programInfo)
, fDesc(desc)
, fGeometryProcessor(nullptr)
, fXferProcessor(nullptr)
, fNumFragmentSamplers(0) {}
Expand Down
7 changes: 4 additions & 3 deletions src/gpu/glsl/GrGLSLProgramBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class GrGLSLProgramBuilder {
return fRenderTarget->renderTargetPriv().getSampleLocations();
}

const GrProgramDesc& desc() const { return fDesc; }
const GrProgramDesc* desc() const { return fDesc; }

void appendUniformDecls(GrShaderFlags visibility, SkString*) const;

Expand Down Expand Up @@ -104,9 +104,10 @@ class GrGLSLProgramBuilder {
int fStageIndex;

const GrRenderTarget* fRenderTarget; // TODO: remove this
const GrProgramDesc& fDesc;
const GrProgramInfo& fProgramInfo;

const GrProgramDesc* fDesc;

GrGLSLBuiltinUniformHandles fUniformHandles;

std::unique_ptr<GrGLSLPrimitiveProcessor> fGeometryProcessor;
Expand All @@ -115,7 +116,7 @@ class GrGLSLProgramBuilder {
int fFragmentProcessorCnt;

protected:
explicit GrGLSLProgramBuilder(GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
explicit GrGLSLProgramBuilder(GrRenderTarget*, const GrProgramInfo&, const GrProgramDesc*);

void addFeature(GrShaderFlags shaders, uint32_t featureBit, const char* extensionName);

Expand Down
15 changes: 8 additions & 7 deletions src/gpu/mtl/GrMtlPipelineStateBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ class GrMtlPipelineStateBuilder : public GrGLSLProgramBuilder {
*
* The GrMtlPipelineState implements what is specified in the GrPipeline and
* GrPrimitiveProcessor as input. After successful generation, the builder result objects are
* available to be used.
* @return the created pipeline if generation was successful; nullptr otherwise
* available to be used. This function may modify the program key by setting the surface origin
* key to 0 (unspecified) if it turns out the program does not care about the surface origin.
* @return true if generation was successful.
*/
static GrMtlPipelineState* CreatePipelineState(GrMtlGpu*,
GrRenderTarget*,
const GrProgramDesc&,
const GrProgramInfo&);
const GrProgramInfo&,
GrProgramDesc*);

private:
GrMtlPipelineStateBuilder(GrMtlGpu*, GrRenderTarget*,
const GrProgramDesc&, const GrProgramInfo&);
GrMtlPipelineStateBuilder(GrMtlGpu*, GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);

GrMtlPipelineState* finalize(GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
GrMtlPipelineState* finalize(GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);

const GrCaps* caps() const override;

Expand All @@ -52,6 +52,7 @@ class GrMtlPipelineStateBuilder : public GrGLSLProgramBuilder {
id<MTLLibrary> generateMtlShaderLibrary(const SkSL::String& sksl,
SkSL::Program::Kind kind,
const SkSL::Program::Settings& settings,
GrProgramDesc* desc,
SkSL::String* msl,
SkSL::Program::Inputs* inputs);
id<MTLLibrary> compileMtlShaderLibrary(const SkSL::String& shader,
Expand Down
Loading

0 comments on commit 77fdf66

Please sign in to comment.