Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Revert "Move GL's SkSL::Compiler to the GPU (like all other backends)"
Browse files Browse the repository at this point in the history
This reverts commit cddfce2.

Reason for revert: Failing Flutter unit tests with *certain* test orderings.

Original change's description:
> Move GL's SkSL::Compiler to the GPU (like all other backends)
>
> This was the only backend that didn't store the compiler on the GrGpu,
> and also the only one that did lazy-instantiation. Trying to standardize
> this code a bit.
>
> Change-Id: Ibdd1bcc2dc9c3756b46a4c6f0543b5bb20fe135d
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337716
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,michaelludwig@google.com,johnstiles@google.com

Change-Id: I043ad395472fe20addcc59784aefe9061dae02ba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338039
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
brianosman authored and Skia Commit-Bot committed Nov 24, 2020
1 parent 3df7196 commit 68ac3b9
Showing 7 changed files with 31 additions and 25 deletions.
12 changes: 11 additions & 1 deletion src/gpu/gl/GrGLContext.cpp
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@

#include "src/gpu/gl/GrGLContext.h"
#include "src/gpu/gl/GrGLGLSL.h"
#include "src/sksl/SkSLCompiler.h"

#ifdef SK_BUILD_FOR_ANDROID
#include <sys/system_properties.h>
@@ -89,7 +90,16 @@ std::unique_ptr<GrGLContext> GrGLContext::Make(sk_sp<const GrGLInterface> interf
return std::unique_ptr<GrGLContext>(new GrGLContext(std::move(args)));
}

GrGLContext::~GrGLContext() {}
GrGLContext::~GrGLContext() {
delete fCompiler;
}

SkSL::Compiler* GrGLContext::compiler() const {
if (!fCompiler) {
fCompiler = new SkSL::Compiler(fGLCaps->shaderCaps());
}
return fCompiler;
}

