Revert GrTextureStripAtlas change due to performance concerns.
authorrileya@google.com <rileya@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 14 Aug 2012 21:33:15 +0000 (21:33 +0000)
committerrileya@google.com <rileya@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 14 Aug 2012 21:33:15 +0000 (21:33 +0000)
git-svn-id: http://skia.googlecode.com/svn/trunk@5103 2bbb7eff-a529-9590-31e7-b0007b416f81

src/effects/gradients/SkGradientShader.cpp
src/effects/gradients/SkGradientShaderPriv.h
src/effects/gradients/SkTwoPointConicalGradient.cpp
src/effects/gradients/SkTwoPointRadialGradient.cpp
src/gpu/effects/GrTextureStripAtlas.cpp

index ccf3909..a07b388 100644 (file)
@@ -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<const GrGradientEffect&>(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 {
index 4c3b6c7..8152d5f 100644 (file)
@@ -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<const GrGradientEffect&>(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;
 };
 
index e1d8cb4..528291b 100644 (file)
@@ -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<const GrConical2Gradient&>(baseData);
     GrAssert(data.isDegenerate() == fIsDegenerate);
index 9d93771..441f5a8 100644 (file)
@@ -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<const GrRadial2Gradient&>(baseData);
     GrAssert(data.isDegenerate() == fIsDegenerate);
index 3f3b29d..1a8a93b 100644 (file)
@@ -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<AtlasEntry*> fEntries;
+};
+
 GrTextureStripAtlas* GrTextureStripAtlas::GetAtlas(const GrTextureStripAtlas::Desc& desc) {
-    static SkTDArray<AtlasEntry> gAtlasEntries;
+    static AtlasEntries gAtlasEntries;
     static GrTHashTable<AtlasEntry, AtlasHashKey, 8> 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<int>(row - fRows);