From 711ef4831363fb8cbdf061dc2c36c65b13c0ccf2 Mon Sep 17 00:00:00 2001 From: kkinnunen Date: Wed, 21 Jan 2015 06:39:14 -0800 Subject: Make GrScratchKey memory buffer correct size on copy Scratch key memory buffer of a copy of a key was too big. The (new) copy was N times uint32_t bytes instead of N bytes. Adds few tests to resource cache. These tests would not catch the too big buffer. This is just a precaution for too small buffers. The main idea of the test change is that the scratch key should contain some information, so that lookup with a scratch key can also return no match. Otherwise testing of scratch lookup result is not indicative of correct code (eg. no-information scratch key will always match). Review URL: https://codereview.chromium.org/860333002 --- tests/ResourceCacheTest.cpp | 171 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 136 insertions(+), 35 deletions(-) (limited to 'tests') diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp index e51b3e0997..dd480d8b34 100644 --- a/tests/ResourceCacheTest.cpp +++ b/tests/ResourceCacheTest.cpp @@ -63,13 +63,18 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context, SkCanva class TestResource : public GrGpuResource { static const size_t kDefaultSize = 100; - + enum ScratchConstructor { kScratchConstructor }; public: SK_DECLARE_INST_COUNT(TestResource); + /** Property that distinctly categorizes the resource. + * For example, textures have width, height, ... */ + enum SimulatedProperty { kProperty1_SimulatedProperty, kProperty2_SimulatedProperty }; + TestResource(GrGpu* gpu, size_t size, GrGpuResource::LifeCycle lifeCycle) : INHERITED(gpu, lifeCycle) , fToDelete(NULL) - , fSize(size) { + , fSize(size) + , fProperty(kProperty1_SimulatedProperty) { ++fNumAlive; this->registerWithCache(); } @@ -77,7 +82,8 @@ public: TestResource(GrGpu* gpu, GrGpuResource::LifeCycle lifeCycle) : INHERITED(gpu, lifeCycle) , fToDelete(NULL) - , fSize(kDefaultSize) { + , fSize(kDefaultSize) + , fProperty(kProperty1_SimulatedProperty) { ++fNumAlive; this->registerWithCache(); } @@ -85,18 +91,14 @@ public: TestResource(GrGpu* gpu) : INHERITED(gpu, kCached_LifeCycle) , fToDelete(NULL) - , fSize(kDefaultSize) { + , fSize(kDefaultSize) + , fProperty(kProperty1_SimulatedProperty) { ++fNumAlive; this->registerWithCache(); } - TestResource(GrGpu* gpu, const GrScratchKey& scratchKey) - : INHERITED(gpu, kCached_LifeCycle) - , fToDelete(NULL) - , fSize(kDefaultSize) { - this->setScratchKey(scratchKey); - ++fNumAlive; - this->registerWithCache(); + static TestResource* CreateScratchTestResource(GrGpu* gpu, SimulatedProperty property) { + return SkNEW_ARGS(TestResource, (gpu, property, kScratchConstructor)); } ~TestResource() { @@ -115,13 +117,34 @@ public: SkRefCnt_SafeAssign(fToDelete, resource); } + static void ComputeScratchKey(SimulatedProperty property, GrScratchKey* key) { + static GrScratchKey::ResourceType t = GrScratchKey::GenerateResourceType(); + static const size_t kTestAmount = 6; + GrScratchKey::Builder builder(key, t, kTestAmount); + for (size_t i = 0; i < kTestAmount; ++i) { + builder[i] = i + static_cast(property); + } + } + private: + TestResource(GrGpu* gpu, SimulatedProperty property, ScratchConstructor) + : INHERITED(gpu, kCached_LifeCycle) + , fToDelete(NULL) + , fSize(kDefaultSize) + , fProperty(property) { + GrScratchKey scratchKey; + ComputeScratchKey(fProperty, &scratchKey); + this->setScratchKey(scratchKey); + ++fNumAlive; + this->registerWithCache(); + } + size_t onGpuMemorySize() const SK_OVERRIDE { return fSize; } TestResource* fToDelete; size_t fSize; static int fNumAlive; - + SimulatedProperty fProperty; typedef GrGpuResource INHERITED; }; int TestResource::fNumAlive = 0; @@ -181,11 +204,6 @@ static void test_no_key(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, 0 == cache2->getResourceBytes()); } -static void make_scratch_key(GrScratchKey* key) { - static GrScratchKey::ResourceType t = GrScratchKey::GenerateResourceType(); - GrScratchKey::Builder builder(key, t, 0); -} - static void test_budgeting(skiatest::Reporter* reporter) { SkAutoTUnref context(GrContext::CreateMockContext()); REPORTER_ASSERT(reporter, SkToBool(context)); @@ -198,15 +216,14 @@ static void test_budgeting(skiatest::Reporter* reporter) { SkASSERT(0 == cache2->getResourceCount() && 0 == cache2->getResourceBytes()); SkASSERT(0 == cache2->getBudgetedResourceCount() && 0 == cache2->getBudgetedResourceBytes()); - GrScratchKey scratchKey; - make_scratch_key(&scratchKey); - GrCacheID::Key keyData; memset(&keyData, 0, sizeof(keyData)); GrResourceKey contentKey(GrCacheID(GrCacheID::GenerateDomain(), keyData), 0); // Create a scratch, a content, and a wrapped resource - TestResource* scratch = SkNEW_ARGS(TestResource, (context->getGpu(), scratchKey)); + TestResource* scratch = + TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); scratch->setSize(10); TestResource* content = SkNEW_ARGS(TestResource, (context->getGpu())); content->setSize(11); @@ -293,9 +310,6 @@ static void test_unbudgeted(skiatest::Reporter* reporter) { SkASSERT(0 == cache2->getResourceCount() && 0 == cache2->getResourceBytes()); SkASSERT(0 == cache2->getBudgetedResourceCount() && 0 == cache2->getBudgetedResourceBytes()); - GrScratchKey scratchKey; - make_scratch_key(&scratchKey); - GrCacheID::Key keyData; memset(&keyData, 0, sizeof(keyData)); GrResourceKey contentKey(GrCacheID(GrCacheID::GenerateDomain(), keyData), 0); @@ -306,7 +320,8 @@ static void test_unbudgeted(skiatest::Reporter* reporter) { TestResource* unbudgeted; // A large uncached or wrapped resource shouldn't evict anything. - scratch = SkNEW_ARGS(TestResource, (context->getGpu(), scratchKey)); + scratch = TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); scratch->setSize(10); scratch->unref(); REPORTER_ASSERT(reporter, 1 == cache2->getResourceCount()); @@ -368,14 +383,23 @@ static void test_duplicate_scratch_key(skiatest::Reporter* reporter) { cache2->purgeAllUnlocked(); SkASSERT(0 == cache2->getResourceCount() && 0 == cache2->getResourceBytes()); - GrScratchKey scratchKey; - make_scratch_key(&scratchKey); - // Create two resources that have the same scratch key. - TestResource* a = SkNEW_ARGS(TestResource, (context->getGpu(), scratchKey)); - TestResource* b = SkNEW_ARGS(TestResource, (context->getGpu(), scratchKey)); + TestResource* a = + TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); + TestResource* b = + TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); a->setSize(11); b->setSize(12); + GrScratchKey scratchKey1; + TestResource::ComputeScratchKey(TestResource::kProperty1_SimulatedProperty, &scratchKey1); + // Check for negative case consistency. (leaks upon test failure.) + REPORTER_ASSERT(reporter, NULL == cache2->findAndRefScratchResource(scratchKey1)); + + GrScratchKey scratchKey; + TestResource::ComputeScratchKey(TestResource::kProperty2_SimulatedProperty, &scratchKey); + // Scratch resources are registered with GrResourceCache2 just by existing. There are 2. REPORTER_ASSERT(reporter, 2 == TestResource::NumAlive()); SkDEBUGCODE(REPORTER_ASSERT(reporter, 2 == cache2->countScratchEntriesForKey(scratchKey));) @@ -412,16 +436,25 @@ static void test_remove_scratch_key(skiatest::Reporter* reporter) { cache2->purgeAllUnlocked(); SkASSERT(0 == cache2->getResourceCount() && 0 == cache2->getResourceBytes()); - GrScratchKey scratchKey; - make_scratch_key(&scratchKey); - // Create two resources that have the same scratch key. - TestResource* a = SkNEW_ARGS(TestResource, (context->getGpu(), scratchKey)); - TestResource* b = SkNEW_ARGS(TestResource, (context->getGpu(), scratchKey)); + TestResource* a = + TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); + TestResource* b = + TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); a->unref(); b->unref(); + + GrScratchKey scratchKey; + // Ensure that scratch key lookup is correct for negative case. + TestResource::ComputeScratchKey(TestResource::kProperty1_SimulatedProperty, &scratchKey); + // (following leaks upon test failure). + REPORTER_ASSERT(reporter, cache2->findAndRefScratchResource(scratchKey) == NULL); + // Scratch resources are registered with GrResourceCache2 just by existing. There are 2. + TestResource::ComputeScratchKey(TestResource::kProperty2_SimulatedProperty, &scratchKey); REPORTER_ASSERT(reporter, 2 == TestResource::NumAlive()); SkDEBUGCODE(REPORTER_ASSERT(reporter, 2 == cache2->countScratchEntriesForKey(scratchKey));) REPORTER_ASSERT(reporter, 2 == cache2->getResourceCount()); @@ -460,6 +493,73 @@ static void test_remove_scratch_key(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, 0 == cache2->getResourceCount()); } +static void test_scratch_key_consistency(skiatest::Reporter* reporter) { + SkAutoTUnref context(GrContext::CreateMockContext()); + REPORTER_ASSERT(reporter, SkToBool(context)); + if (NULL == context) { + return; + } + context->setResourceCacheLimits(5, 30000); + GrResourceCache2* cache2 = context->getResourceCache2(); + cache2->purgeAllUnlocked(); + SkASSERT(0 == cache2->getResourceCount() && 0 == cache2->getResourceBytes()); + + // Create two resources that have the same scratch key. + TestResource* a = + TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); + TestResource* b = + TestResource::CreateScratchTestResource(context->getGpu(), + TestResource::kProperty2_SimulatedProperty); + a->unref(); + b->unref(); + + GrScratchKey scratchKey; + // Ensure that scratch key comparison and assignment is consistent. + GrScratchKey scratchKey1; + TestResource::ComputeScratchKey(TestResource::kProperty1_SimulatedProperty, &scratchKey1); + GrScratchKey scratchKey2; + TestResource::ComputeScratchKey(TestResource::kProperty2_SimulatedProperty, &scratchKey2); + REPORTER_ASSERT(reporter, scratchKey1.size() <= sizeof(scratchKey1)); + REPORTER_ASSERT(reporter, scratchKey1 != scratchKey2); + REPORTER_ASSERT(reporter, scratchKey2 != scratchKey1); + scratchKey = scratchKey1; + REPORTER_ASSERT(reporter, scratchKey.size() <= sizeof(scratchKey)); + REPORTER_ASSERT(reporter, scratchKey1 == scratchKey); + REPORTER_ASSERT(reporter, scratchKey == scratchKey1); + REPORTER_ASSERT(reporter, scratchKey2 != scratchKey); + REPORTER_ASSERT(reporter, scratchKey != scratchKey2); + scratchKey = scratchKey2; + REPORTER_ASSERT(reporter, scratchKey.size() <= sizeof(scratchKey)); + REPORTER_ASSERT(reporter, scratchKey1 != scratchKey); + REPORTER_ASSERT(reporter, scratchKey != scratchKey1); + REPORTER_ASSERT(reporter, scratchKey2 == scratchKey); + REPORTER_ASSERT(reporter, scratchKey == scratchKey2); + + // Ensure that scratch key lookup is correct for negative case. + TestResource::ComputeScratchKey(TestResource::kProperty1_SimulatedProperty, &scratchKey); + // (following leaks upon test failure). + REPORTER_ASSERT(reporter, cache2->findAndRefScratchResource(scratchKey) == NULL); + + // Find the first resource with a scratch key and a copy of a scratch key. + TestResource::ComputeScratchKey(TestResource::kProperty2_SimulatedProperty, &scratchKey); + GrGpuResource* find = cache2->findAndRefScratchResource(scratchKey); + REPORTER_ASSERT(reporter, find != NULL); + find->unref(); + + scratchKey2 = scratchKey; + find = cache2->findAndRefScratchResource(scratchKey2); + REPORTER_ASSERT(reporter, find != NULL); + REPORTER_ASSERT(reporter, find == a || find == b); + + GrGpuResource* find2 = cache2->findAndRefScratchResource(scratchKey2); + REPORTER_ASSERT(reporter, find2 != NULL); + REPORTER_ASSERT(reporter, find2 == a || find2 == b); + REPORTER_ASSERT(reporter, find2 != find); + find2->unref(); + find->unref(); +} + static void test_duplicate_content_key(skiatest::Reporter* reporter) { SkAutoTUnref context(GrContext::CreateMockContext()); REPORTER_ASSERT(reporter, SkToBool(context)); @@ -809,6 +909,7 @@ DEF_GPUTEST(ResourceCache, reporter, factory) { test_duplicate_content_key(reporter); test_duplicate_scratch_key(reporter); test_remove_scratch_key(reporter); + test_scratch_key_consistency(reporter); test_purge_invalidated(reporter); test_cache_chained_purge(reporter); test_resource_size_changed(reporter); -- cgit v1.2.3