From 9a92714d1344fd66c174d09cd080c89ed35d56ea Mon Sep 17 00:00:00 2001 From: "rileya@google.com" Date: Tue, 14 Aug 2012 21:33:15 +0000 Subject: [PATCH] Revert GrTextureStripAtlas change due to performance concerns. git-svn-id: http://skia.googlecode.com/svn/trunk@5103 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/effects/gradients/SkGradientShader.cpp | 68 +++++----------------- src/effects/gradients/SkGradientShaderPriv.h | 22 ------- .../gradients/SkTwoPointConicalGradient.cpp | 4 +- src/effects/gradients/SkTwoPointRadialGradient.cpp | 4 +- src/gpu/effects/GrTextureStripAtlas.cpp | 17 ++++-- 5 files changed, 29 insertions(+), 86 deletions(-) diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp index ccf3909..a07b388 100644 --- a/src/effects/gradients/SkGradientShader.cpp +++ b/src/effects/gradients/SkGradientShader.cpp @@ -672,38 +672,20 @@ SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END #if SK_SUPPORT_GPU -#include "effects/GrTextureStripAtlas.h" #include "SkGr.h" GrGLGradientStage::GrGLGradientStage(const GrProgramStageFactory& factory) - : INHERITED(factory) - , fCachedYCoord(GR_ScalarMax) - , fFSYUni(GrGLUniformManager::kInvalidUniformHandle) { } + : INHERITED(factory) { } GrGLGradientStage::~GrGLGradientStage() { } -void GrGLGradientStage::setupVariables(GrGLShaderBuilder* builder) { - fFSYUni = builder->addUniform(GrGLShaderBuilder::kFragment_ShaderType, - kFloat_GrSLType, "GradientYCoordFS"); -} - -void GrGLGradientStage::setData(const GrGLUniformManager& uman, - const GrCustomStage& stage, - const GrRenderTarget*, - int stageNum) { - GrScalar yCoord = static_cast(stage).getYCoord(); - if (yCoord != fCachedYCoord) { - uman.set1f(fFSYUni, yCoord); - fCachedYCoord = yCoord; - } -} - void GrGLGradientStage::emitColorLookup(GrGLShaderBuilder* builder, const char* tName, const char* outputColor, const char* samplerName) { - builder->fSampleCoords.printf("vec2(%s, %s)", tName, - builder->getUniformVariable(fFSYUni).c_str()); + // Texture is effectively 1D so the y coordinate is 0.5, if we pack multiple + // gradients into a texture, we could instead pick the appropriate row here + builder->fSampleCoords.printf("vec2(%s, 0.5)", tName); builder->fComplexCoord = true; builder->emitDefaultFetch(outputColor, samplerName); } @@ -714,7 +696,7 @@ GrGradientEffect::GrGradientEffect(GrContext* ctx, const SkGradientShaderBase& shader, GrSamplerState* sampler) : fTexture (NULL) - , fUseTexture (true) { + , fUseTexture (false) { // TODO: check for simple cases where we don't need a texture: //GradientInfo info; //shader.asAGradient(&info); @@ -723,40 +705,20 @@ GrGradientEffect::GrGradientEffect(GrContext* ctx, SkBitmap bitmap; shader.getGradientTableBitmap(&bitmap); - GrTextureStripAtlas::Desc desc; - desc.fWidth = bitmap.width(); - desc.fHeight = 32; - desc.fRowHeight = bitmap.height(); - desc.fContext = ctx; - desc.fConfig = SkBitmapConfig2GrPixelConfig(bitmap.config()); - fAtlas = GrTextureStripAtlas::GetAtlas(desc); - GrAssert(NULL != fAtlas); - - fRow = fAtlas->lockRow(bitmap); - if (-1 != fRow) { - fYCoord = fAtlas->getYOffset(fRow) + GR_ScalarHalf * - fAtlas->getVerticalScaleFactor(); - fTexture = fAtlas->getTexture(); - } else { - GrContext::TextureCacheEntry entry = GrLockCachedBitmapTexture(ctx, bitmap, - sampler->textureParams()); - fTexture = entry.texture(); - SkSafeRef(fTexture); - fYCoord = GR_ScalarHalf; + GrContext::TextureCacheEntry entry = GrLockCachedBitmapTexture(ctx, bitmap, + sampler->textureParams()); + fTexture = entry.texture(); + SkSafeRef(fTexture); + fUseTexture = true; - // Unlock immediately, this is not great, but we don't have a way of - // knowing when else to unlock it currently, so it may get purged from - // the cache, but it'll still be ref'd until it's no longer being used. - GrUnlockCachedBitmapTexture(ctx, entry); - } + // Unlock immediately, this is not great, but we don't have a way of + // knowing when else to unlock it currently, so it may get purged from + // the cache, but it'll still be ref'd until it's no longer being used. + GrUnlockCachedBitmapTexture(ctx, entry); } GrGradientEffect::~GrGradientEffect() { - if (this->useAtlas()) { - fAtlas->unlockRow(fRow); - } else { - SkSafeUnref(fTexture); - } + SkSafeUnref(fTexture); } unsigned int GrGradientEffect::numTextures() const { diff --git a/src/effects/gradients/SkGradientShaderPriv.h b/src/effects/gradients/SkGradientShaderPriv.h index 4c3b6c7..8152d5f 100644 --- a/src/effects/gradients/SkGradientShaderPriv.h +++ b/src/effects/gradients/SkGradientShaderPriv.h @@ -220,8 +220,6 @@ class GrProgramStageFactory; * determines the gradient value. */ - class GrTextureStripAtlas; - // Base class for Gr gradient effects class GrGradientEffect : public GrCustomStage { public: @@ -235,14 +233,6 @@ public: GrTexture* texture(unsigned int index) const; bool useTexture() const { return fUseTexture; } - bool useAtlas() const { return SkToBool(-1 != fRow); } - GrScalar getYCoord() const { GrAssert(fUseTexture); return fYCoord; }; - - bool isEqual(const GrCustomStage& stage) const { - const GrGradientEffect& s = static_cast(stage); - return INHERITED::isEqual(stage) && this->useAtlas() == s.useAtlas() && - fYCoord == s.getYCoord(); - } protected: @@ -262,9 +252,6 @@ protected: private: GrTexture* fTexture; bool fUseTexture; - GrScalar fYCoord; - GrTextureStripAtlas* fAtlas; - int fRow; typedef GrCustomStage INHERITED; @@ -279,12 +266,6 @@ public: GrGLGradientStage(const GrProgramStageFactory& factory); virtual ~GrGLGradientStage(); - virtual void setupVariables(GrGLShaderBuilder* builder) SK_OVERRIDE; - virtual void setData(const GrGLUniformManager&, - const GrCustomStage&, - const GrRenderTarget*, - int stageNum) SK_OVERRIDE; - // emit code that gets a fragment's color from an expression for t; for now // this always uses the texture, but for simpler cases we'll be able to lerp void emitColorLookup(GrGLShaderBuilder* builder, const char* t, @@ -292,9 +273,6 @@ public: private: - GrScalar fCachedYCoord; - GrGLUniformManager::UniformHandle fFSYUni; - typedef GrGLProgramStage INHERITED; }; diff --git a/src/effects/gradients/SkTwoPointConicalGradient.cpp b/src/effects/gradients/SkTwoPointConicalGradient.cpp index e1d8cb4..528291b 100644 --- a/src/effects/gradients/SkTwoPointConicalGradient.cpp +++ b/src/effects/gradients/SkTwoPointConicalGradient.cpp @@ -468,7 +468,6 @@ GrGLConical2Gradient::GrGLConical2Gradient( } void GrGLConical2Gradient::setupVariables(GrGLShaderBuilder* builder) { - INHERITED::setupVariables(builder); // 2 copies of uniform array, 1 for each of vertex & fragment shader, // to work around Xoom bug. Doesn't seem to cause performance decrease // in test apps, but need to keep an eye on it. @@ -632,9 +631,8 @@ void GrGLConical2Gradient::emitFS(GrGLShaderBuilder* builder, void GrGLConical2Gradient::setData(const GrGLUniformManager& uman, const GrCustomStage& baseData, - const GrRenderTarget* target, + const GrRenderTarget*, int stageNum) { - INHERITED::setData(uman, baseData, target, stageNum); const GrConical2Gradient& data = static_cast(baseData); GrAssert(data.isDegenerate() == fIsDegenerate); diff --git a/src/effects/gradients/SkTwoPointRadialGradient.cpp b/src/effects/gradients/SkTwoPointRadialGradient.cpp index 9d93771..441f5a8 100644 --- a/src/effects/gradients/SkTwoPointRadialGradient.cpp +++ b/src/effects/gradients/SkTwoPointRadialGradient.cpp @@ -501,7 +501,6 @@ GrGLRadial2Gradient::GrGLRadial2Gradient( } void GrGLRadial2Gradient::setupVariables(GrGLShaderBuilder* builder) { - INHERITED::setupVariables(builder); // 2 copies of uniform array, 1 for each of vertex & fragment shader, // to work around Xoom bug. Doesn't seem to cause performance decrease // in test apps, but need to keep an eye on it. @@ -607,9 +606,8 @@ void GrGLRadial2Gradient::emitFS(GrGLShaderBuilder* builder, void GrGLRadial2Gradient::setData(const GrGLUniformManager& uman, const GrCustomStage& baseData, - const GrRenderTarget* target, + const GrRenderTarget*, int stageNum) { - INHERITED::setData(uman, baseData, target, stageNum); const GrRadial2Gradient& data = static_cast(baseData); GrAssert(data.isDegenerate() == fIsDegenerate); diff --git a/src/gpu/effects/GrTextureStripAtlas.cpp b/src/gpu/effects/GrTextureStripAtlas.cpp index 3f3b29d..1a8a93b 100644 --- a/src/gpu/effects/GrTextureStripAtlas.cpp +++ b/src/gpu/effects/GrTextureStripAtlas.cpp @@ -34,8 +34,14 @@ public: GrTextureStripAtlas* fAtlas; }; +// Ugly way of ensuring that we clean up the atlases on exit +struct AtlasEntries { + ~AtlasEntries() { fEntries.deleteAll(); } + SkTDArray fEntries; +}; + GrTextureStripAtlas* GrTextureStripAtlas::GetAtlas(const GrTextureStripAtlas::Desc& desc) { - static SkTDArray gAtlasEntries; + static AtlasEntries gAtlasEntries; static GrTHashTable gAtlasCache; AtlasHashKey key; key.setKeyData(desc.asKey()); @@ -43,7 +49,8 @@ GrTextureStripAtlas* GrTextureStripAtlas::GetAtlas(const GrTextureStripAtlas::De if (NULL != entry) { return entry->fAtlas; } else { - entry = gAtlasEntries.push(); + entry = SkNEW(AtlasEntry); + gAtlasEntries.fEntries.push(entry); entry->fAtlas = SkNEW_ARGS(GrTextureStripAtlas, (desc)); entry->fKey = key; gAtlasCache.insert(key, entry); @@ -108,6 +115,8 @@ int GrTextureStripAtlas::lockRow(const SkBitmap& data) { this->removeFromLRU(row); uint32_t oldKey = row->fKey; + row->fKey = key; + row->fLocks = 1; // If we are writing into a row that already held bitmap data, we need to remove the // reference to that genID which is stored in our sorted table of key values. @@ -116,15 +125,13 @@ int GrTextureStripAtlas::lockRow(const SkBitmap& data) { // Find the entry in the list; if it's before the index where we plan on adding the new // entry, we decrement since it will shift elements ahead of it back by one. int oldIndex = this->searchByKey(oldKey); - if (oldIndex < index) { + if (oldIndex <= index) { --index; } fKeyTable.remove(oldIndex); } - row->fKey = key; - row->fLocks = 1; fKeyTable.insert(index, 1, &row); rowNumber = static_cast(row - fRows); -- 2.7.4