Skip to content

Commit 77fdf66

Browse files
austinEngSkia Commit-Bot
authored andcommitted
Revert "Cleanup program building a bit"
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 <[email protected]> > Commit-Queue: Robert Phillips <[email protected]> [email protected],[email protected] # 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 <[email protected]> Reviewed-by: Kenneth Russell <[email protected]> Reviewed-by: Stephen White <[email protected]> Commit-Queue: Stephen White <[email protected]> Auto-Submit: Austin Eng <[email protected]>
1 parent f3560b6 commit 77fdf66

16 files changed

+117
-102
lines changed

src/gpu/gl/GrGLGpu.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1817,7 +1817,7 @@ void GrGLGpu::disableWindowRectangles() {
18171817

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

1820-
sk_sp<GrGLProgram> program(fProgramCache->findOrCreateProgram(renderTarget, programInfo));
1820+
sk_sp<GrGLProgram> program(fProgramCache->refProgram(this, renderTarget, programInfo));
18211821
if (!program) {
18221822
GrCapsDebugf(this->caps(), "Failed to create program!\n");
18231823
return false;

src/gpu/gl/GrGLGpu.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ class GrGLGpu final : public GrGpu, private GrMesh::SendToGpuImpl {
316316

317317
void abandon();
318318
void reset();
319-
sk_sp<GrGLProgram> findOrCreateProgram(GrRenderTarget*, const GrProgramInfo&);
319+
GrGLProgram* refProgram(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&);
320320
bool precompileShader(const SkData& key, const SkData& data);
321321

322322
private:

src/gpu/gl/GrGLGpuProgramCache.cpp

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,14 @@ void GrGLGpu::ProgramCache::reset() {
4545
fMap.reset();
4646
}
4747

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

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

@@ -60,24 +61,26 @@ sk_sp<GrGLProgram> GrGLGpu::ProgramCache::findOrCreateProgram(GrRenderTarget* re
6061
// We've pre-compiled the GL program, but don't have the GrGLProgram scaffolding
6162
const GrGLPrecompiledProgram* precompiledProgram = &((*entry)->fPrecompiledProgram);
6263
SkASSERT(precompiledProgram->fProgramID != 0);
63-
(*entry)->fProgram = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget, desc,
64-
programInfo, precompiledProgram);
65-
if (!(*entry)->fProgram) {
64+
GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
65+
&desc, fGpu,
66+
precompiledProgram);
67+
if (nullptr == program) {
6668
// Should we purge the program ID from the cache at this point?
6769
SkDEBUGFAIL("Couldn't create program from precompiled program");
6870
return nullptr;
6971
}
72+
(*entry)->fProgram.reset(program);
7073
} else if (!entry) {
7174
// We have a cache miss
72-
sk_sp<GrGLProgram> program = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget,
73-
desc, programInfo);
74-
if (!program) {
75+
GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
76+
&desc, fGpu);
77+
if (nullptr == program) {
7578
return nullptr;
7679
}
77-
entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(std::move(program))));
80+
entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(sk_sp<GrGLProgram>(program))));
7881
}
7982

80-
return (*entry)->fProgram;
83+
return SkRef((*entry)->fProgram.get());
8184
}
8285

