Fix GrTextureStripAtlasManager cleanup order bug

Bug: 820703
Change-Id: I6f1a895ceb213d38361bc03a472cf2a48e4720a5
Reviewed-on: https://skia-review.googlesource.com/115001
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This commit is contained in:
Robert Phillips 2018-03-19 10:57:42 -04:00 committed by Skia Commit-Bot
parent f7466bd84a
commit 96b6d537c2
7 changed files with 100 additions and 19 deletions

View File

@ -261,6 +261,7 @@ tests_sources = [
"$_tests/TextBlobCacheTest.cpp",
"$_tests/TextBlobTest.cpp",
"$_tests/TextureProxyTest.cpp",
"$_tests/TextureStripAtlasManagerTest.cpp",
"$_tests/Time.cpp",
"$_tests/TopoSortTest.cpp",
"$_tests/ToSRGBColorFilter.cpp",

View File

@ -332,7 +332,7 @@ public:
const char* name() const override { return "ColorTable"; }
const GrTextureStripAtlas* atlas() const { return fAtlas; }
const GrTextureStripAtlas* atlas() const { return fAtlas.get(); }
int atlasRow() const { return fRow; }
std::unique_ptr<GrFragmentProcessor> clone() const override;
@ -344,12 +344,12 @@ private:
bool onIsEqual(const GrFragmentProcessor&) const override;
ColorTableEffect(sk_sp<GrTextureProxy> proxy, GrTextureStripAtlas* atlas, int row);
ColorTableEffect(sk_sp<GrTextureProxy> proxy, sk_sp<GrTextureStripAtlas> atlas, int row);
GR_DECLARE_FRAGMENT_PROCESSOR_TEST
TextureSampler fTextureSampler;
GrTextureStripAtlas* fAtlas;
sk_sp<GrTextureStripAtlas> fAtlas;
int fRow;
typedef GrFragmentProcessor INHERITED;
@ -452,7 +452,7 @@ std::unique_ptr<GrFragmentProcessor> ColorTableEffect::Make(GrContext* context,
auto atlasManager = context->contextPriv().textureStripAtlasManager();
GrTextureStripAtlas* atlas = atlasManager->getAtlas(desc);
sk_sp<GrTextureStripAtlas> atlas = atlasManager->refAtlas(desc);
int row = atlas->lockRow(context, bitmap);
sk_sp<GrTextureProxy> proxy;
if (-1 == row) {
@ -474,14 +474,16 @@ std::unique_ptr<GrFragmentProcessor> ColorTableEffect::Make(GrContext* context,
return nullptr;
}
return std::unique_ptr<GrFragmentProcessor>(new ColorTableEffect(std::move(proxy), atlas, row));
return std::unique_ptr<GrFragmentProcessor>(new ColorTableEffect(std::move(proxy),
std::move(atlas), row));
}
ColorTableEffect::ColorTableEffect(sk_sp<GrTextureProxy> proxy, GrTextureStripAtlas* atlas, int row)
ColorTableEffect::ColorTableEffect(sk_sp<GrTextureProxy> proxy,
sk_sp<GrTextureStripAtlas> atlas, int row)
: INHERITED(kColorTableEffect_ClassID,
kNone_OptimizationFlags) // Not bothering with table-specific optimizations.
, fTextureSampler(std::move(proxy))
, fAtlas(atlas)
, fAtlas(std::move(atlas))
, fRow(row) {
this->addTextureSampler(&fTextureSampler);
}

View File

@ -9,8 +9,9 @@
#define GrTextureStripAtlas_DEFINED
#include "SkBitmap.h"
#include "SkOpts.h"
#include "SkGr.h"
#include "SkOpts.h"
#include "SkRefCnt.h"
#include "SkTDArray.h"
#include "SkTDynamicHash.h"
#include "SkTypes.h"
@ -22,7 +23,7 @@ class GrTextureProxy;
* Maintains a single large texture whose rows store many textures of a small fixed height,
* stored in rows across the x-axis such that we can safely wrap/repeat them horizontally.
*/
class GrTextureStripAtlas {
class GrTextureStripAtlas : public SkRefCnt {
public:
/**
* Descriptor struct which we'll use as a hash table key
@ -87,7 +88,7 @@ private:
* The state of a single row in our cache, next/prev pointers allow these to be chained
* together to represent LRU status
*/
struct AtlasRow : SkNoncopyable {
struct AtlasRow : ::SkNoncopyable {
AtlasRow() : fKey(kEmptyAtlasRowKey), fLocks(0), fNext(nullptr), fPrev(nullptr) { }
// GenerationID of the bitmap that is represented by this row, 0xffffffff means "empty"
uint32_t fKey;
@ -174,7 +175,7 @@ public:
/**
* Try to find an atlas with the required parameters, creates a new one if necessary
*/
GrTextureStripAtlas* getAtlas(const GrTextureStripAtlas::Desc&);
sk_sp<GrTextureStripAtlas> refAtlas(const GrTextureStripAtlas::Desc&);
private:
void deleteAllAtlases();
@ -182,11 +183,11 @@ private:
// Hash table entry for atlases
class AtlasEntry : public ::SkNoncopyable {
public:
AtlasEntry(const GrTextureStripAtlas::Desc& desc, GrTextureStripAtlas* atlas)
AtlasEntry(const GrTextureStripAtlas::Desc& desc, sk_sp<GrTextureStripAtlas> atlas)
: fDesc(desc)
, fAtlas(atlas) {
, fAtlas(std::move(atlas)) {
}
~AtlasEntry() { delete fAtlas; }
~AtlasEntry() { }
// for SkTDynamicHash
static const GrTextureStripAtlas::Desc& GetKey(const AtlasEntry& entry) {
@ -197,7 +198,7 @@ private:
}
const GrTextureStripAtlas::Desc fDesc;
GrTextureStripAtlas* fAtlas;
sk_sp<GrTextureStripAtlas> fAtlas;
};
typedef SkTDynamicHash<AtlasEntry, GrTextureStripAtlas::Desc> AtlasHash;

View File

@ -39,11 +39,12 @@ void GrTextureStripAtlasManager::abandon() {
this->deleteAllAtlases();
}
GrTextureStripAtlas* GrTextureStripAtlasManager::getAtlas(const GrTextureStripAtlas::Desc& desc) {
sk_sp<GrTextureStripAtlas> GrTextureStripAtlasManager::refAtlas(
const GrTextureStripAtlas::Desc& desc) {
AtlasEntry* entry = fAtlasCache.find(desc);
if (!entry) {
// TODO: Does the AtlasEntry need a copy of the Desc if the GrTextureStripAtlas has one?
entry = new AtlasEntry(desc, new GrTextureStripAtlas(desc));
entry = new AtlasEntry(desc, sk_sp<GrTextureStripAtlas>(new GrTextureStripAtlas(desc)));
fAtlasCache.add(entry);
}

View File

@ -1298,7 +1298,7 @@ GrGradientEffect::GrGradientEffect(ClassID classID, const CreateArgs& args, bool
desc.fHeight = 32;
desc.fRowHeight = bitmap.height(); // always 1 here
desc.fConfig = SkImageInfo2GrPixelConfig(bitmap.info(), *args.fContext->caps());
fAtlas = atlasManager->getAtlas(desc);
fAtlas = atlasManager->refAtlas(desc);
SkASSERT(fAtlas);
// We always filter the gradient table. Each table is one row of a texture, always

View File

@ -334,7 +334,7 @@ private:
GrCoordTransform fCoordTransform;
TextureSampler fTextureSampler;
SkScalar fYCoord;
GrTextureStripAtlas* fAtlas;
sk_sp<GrTextureStripAtlas> fAtlas;
int fRow;
bool fIsOpaque;

View File

@ -0,0 +1,76 @@
/*
* Copyright 2018 Google Inc.
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "SkCanvas.h"
#include "SkGradientShader.h"
#include "SkPaint.h"
#include "SkSurface.h"
#include "SkTableColorFilter.h"
#include "Resources.h"
#include "Test.h"
#if SK_SUPPORT_GPU // These are all GPU-backend specific tests
// The gradient shader will use the texture strip atlas if it has too many colors. Make sure
// abandoning the context works.
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(TextureStripAtlasManagerGradientTest, reporter, ctxInfo) {
GrContext* context = ctxInfo.grContext();
static const SkColor gColors[] = { SK_ColorRED, SK_ColorGREEN, SK_ColorBLUE,
SK_ColorCYAN, SK_ColorMAGENTA, SK_ColorYELLOW,
SK_ColorBLACK };
static const SkScalar gPos[] = { 0, 0.17f, 0.32f, 0.49f, 0.66f, 0.83f, 1.0f };
SkPaint p;
p.setShader(SkGradientShader::MakeTwoPointConical(SkPoint::Make(0, 0),
1.0f,
SkPoint::Make(10.0f, 20.0f),
2.0f,
gColors,
gPos,
7,
SkShader::kClamp_TileMode));
SkImageInfo info = SkImageInfo::MakeN32Premul(128, 128);
auto surface(SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info));
SkCanvas* canvas = surface->getCanvas();
SkRect r = SkRect::MakeXYWH(10, 10, 100, 100);
canvas->drawRect(r, p);
context->abandonContext();
}
// The table color filter uses the texture strip atlas. Make sure abandoning the context works.
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(TextureStripAtlasManagerColorFilterTest, reporter, ctxInfo) {
GrContext* context = ctxInfo.grContext();
sk_sp<SkImage> img = GetResourceAsImage("images/mandrill_128.png");
uint8_t identity[256];
for (int i = 0; i < 256; i++) {
identity[i] = i;
}
SkPaint p;
p.setColorFilter(SkTableColorFilter::Make(identity));
SkImageInfo info = SkImageInfo::MakeN32Premul(128, 128);
auto surface(SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info));
SkCanvas* canvas = surface->getCanvas();
canvas->drawImage(std::move(img), 0, 0, &p);
context->abandonContext();
}
#endif