GrGLContextInfo::GrGLContextInfo(ConstructorArgs&& args) {
fInterface = std::move(args.fInterface);
11 changes: 9 additions & 2 deletions src/gpu/gl/GrGLContext.h
Original file line number Diff line number Diff line change
@@ -16,6 +16,9 @@
#include "src/gpu/glsl/GrGLSL.h"

struct GrContextOptions;
namespace SkSL {
class Compiler;
} // namespace SkSL

/**
* Encapsulates information about an OpenGL context including the OpenGL
@@ -79,7 +82,7 @@ class GrGLContextInfo {
};

/**
* Extension of GrGLContextInfo that also provides access to GrGLInterface.
* Extension of GrGLContextInfo that also provides access to GrGLInterface and SkSL::Compiler.
*/
class GrGLContext : public GrGLContextInfo {
public:
@@ -91,10 +94,14 @@ class GrGLContext : public GrGLContextInfo {

const GrGLInterface* glInterface() const { return fInterface.get(); }

SkSL::Compiler* compiler() const;

~GrGLContext() override;

private:
GrGLContext(ConstructorArgs&& args) : INHERITED(std::move(args)) {}
GrGLContext(ConstructorArgs&& args) : INHERITED(std::move(args)), fCompiler(nullptr) {}

mutable SkSL::Compiler* fCompiler;

using INHERITED = GrGLContextInfo;
};
9 changes: 4 additions & 5 deletions src/gpu/gl/GrGLGpu.cpp
Original file line number Diff line number Diff line change
@@ -352,7 +352,6 @@ GrGLGpu::GrGLGpu(std::unique_ptr<GrGLContext> ctx, GrDirectContext* direct)
this->checkAndResetOOMed();

fCaps = sk_ref_sp(fGLContext->caps());
fCompiler = std::make_unique<SkSL::Compiler>(fCaps->shaderCaps());

fHWTextureUnitBindings.reset(this->numTextureUnits());

@@ -3049,14 +3048,14 @@ bool GrGLGpu::createCopyProgram(GrTexture* srcTex) {
SkSL::String sksl(vshaderTxt.c_str(), vshaderTxt.size());
SkSL::Program::Settings settings;
SkSL::String glsl;
std::unique_ptr<SkSL::Program> program = GrSkSLtoGLSL(this, SkSL::Program::kVertex_Kind,
std::unique_ptr<SkSL::Program> program = GrSkSLtoGLSL(*fGLContext, SkSL::Program::kVertex_Kind,
sksl, settings, &glsl, errorHandler);
GrGLuint vshader = GrGLCompileAndAttachShader(*fGLContext, fCopyPrograms[progIdx].fProgram,
GR_GL_VERTEX_SHADER, glsl, &fStats, errorHandler);
SkASSERT(program->fInputs.isEmpty());

sksl.assign(fshaderTxt.c_str(), fshaderTxt.size());
program = GrSkSLtoGLSL(this, SkSL::Program::kFragment_Kind, sksl, settings, &glsl,
program = GrSkSLtoGLSL(*fGLContext, SkSL::Program::kFragment_Kind, sksl, settings, &glsl,
errorHandler);
GrGLuint fshader = GrGLCompileAndAttachShader(*fGLContext, fCopyPrograms[progIdx].fProgram,
GR_GL_FRAGMENT_SHADER, glsl, &fStats,
@@ -3202,14 +3201,14 @@ bool GrGLGpu::createMipmapProgram(int progIdx) {
SkSL::String sksl(vshaderTxt.c_str(), vshaderTxt.size());
SkSL::Program::Settings settings;
SkSL::String glsl;
std::unique_ptr<SkSL::Program> program = GrSkSLtoGLSL(this, SkSL::Program::kVertex_Kind,
std::unique_ptr<SkSL::Program> program = GrSkSLtoGLSL(*fGLContext, SkSL::Program::kVertex_Kind,
sksl, settings, &glsl, errorHandler);
GrGLuint vshader = GrGLCompileAndAttachShader(*fGLContext, fMipmapPrograms[progIdx].fProgram,
GR_GL_VERTEX_SHADER, glsl, &fStats, errorHandler);
SkASSERT(program->fInputs.isEmpty());

sksl.assign(fshaderTxt.c_str(), fshaderTxt.size());
program = GrSkSLtoGLSL(this, SkSL::Program::kFragment_Kind, sksl, settings, &glsl,
program = GrSkSLtoGLSL(*fGLContext, SkSL::Program::kFragment_Kind, sksl, settings, &glsl,
errorHandler);
GrGLuint fshader = GrGLCompileAndAttachShader(*fGLContext, fMipmapPrograms[progIdx].fProgram,
GR_GL_FRAGMENT_SHADER, glsl, &fStats,
10 changes: 0 additions & 10 deletions src/gpu/gl/GrGLGpu.h
Original file line number Diff line number Diff line change
@@ -30,10 +30,6 @@ class GrGLOpsRenderPass;
class GrPipeline;
class GrSwizzle;

namespace SkSL {
class Compiler;
}

class GrGLGpu final : public GrGpu {
public:
static sk_sp<GrGpu> Make(sk_sp<const GrGLInterface>, const GrContextOptions&, GrDirectContext*);
@@ -197,10 +193,6 @@ class GrGLGpu final : public GrGpu {
// Version for programs that aren't GrGLProgram.
void flushProgram(GrGLuint);

SkSL::Compiler* shaderCompiler() const {
return fCompiler.get();
}

private:
GrGLGpu(std::unique_ptr<GrGLContext>, GrDirectContext*);

@@ -521,8 +513,6 @@ class GrGLGpu final : public GrGpu {
// GL program-related state
std::unique_ptr<ProgramCache> fProgramCache;

std::unique_ptr<SkSL::Compiler> fCompiler;

///////////////////////////////////////////////////////////////////////////
///@name Caching of GL State
///@{
8 changes: 4 additions & 4 deletions src/gpu/gl/builders/GrGLProgramBuilder.cpp
Original file line number Diff line number Diff line change
@@ -329,7 +329,7 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr
if (fFS.fForceHighPrecision) {
settings.fForceHighPrecision = true;
}
std::unique_ptr<SkSL::Program> fs = GrSkSLtoGLSL(this->gpu(),
std::unique_ptr<SkSL::Program> fs = GrSkSLtoGLSL(gpu()->glContext(),
SkSL::Program::kFragment_Kind,
*sksl[kFragment_GrShaderType],
settings,
@@ -354,7 +354,7 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr
*/
if (glsl[kVertex_GrShaderType].empty()) {
// Don't have cached GLSL, need to compile SkSL->GLSL
std::unique_ptr<SkSL::Program> vs = GrSkSLtoGLSL(this->gpu(),
std::unique_ptr<SkSL::Program> vs = GrSkSLtoGLSL(gpu()->glContext(),
SkSL::Program::kVertex_Kind,
*sksl[kVertex_GrShaderType],
settings,
@@ -418,7 +418,7 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr
if (glsl[kGeometry_GrShaderType].empty()) {
// Don't have cached GLSL, need to compile SkSL->GLSL
std::unique_ptr<SkSL::Program> gs;
gs = GrSkSLtoGLSL(this->gpu(),
gs = GrSkSLtoGLSL(gpu()->glContext(),
SkSL::Program::kGeometry_Kind,
*sksl[kGeometry_GrShaderType],
settings,
@@ -601,7 +601,7 @@ bool GrGLProgramBuilder::PrecompileProgram(GrGLPrecompiledProgram* precompiledPr

auto compileShader = [&](SkSL::Program::Kind kind, const SkSL::String& sksl, GrGLenum type) {
SkSL::String glsl;
auto program = GrSkSLtoGLSL(gpu, kind, sksl, settings, &glsl, errorHandler);
auto program = GrSkSLtoGLSL(gpu->glContext(), kind, sksl, settings, &glsl, errorHandler);
if (!program) {
return false;
}
4 changes: 2 additions & 2 deletions src/gpu/gl/builders/GrGLShaderStringBuilder.cpp
Original file line number Diff line number Diff line change
@@ -17,13 +17,13 @@
static const bool gPrintSKSL = false;
static const bool gPrintGLSL = false;

std::unique_ptr<SkSL::Program> GrSkSLtoGLSL(const GrGLGpu* gpu,
std::unique_ptr<SkSL::Program> GrSkSLtoGLSL(const GrGLContext& context,
SkSL::Program::Kind programKind,
const SkSL::String& sksl,
const SkSL::Program::Settings& settings,
SkSL::String* glsl,
GrContextOptions::ShaderErrorHandler* errorHandler) {
SkSL::Compiler* compiler = gpu->shaderCompiler();
SkSL::Compiler* compiler = context.compiler();
std::unique_ptr<SkSL::Program> program;
#ifdef SK_DEBUG
SkSL::String src = GrShaderUtils::PrettyPrint(sksl);
2 changes: 1 addition & 1 deletion src/gpu/gl/builders/GrGLShaderStringBuilder.h
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@
#include "src/gpu/gl/GrGLContext.h"
#include "src/sksl/SkSLGLSLCodeGenerator.h"

std::unique_ptr<SkSL::Program> GrSkSLtoGLSL(const GrGLGpu* gpu,
std::unique_ptr<SkSL::Program> GrSkSLtoGLSL(const GrGLContext& context,
SkSL::Program::Kind programKind,
const SkSL::String& sksl,
const SkSL::Program::Settings& settings,

0 comments on commit 68ac3b9

Please sign in to comment.