8386
bool GrGLGpu::ProgramCache::precompileShader(const SkData& key, const SkData& data) {

src/gpu/gl/builders/GrGLProgramBuilder.cpp

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,21 @@ static void cleanup_program(GrGLGpu* gpu, GrGLuint programID,
4545
cleanup_shaders(gpu, shaderIDs);
4646
}
4747

48-
sk_sp<GrGLProgram> GrGLProgramBuilder::CreateProgram(
49-
GrGLGpu* gpu,
50-
GrRenderTarget* renderTarget,
51-
const GrProgramDesc& desc,
48+
GrGLProgram* GrGLProgramBuilder::CreateProgram(GrRenderTarget* renderTarget,
5249
const GrProgramInfo& programInfo,
50+
GrProgramDesc* desc,
51+
GrGLGpu* gpu,
5352
const GrGLPrecompiledProgram* precompiledProgram) {
5453
ATRACE_ANDROID_FRAMEWORK_ALWAYS("shader_compile");
5554
GrAutoLocaleSetter als("C");
5655

5756
// create a builder. This will be handed off to effects so they can use it to add
5857
// uniforms, varyings, textures, etc
59-
GrGLProgramBuilder builder(gpu, renderTarget, desc, programInfo);
58+
GrGLProgramBuilder builder(gpu, renderTarget, programInfo, desc);
6059

6160
auto persistentCache = gpu->getContext()->priv().getPersistentCache();
6261
if (persistentCache && !precompiledProgram) {
63-
sk_sp<SkData> key = SkData::MakeWithoutCopy(desc.asKey(), desc.keyLength());
62+
sk_sp<SkData> key = SkData::MakeWithoutCopy(desc->asKey(), desc->keyLength());
6463
builder.fCached = persistentCache->load(*key);
6564
// the eventual end goal is to completely skip emitAndInstallProcs on a cache hit, but it's
6665
// doing necessary setup in addition to generating the SkSL code. Currently we are only able
@@ -76,9 +75,9 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::CreateProgram(
7675

7776
GrGLProgramBuilder::GrGLProgramBuilder(GrGLGpu* gpu,
7877
GrRenderTarget* renderTarget,
79-
const GrProgramDesc& desc,
80-
const GrProgramInfo& programInfo)
81-
: INHERITED(renderTarget, desc, programInfo)
78+
const GrProgramInfo& programInfo,
79+
GrProgramDesc* desc)
80+
: INHERITED(renderTarget, programInfo, desc)
8281
, fGpu(gpu)
8382
, fVaryingHandler(this)
8483
, fUniformHandler(this)
@@ -160,7 +159,7 @@ void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs,
160159
if (!this->gpu()->getContext()->priv().getPersistentCache()) {
161160
return;
162161
}
163-
sk_sp<SkData> key = SkData::MakeWithoutCopy(this->desc().asKey(), this->desc().keyLength());
162+
sk_sp<SkData> key = SkData::MakeWithoutCopy(this->desc()->asKey(), this->desc()->keyLength());
164163
if (fGpu->glCaps().programBinarySupport()) {
165164
// binary cache
166165
GrGLsizei length = 0;
@@ -199,7 +198,7 @@ void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs,
199198
}
200199
}
201200

202-
sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
201+
GrGLProgram* GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
203202
TRACE_EVENT0("skia.gpu", TRACE_FUNC);
204203

205204
// verify we can get a program id
@@ -542,22 +541,22 @@ void GrGLProgramBuilder::resolveProgramResourceLocations(GrGLuint programID, boo
542541
}
543542
}
544543

545-
sk_sp<GrGLProgram> GrGLProgramBuilder::createProgram(GrGLuint programID) {
546-
return sk_sp<GrGLProgram>(new GrGLProgram(fGpu,
547-
fUniformHandles,
548-
programID,
549-
fUniformHandler.fUniforms,
550-
fUniformHandler.fSamplers,
551-
fVaryingHandler.fPathProcVaryingInfos,
552-
std::move(fGeometryProcessor),
553-
std::move(fXferProcessor),
554-
std::move(fFragmentProcessors),
555-
fFragmentProcessorCnt,
556-
std::move(fAttributes),
557-
fVertexAttributeCnt,
558-
fInstanceAttributeCnt,
559-
fVertexStride,
560-
fInstanceStride));
544+
GrGLProgram* GrGLProgramBuilder::createProgram(GrGLuint programID) {
545+
return new GrGLProgram(fGpu,
546+
fUniformHandles,
547+
programID,
548+
fUniformHandler.fUniforms,
549+
fUniformHandler.fSamplers,
550+
fVaryingHandler.fPathProcVaryingInfos,
551+
std::move(fGeometryProcessor),
552+
std::move(fXferProcessor),
553+
std::move(fFragmentProcessors),
554+
fFragmentProcessorCnt,
555+
std::move(fAttributes),
556+
fVertexAttributeCnt,
557+
fInstanceAttributeCnt,
558+
fVertexStride,
559+
fInstanceStride);
561560
}
562561

563562
bool GrGLProgramBuilder::PrecompileProgram(GrGLPrecompiledProgram* precompiledProgram,

src/gpu/gl/builders/GrGLProgramBuilder.h

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,18 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
4040
* The program implements what is specified in the stages given as input.
4141
* After successful generation, the builder result objects are available
4242
* to be used.
43+
* This function may modify the GrProgramDesc by setting the surface origin
44+
* key to 0 (unspecified) if it turns out the program does not care about
45+
* the surface origin.
4346
* If a GL program has already been created, the program ID and inputs can
4447
* be supplied to skip the shader compilation.
45-
* @return the created program if generation was successful.
48+
* @return true if generation was successful.
4649
*/
47-
static sk_sp<GrGLProgram> CreateProgram(GrGLGpu*,
48-
GrRenderTarget*,
49-
const GrProgramDesc&,
50-
const GrProgramInfo&,
51-
const GrGLPrecompiledProgram* = nullptr);
50+
static GrGLProgram* CreateProgram(GrRenderTarget*,
51+
const GrProgramInfo&,
52+
GrProgramDesc*,
53+
GrGLGpu*,
54+
const GrGLPrecompiledProgram* = nullptr);
5255

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

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

5962
private:
60-
GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
63+
GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);
6164

6265
void addInputVars(const SkSL::Program::Inputs& inputs);
6366
bool compileAndAttachShaders(const SkSL::String& glsl,
@@ -71,14 +74,14 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
7174
void storeShaderInCache(const SkSL::Program::Inputs& inputs, GrGLuint programID,
7275
const SkSL::String shaders[], bool isSkSL,
7376
SkSL::Program::Settings* settings);
74-
sk_sp<GrGLProgram> finalize(const GrGLPrecompiledProgram*);
77+
GrGLProgram* finalize(const GrGLPrecompiledProgram*);
7578
void bindProgramResourceLocations(GrGLuint programID);
7679
bool checkLinkStatus(GrGLuint programID, GrContextOptions::ShaderErrorHandler* errorHandler,
7780
SkSL::String* sksl[], const SkSL::String glsl[]);
7881
void resolveProgramResourceLocations(GrGLuint programID, bool force);
7982

8083
// Subclasses create different programs
81-
sk_sp<GrGLProgram> createProgram(GrGLuint programID);
84+
GrGLProgram* createProgram(GrGLuint programID);
8285

8386
GrGLSLUniformHandler* uniformHandler() override { return &fUniformHandler; }
8487
const GrGLSLUniformHandler* uniformHandler() const override { return &fUniformHandler; }

src/gpu/glsl/GrGLSLProgramBuilder.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@
2121
const int GrGLSLProgramBuilder::kVarsPerBlock = 8;
2222

2323
GrGLSLProgramBuilder::GrGLSLProgramBuilder(GrRenderTarget* renderTarget,
24-
const GrProgramDesc& desc,
25-
const GrProgramInfo& programInfo)
24+
const GrProgramInfo& programInfo,
25+
const GrProgramDesc* desc)
2626
: fVS(this)
2727
, fGS(this)
2828
, fFS(this)
2929
, fStageIndex(-1)
3030
, fRenderTarget(renderTarget)
31-
, fDesc(desc)
3231
, fProgramInfo(programInfo)
32+
, fDesc(desc)
3333
, fGeometryProcessor(nullptr)
3434
, fXferProcessor(nullptr)
3535
, fNumFragmentSamplers(0) {}

src/gpu/glsl/GrGLSLProgramBuilder.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class GrGLSLProgramBuilder {
5757
return fRenderTarget->renderTargetPriv().getSampleLocations();
5858
}
5959

60-
const GrProgramDesc& desc() const { return fDesc; }
60+
const GrProgramDesc* desc() const { return fDesc; }
6161

6262
void appendUniformDecls(GrShaderFlags visibility, SkString*) const;
6363

@@ -104,9 +104,10 @@ class GrGLSLProgramBuilder {
104104
int fStageIndex;
105105

106106
const GrRenderTarget* fRenderTarget; // TODO: remove this
107-
const GrProgramDesc& fDesc;
108107
const GrProgramInfo& fProgramInfo;
109108

109+
const GrProgramDesc* fDesc;
110+
110111
GrGLSLBuiltinUniformHandles fUniformHandles;
111112

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

117118
protected:
118-
explicit GrGLSLProgramBuilder(GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
119+
explicit GrGLSLProgramBuilder(GrRenderTarget*, const GrProgramInfo&, const GrProgramDesc*);
119120

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

src/gpu/mtl/GrMtlPipelineStateBuilder.h

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,19 @@ class GrMtlPipelineStateBuilder : public GrGLSLProgramBuilder {
2929
*
3030
* The GrMtlPipelineState implements what is specified in the GrPipeline and
3131
* GrPrimitiveProcessor as input. After successful generation, the builder result objects are
32-
* available to be used.
33-
* @return the created pipeline if generation was successful; nullptr otherwise
32+
* available to be used. This function may modify the program key by setting the surface origin
33+
* key to 0 (unspecified) if it turns out the program does not care about the surface origin.
34+
* @return true if generation was successful.
3435
*/
3536
static GrMtlPipelineState* CreatePipelineState(GrMtlGpu*,
3637
GrRenderTarget*,
37-
const GrProgramDesc&,
38-
const GrProgramInfo&);
38+
const GrProgramInfo&,
39+
GrProgramDesc*);
3940

4041
private:
41-
GrMtlPipelineStateBuilder(GrMtlGpu*, GrRenderTarget*,
42-
const GrProgramDesc&, const GrProgramInfo&);
42+
GrMtlPipelineStateBuilder(GrMtlGpu*, GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);
4343

44-
GrMtlPipelineState* finalize(GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
44+
GrMtlPipelineState* finalize(GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);
4545

4646
const GrCaps* caps() const override;
4747

@@ -52,6 +52,7 @@ class GrMtlPipelineStateBuilder : public GrGLSLProgramBuilder {
5252
id<MTLLibrary> generateMtlShaderLibrary(const SkSL::String& sksl,
5353
SkSL::Program::Kind kind,
5454
const SkSL::Program::Settings& settings,
55+
GrProgramDesc* desc,
5556
SkSL::String* msl,
5657
SkSL::Program::Inputs* inputs);
5758
id<MTLLibrary> compileMtlShaderLibrary(const SkSL::String& shader,

0 commit comments

Comments
 (0)