From fc296295257a9300098df56a1e3975580e13d329 Mon Sep 17 00:00:00 2001 From: "bsalomon@google.com" Date: Fri, 6 May 2011 13:53:47 +0000 Subject: [PATCH] Replace GrStringBuilder with SkString. First step in cleaning up the shader generator. Slight performance hit when creating a new shader (<10% of total shader gen time on my Windows box is spent in building our string before handing it to GL). Much of this can be recovered by better usage pattern of SkString in coming revisions. Review URL: http://codereview.appspot.com/4465053/ git-svn-id: http://skia.googlecode.com/svn/trunk@1266 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gpu/include/GrStringBuilder.h | 160 +----------------------- gpu/src/GrGLEffect.h | 12 +- gpu/src/GrGLProgram.cpp | 121 +++++++++--------- gpu/src/gr_files.mk | 1 - gyp/skia.gyp | 4 +- include/core/SkString.h | 5 + src/core/SkString.cpp | 9 ++ xcode/gpu/gpu.xcodeproj/project.pbxproj | 4 + 8 files changed, 88 insertions(+), 228 deletions(-) diff --git a/gpu/include/GrStringBuilder.h b/gpu/include/GrStringBuilder.h index bcf124f337..73b3796e31 100644 --- a/gpu/include/GrStringBuilder.h +++ b/gpu/include/GrStringBuilder.h @@ -18,165 +18,9 @@ #ifndef GrStringBuilder_DEFINED #define GrStringBuilder_DEFINED -#include -#include +#include "SkString.h" -// Class used to concat strings together into a single string -// See below for GrSStringBuilder subclass that has a pool of -// stack storage (to avoid malloc). -class GrStringBuilder { -public: - GrStringBuilder() : - fChars() { - fChars.push_back() = '\0'; - } - - GrStringBuilder(const GrStringBuilder& s) : - fChars(s.fChars) { - GrAssert('\0' == s.fChars.back()); - } - - GrStringBuilder(const char* s) : - fChars(s, strlen(s)+1) { - } - - GrStringBuilder(const GrStringBuilder& a, const GrStringBuilder& b) { - GrAssert('\0' == a.fChars.back()); - GrAssert('\0' == b.fChars.back()); - - fChars.push_back_n(a.fChars.count() + b.fChars.count() - 1); - char* s = &fChars.front(); - memcpy(s, &a.fChars.front(), a.fChars.count() - 1); - s += a.fChars.count() - 1; - memcpy(s, &b.fChars.front(), b.fChars.count()); - } - - GrStringBuilder& operator =(const GrStringBuilder& s) { - fChars = s.fChars; - return *this; - } - - GrStringBuilder& operator =(const char* s) { - GrAssert('\0' == fChars.back()); - - int l = strlen(s); - fChars.resize_back(l + 1); - memcpy(&fChars.front(), s, l + 1); - return *this; - } - - GrStringBuilder& operator +=(const GrStringBuilder& s) { - GrAssert('\0' == fChars.back()); - GrAssert('\0' == s.fChars.back()); - fChars.push_back_n(s.length()); - memcpy(&fChars.fromBack(s.length()), &s.fChars.front(), s.fChars.count()); - return *this; - } - - GrStringBuilder& operator +=(const char* s) { - GrAssert('\0' == fChars.back()); - int l = strlen(s); - fChars.push_back_n(l); - memcpy(&fChars.fromBack(l), s, l + 1); - return *this; - } - - GrStringBuilder& operator +=(char c) { - GrAssert('\0' == fChars.back()); - fChars.back() = c; - fChars.push_back() = '\0'; - return *this; - } - - void appendInt(int x) { - GR_STATIC_ASSERT(4 == sizeof(int)); - // -, 10 digits, null char - char temp[12]; - sprintf(temp, "%d", x); - *this += temp; - } - - char& operator [](int i) { - GrAssert(i < length()); - return fChars[i]; - } - - char operator [](int i) const { - GrAssert(i < length()); - return fChars[i]; - } - - const char* cstr() const { return &fChars.front(); } - - int length() const { return fChars.count() - 1; } - -protected: - // helpers for GrSStringBuilder (with storage on the stack) - - GrStringBuilder(void* stackChars, int stackCount) : - fChars(stackCount ? stackChars : NULL, - stackCount) { - fChars.push_back() = '\0'; - } - - GrStringBuilder(void* stackChars, - int stackCount, - const GrStringBuilder& s) : - fChars(s.fChars, - (stackCount ? stackChars : NULL), - stackCount) { - } - - GrStringBuilder(void* stackChars, - int stackCount, - const char* s) : - fChars(s, - strlen(s)+1, - stackCount ? stackChars : NULL, - stackCount) { - } - - GrStringBuilder(void* stackChars, - int stackCount, - const GrStringBuilder& a, - const GrStringBuilder& b) : - fChars(stackCount ? stackChars : NULL, - stackCount) { - GrAssert('\0' == a.fChars.back()); - GrAssert('\0' == b.fChars.back()); - - fChars.push_back_n(a.fChars.count() + b.fChars.count() - 1); - char* s = &fChars.front(); - memcpy(s, &a.fChars.front(), a.fChars.count() - 1); - s += a.fChars.count() - 1; - memcpy(s, &b.fChars.front(), b.fChars.count()); - } - -private: - GrTArray fChars; -}; - -template -class GrSStringBuilder : public GrStringBuilder { -public: - GrSStringBuilder() : GrStringBuilder(fStackChars, STACK_COUNT) {} - - GrSStringBuilder(const GrStringBuilder& s) : GrStringBuilder(fStackChars, - STACK_COUNT, - s) { - } - - GrSStringBuilder(const char* s) : GrStringBuilder(fStackChars, - STACK_COUNT, - s) { - } - - GrSStringBuilder(const GrStringBuilder& a, const GrStringBuilder& b) : - GrStringBuilder(fStackChars, STACK_COUNT, a, b) { - } -private: - char fStackChars[STACK_COUNT]; -}; +typedef SkString GrStringBuilder; #endif diff --git a/gpu/src/GrGLEffect.h b/gpu/src/GrGLEffect.h index 7c85b6dacf..ef00df8f4b 100644 --- a/gpu/src/GrGLEffect.h +++ b/gpu/src/GrGLEffect.h @@ -23,12 +23,12 @@ class GrEffect; struct ShaderCodeSegments { - GrSStringBuilder<256> fVSUnis; - GrSStringBuilder<256> fVSAttrs; - GrSStringBuilder<256> fVaryings; - GrSStringBuilder<256> fFSUnis; - GrSStringBuilder<512> fVSCode; - GrSStringBuilder<512> fFSCode; + GrStringBuilder fVSUnis; + GrStringBuilder fVSAttrs; + GrStringBuilder fVaryings; + GrStringBuilder fFSUnis; + GrStringBuilder fVSCode; + GrStringBuilder fFSCode; }; /** diff --git a/gpu/src/GrGLProgram.cpp b/gpu/src/GrGLProgram.cpp index ce4f00034f..76a7f102bf 100644 --- a/gpu/src/GrGLProgram.cpp +++ b/gpu/src/GrGLProgram.cpp @@ -55,11 +55,11 @@ const char* GrShaderPrecision() { #define COL_UNI_NAME "uColor" // for variable names etc -typedef GrSStringBuilder<16> GrTokenString; +typedef GrStringBuilder GrTokenString; static inline void tex_attr_name(int coordIdx, GrStringBuilder* s) { *s = "aTexCoord"; - s->appendInt(coordIdx); + s->appendS32(coordIdx); } static inline const char* float_vector_type(int count) { @@ -92,32 +92,32 @@ static void tex_matrix_name(int stage, GrStringBuilder* s) { #else *s = "uTexM"; #endif - s->appendInt(stage); + s->appendS32(stage); } static void normalized_texel_size_name(int stage, GrStringBuilder* s) { *s = "uTexelSize"; - s->appendInt(stage); + s->appendS32(stage); } static void sampler_name(int stage, GrStringBuilder* s) { *s = "uSampler"; - s->appendInt(stage); + s->appendS32(stage); } static void stage_varying_name(int stage, GrStringBuilder* s) { *s = "vStage"; - s->appendInt(stage); + s->appendS32(stage); } static void radial2_param_name(int stage, GrStringBuilder* s) { *s = "uRadial2Params"; - s->appendInt(stage); + s->appendS32(stage); } static void radial2_varying_name(int stage, GrStringBuilder* s) { *s = "vB"; - s->appendInt(stage); + s->appendS32(stage); } GrGLProgram::GrGLProgram() { @@ -247,8 +247,8 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { int tcIdx = GrDrawTarget::VertexTexCoordsForStage(s, layout); // we better have input tex coordinates if stage is enabled. GrAssert(tcIdx >= 0); - GrAssert(texCoordAttrs[tcIdx].length()); - stageInCoords[s] = texCoordAttrs[tcIdx].cstr(); + GrAssert(texCoordAttrs[tcIdx].size()); + stageInCoords[s] = texCoordAttrs[tcIdx].c_str(); } ++numActiveStages; } @@ -263,7 +263,7 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { GrTokenString outColor; if (currActiveStage < (numActiveStages - 1)) { outColor = "color"; - outColor.appendInt(currActiveStage); + outColor.appendS32(currActiveStage); segments.fFSCode += "\tvec4 "; segments.fFSCode += outColor; segments.fFSCode += ";\n"; @@ -273,8 +273,8 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { genStageCode(s, fProgramDesc.fStages[s], - inColor.length() ? inColor.cstr() : NULL, - outColor.cstr(), + inColor.size() ? inColor.c_str() : NULL, + outColor.c_str(), stageInCoords[s], &segments, &programData->fUniLocations.fStages[s]); @@ -284,7 +284,7 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { } } else { segments.fFSCode += "\tgl_FragColor = "; - if (inColor.length()) { + if (inColor.size()) { segments.fFSCode += inColor; } else { segments.fFSCode += "vec4(1,1,1,1)"; @@ -294,37 +294,36 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { segments.fFSCode += "}\n"; segments.fVSCode += "}\n"; - const char* strings[4]; int lengths[4]; int stringCnt = 0; - if (segments.fVSUnis.length()) { - strings[stringCnt] = segments.fVSUnis.cstr(); - lengths[stringCnt] = segments.fVSUnis.length(); + if (segments.fVSUnis.size()) { + strings[stringCnt] = segments.fVSUnis.c_str(); + lengths[stringCnt] = segments.fVSUnis.size(); ++stringCnt; } - if (segments.fVSAttrs.length()) { - strings[stringCnt] = segments.fVSAttrs.cstr(); - lengths[stringCnt] = segments.fVSAttrs.length(); + if (segments.fVSAttrs.size()) { + strings[stringCnt] = segments.fVSAttrs.c_str(); + lengths[stringCnt] = segments.fVSAttrs.size(); ++stringCnt; } - if (segments.fVaryings.length()) { - strings[stringCnt] = segments.fVaryings.cstr(); - lengths[stringCnt] = segments.fVaryings.length(); + if (segments.fVaryings.size()) { + strings[stringCnt] = segments.fVaryings.c_str(); + lengths[stringCnt] = segments.fVaryings.size(); ++stringCnt; } - GrAssert(segments.fVSCode.length()); - strings[stringCnt] = segments.fVSCode.cstr(); - lengths[stringCnt] = segments.fVSCode.length(); + GrAssert(segments.fVSCode.size()); + strings[stringCnt] = segments.fVSCode.c_str(); + lengths[stringCnt] = segments.fVSCode.size(); ++stringCnt; #if PRINT_SHADERS - GrPrintf(segments.fVSUnis.cstr()); - GrPrintf(segments.fVSAttrs.cstr()); - GrPrintf(segments.fVaryings.cstr()); - GrPrintf(segments.fVSCode.cstr()); + GrPrintf(segments.fVSUnis.c_str()); + GrPrintf(segments.fVSAttrs.c_str()); + GrPrintf(segments.fVaryings.c_str()); + GrPrintf(segments.fVSCode.c_str()); GrPrintf("\n"); #endif programData->fVShaderID = CompileShader(GR_GL_VERTEX_SHADER, @@ -339,27 +338,27 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { lengths[stringCnt] = strlen(GrShaderPrecision()); ++stringCnt; } - if (segments.fFSUnis.length()) { - strings[stringCnt] = segments.fFSUnis.cstr(); - lengths[stringCnt] = segments.fFSUnis.length(); + if (segments.fFSUnis.size()) { + strings[stringCnt] = segments.fFSUnis.c_str(); + lengths[stringCnt] = segments.fFSUnis.size(); ++stringCnt; } - if (segments.fVaryings.length()) { - strings[stringCnt] = segments.fVaryings.cstr(); - lengths[stringCnt] = segments.fVaryings.length(); + if (segments.fVaryings.size()) { + strings[stringCnt] = segments.fVaryings.c_str(); + lengths[stringCnt] = segments.fVaryings.size(); ++stringCnt; } - GrAssert(segments.fFSCode.length()); - strings[stringCnt] = segments.fFSCode.cstr(); - lengths[stringCnt] = segments.fFSCode.length(); + GrAssert(segments.fFSCode.size()); + strings[stringCnt] = segments.fFSCode.c_str(); + lengths[stringCnt] = segments.fFSCode.size(); ++stringCnt; #if PRINT_SHADERS GrPrintf(GrShaderPrecision()); - GrPrintf(segments.fFSUnis.cstr()); - GrPrintf(segments.fVaryings.cstr()); - GrPrintf(segments.fFSCode.cstr()); + GrPrintf(segments.fFSUnis.c_str()); + GrPrintf(segments.fVaryings.c_str()); + GrPrintf(segments.fFSCode.c_str()); GrPrintf("\n"); #endif programData->fFShaderID = CompileShader(GR_GL_FRAGMENT_SHADER, @@ -376,10 +375,10 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { // Bind the attrib locations to same values for all shaders GR_GL(BindAttribLocation(progID, POS_ATTR_LOCATION, POS_ATTR_NAME)); for (int t = 0; t < GrDrawTarget::kMaxTexCoords; ++t) { - if (texCoordAttrs[t].length()) { + if (texCoordAttrs[t].size()) { GR_GL(BindAttribLocation(progID, TEX_ATTR_LOCATION(t), - texCoordAttrs[t].cstr())); + texCoordAttrs[t].c_str())); } } @@ -398,7 +397,7 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { tex_matrix_name(s, &matName); GR_GL(BindAttribLocation(progID, TEXMAT_ATTR_LOCATION(s), - matName.cstr())); + matName.c_str())); program->fUniLocations.fStages[s].fTextureMatrixUni = BOGUS_MATRIX_UNI_LOCATION; } @@ -451,7 +450,7 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { tex_matrix_name(s, &texMName); locations.fTextureMatrixUni = GR_GL(GetUniformLocation( progID, - texMName.cstr())); + texMName.c_str())); GrAssert(-1 != locations.fTextureMatrixUni); } else { locations.fTextureMatrixUni = -1; @@ -464,7 +463,7 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { sampler_name(s, &samplerName); locations.fSamplerUni = GR_GL(GetUniformLocation( progID, - samplerName.cstr())); + samplerName.c_str())); GrAssert(-1 != locations.fSamplerUni); } else { locations.fSamplerUni = -1; @@ -474,7 +473,7 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { GrTokenString texelSizeName; normalized_texel_size_name(s, &texelSizeName); locations.fNormalizedTexelSizeUni = - GR_GL(GetUniformLocation(progID, texelSizeName.cstr())); + GR_GL(GetUniformLocation(progID, texelSizeName.c_str())); GrAssert(-1 != locations.fNormalizedTexelSizeUni); } else { locations.fNormalizedTexelSizeUni = -1; @@ -485,7 +484,7 @@ void GrGLProgram::genProgram(GrGLProgram::CachedData* programData) const { radial2_param_name(s, &radial2ParamName); locations.fRadial2Uni = GR_GL(GetUniformLocation( progID, - radial2ParamName.cstr())); + radial2ParamName.c_str())); GrAssert(-1 != locations.fRadial2Uni); } else { locations.fRadial2Uni = -1; @@ -698,7 +697,7 @@ void GrGLProgram::genStageCode(int stageNum, fsCoordName = varyingName; } else { fsCoordName = "inCoord"; - fsCoordName.appendInt(stageNum); + fsCoordName.appendS32(stageNum); segments->fFSCode += "\t"; segments->fFSCode += float_vector_type(coordDims); @@ -714,7 +713,7 @@ void GrGLProgram::genStageCode(int stageNum, } } - GrSStringBuilder<96> sampleCoords; + GrStringBuilder sampleCoords; bool complexCoord = false; switch (desc.fCoordMapping) { case ProgramDesc::StageDesc::kIdentity_CoordMapping: @@ -735,13 +734,13 @@ void GrGLProgram::genStageCode(int stageNum, complexCoord = true; break; case ProgramDesc::StageDesc::kRadial2Gradient_CoordMapping: { - GrTokenString cName = "c"; - GrTokenString ac4Name = "ac4"; - GrTokenString rootName = "root"; + GrTokenString cName("c"); + GrTokenString ac4Name("ac4"); + GrTokenString rootName("root"); - cName.appendInt(stageNum); - ac4Name.appendInt(stageNum); - rootName.appendInt(stageNum); + cName.appendS32(stageNum); + ac4Name.appendS32(stageNum); + rootName.appendS32(stageNum); GrTokenString bVar; if (coordDims == varyingDims) { @@ -750,7 +749,7 @@ void GrGLProgram::genStageCode(int stageNum, } else { GrAssert(3 == varyingDims); bVar = "b"; - bVar.appendInt(stageNum); + bVar.appendS32(stageNum); segments->fFSCode += "\tfloat "; segments->fFSCode += bVar; segments->fFSCode += " = 2.0 * ("; @@ -809,7 +808,7 @@ void GrGLProgram::genStageCode(int stageNum, locations->fNormalizedTexelSizeUni = 1; if (complexCoord) { GrTokenString coordVar("tCoord"); - coordVar.appendInt(stageNum); + coordVar.appendS32(stageNum); segments->fFSCode += "\t"; segments->fFSCode += float_vector_type(coordDims); segments->fFSCode += " "; @@ -821,7 +820,7 @@ void GrGLProgram::genStageCode(int stageNum, } static const char sign[] = {'-','+'}; GrTokenString stageAccumVar("stage2x2Accum"); - stageAccumVar.appendInt(stageNum); + stageAccumVar.appendS32(stageNum); segments->fFSCode += "\tvec4 "; segments->fFSCode += stageAccumVar; segments->fFSCode += " = "; diff --git a/gpu/src/gr_files.mk b/gpu/src/gr_files.mk index a40d7bd227..fef97844c8 100644 --- a/gpu/src/gr_files.mk +++ b/gpu/src/gr_files.mk @@ -11,7 +11,6 @@ SOURCE := \ GrGLTexture.cpp \ GrGLVertexBuffer.cpp \ GrGpu.cpp \ - GrGpuGLShaders2.cpp \ GrGpuGLFixed.cpp \ GrGpuFactory.cpp \ GrGLUtil.cpp \ diff --git a/gyp/skia.gyp b/gyp/skia.gyp index b2b43ccee3..8f228c9241 100644 --- a/gyp/skia.gyp +++ b/gyp/skia.gyp @@ -901,6 +901,8 @@ 'type': 'static_library', 'include_dirs': [ '../gpu/include', + '../include/core', + '../include/config', ], #'dependencies': [ # 'libtess', @@ -995,8 +997,6 @@ '../gpu/src/GrGpuGLFixed.h', '../gpu/src/GrGpuGLShaders.cpp', '../gpu/src/GrGpuGLShaders.h', - '../gpu/src/GrGpuGLShaders2.cpp', - '../gpu/src/GrGpuGLShaders2.h', '../gpu/src/GrInOrderDrawBuffer.cpp', '../gpu/src/GrMatrix.cpp', '../gpu/src/GrMemory.cpp', diff --git a/include/core/SkString.h b/include/core/SkString.h index 38604dd375..0295b7536d 100644 --- a/include/core/SkString.h +++ b/include/core/SkString.h @@ -99,6 +99,7 @@ public: // these methods edit the string SkString& operator=(const SkString&); + SkString& operator=(const char text[]); char* writable_str(); char& operator[](size_t n) { return this->writable_str()[n]; } @@ -144,6 +145,10 @@ public: void remove(size_t offset, size_t length); + SkString& operator+=(const SkString& s) { this->append(s); return *this; } + SkString& operator+=(const char text[]) { this->append(text); return *this; } + SkString& operator+=(const char c) { this->append(&c, 1); return *this; } + /** * Swap contents between this and other. This function is guaranteed * to never fail or throw. diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp index 49182ea77c..f461a7a44a 100644 --- a/src/core/SkString.cpp +++ b/src/core/SkString.cpp @@ -317,6 +317,15 @@ SkString& SkString::operator=(const SkString& src) { return *this; } +SkString& SkString::operator=(const char text[]) { + this->validate(); + + SkString tmp(text); + this->swap(tmp); + + return *this; +} + void SkString::reset() { this->validate(); diff --git a/xcode/gpu/gpu.xcodeproj/project.pbxproj b/xcode/gpu/gpu.xcodeproj/project.pbxproj index 438ac39f36..ab37ab8f82 100644 --- a/xcode/gpu/gpu.xcodeproj/project.pbxproj +++ b/xcode/gpu/gpu.xcodeproj/project.pbxproj @@ -570,6 +570,10 @@ ); GCC_WARN_ABOUT_RETURN_TYPE = YES; GCC_WARN_UNUSED_VARIABLE = YES; + HEADER_SEARCH_PATHS = ( + ../../include/core, + ../../include/config, + ); ONLY_ACTIVE_ARCH = YES; PREBINDING = NO; SDKROOT = macosx10.6;