From 91359bed48bc006a4319da86eb26db3b2e6d4afb Mon Sep 17 00:00:00 2001 From: mtklein Date: Sun, 8 Jun 2014 07:02:47 -0700 Subject: [PATCH] Revert of Clean up Test's resourcePath code. (https://codereview.chromium.org/319473003/) Reason for revert: Some benchmarks are written in a way that makes this change unsafe (e.g. const char* resPath = GetResourcePath().c_str(); in SkipZeroesBench) and Valgrind and ASAN caught that. We can try again after a more careful cleanup of GetResourcePath(). Original issue's description: > Clean up resourcePath code. > > 1) Make the implementation of SetResourcePath/GetResourcePath of GM and SkBenchmark match with the one in Test. > 2) Make gResourcePath a static pointer to const char and move it inside the classes. > > BUG=None > TEST=make tests && out/Debug/tests > make gm && out/Debug/gm > make bench && out/Debug/bench > R=mtklein@google.com > > Committed: https://skia.googlesource.com/skia/+/52e4f413ffe2d281f9e90ff2147db08083ffcba7 R=tfarina@chromium.org TBR=tfarina@chromium.org NOTREECHECKS=true NOTRY=true BUG=None Author: mtklein@google.com Review URL: https://codereview.chromium.org/320733002 --- bench/SkBenchmark.cpp | 9 +-------- bench/SkBenchmark.h | 7 ++++--- gm/copyTo4444.cpp | 3 ++- gm/etc1bitmap.cpp | 6 ++++-- gm/factory.cpp | 3 ++- gm/gm.cpp | 12 +----------- gm/gm.h | 11 ++++++++--- tests/Test.cpp | 9 +++------ tests/Test.h | 2 -- 9 files changed, 25 insertions(+), 37 deletions(-) diff --git a/bench/SkBenchmark.cpp b/bench/SkBenchmark.cpp index 1f12ed3..d995415 100644 --- a/bench/SkBenchmark.cpp +++ b/bench/SkBenchmark.cpp @@ -12,7 +12,7 @@ const char* SkTriState::Name[] = { "default", "true", "false" }; template BenchRegistry* BenchRegistry::gHead; -const char* SkBenchmark::gResourcePath; +SkString SkBenchmark::gResourcePath; SkBenchmark::SkBenchmark() { fForceAlpha = 0xFF; @@ -55,13 +55,6 @@ void SkBenchmark::setupPaint(SkPaint* paint) { } } -void SkBenchmark::SetResourcePath(const char* resourcePath) { - gResourcePath = resourcePath; -} - -SkString SkBenchmark::GetResourcePath() { - return SkString(gResourcePath); -} /////////////////////////////////////////////////////////////////////////////// diff --git a/bench/SkBenchmark.h b/bench/SkBenchmark.h index 00a816c..bf28689 100644 --- a/bench/SkBenchmark.h +++ b/bench/SkBenchmark.h @@ -107,8 +107,9 @@ public: fClearMask = clearMask; } - static void SetResourcePath(const char*); - static SkString GetResourcePath(); + static void SetResourcePath(const char* resPath) { gResourcePath.set(resPath); } + + static SkString& GetResourcePath() { return gResourcePath; } protected: virtual void setupPaint(SkPaint* paint); @@ -128,7 +129,7 @@ private: bool fForceFilter; SkTriState::State fDither; uint32_t fOrMask, fClearMask; - static const char* gResourcePath; + static SkString gResourcePath; typedef SkRefCnt INHERITED; }; diff --git a/gm/copyTo4444.cpp b/gm/copyTo4444.cpp index 6233301..7e2c279 100644 --- a/gm/copyTo4444.cpp +++ b/gm/copyTo4444.cpp @@ -30,7 +30,8 @@ protected: virtual void onDraw(SkCanvas* canvas) { SkBitmap bm, bm4444; - SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "mandrill_512.png"); + SkString filename = SkOSPath::SkPathJoin( + INHERITED::gResourcePath.c_str(), "mandrill_512.png"); if (!SkImageDecoder::DecodeFile(filename.c_str(), &bm, SkBitmap::kARGB_8888_Config, SkImageDecoder::kDecodePixels_Mode)) { diff --git a/gm/etc1bitmap.cpp b/gm/etc1bitmap.cpp index ce0aa7d..cdf8617 100644 --- a/gm/etc1bitmap.cpp +++ b/gm/etc1bitmap.cpp @@ -93,7 +93,8 @@ protected: virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE { SkBitmap bm; - SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "mandrill_128."); + SkString filename = SkOSPath::SkPathJoin( + INHERITED::gResourcePath.c_str(), "mandrill_128."); filename.append(this->fileExtension()); SkAutoTUnref fileData(SkData::NewFromFileName(filename.c_str())); @@ -168,7 +169,8 @@ protected: virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE { SkBitmap bm; - SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "mandrill_128.pkm"); + SkString filename = SkOSPath::SkPathJoin( + INHERITED::gResourcePath.c_str(), "mandrill_128.pkm"); SkAutoDataUnref fileData(SkData::NewFromFileName(filename.c_str())); if (NULL == fileData) { diff --git a/gm/factory.cpp b/gm/factory.cpp index aa643a2..bd79c9d 100644 --- a/gm/factory.cpp +++ b/gm/factory.cpp @@ -28,7 +28,8 @@ public: protected: virtual void onOnceBeforeDraw() SK_OVERRIDE { // Copyright-free file from http://openclipart.org/detail/29213/paper-plane-by-ddoo - SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath, "plane.png"); + SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath.c_str(), + "plane.png"); SkAutoDataUnref data(SkData::NewFromFileName(filename.c_str())); if (NULL != data.get()) { // Create a cache which will boot the pixels out anytime the diff --git a/gm/gm.cpp b/gm/gm.cpp index 8da45c5..803874f 100644 --- a/gm/gm.cpp +++ b/gm/gm.cpp @@ -6,10 +6,9 @@ */ #include "gm.h" - using namespace skiagm; -const char* GM::gResourcePath; +SkString GM::gResourcePath; GM::GM() { fMode = kGM_Mode; @@ -18,7 +17,6 @@ GM::GM() { fHaveCalledOnceBeforeDraw = false; fStarterMatrix.reset(); } - GM::~GM() {} void GM::draw(SkCanvas* canvas) { @@ -66,13 +64,5 @@ void GM::drawSizeBounds(SkCanvas* canvas, SkColor color) { canvas->drawRect(r, paint); } -void GM::SetResourcePath(const char* resourcePath) { - gResourcePath = resourcePath; -} - -SkString GM::GetResourcePath() { - return SkString(gResourcePath); -} - // need to explicitly declare this, or we get some weird infinite loop llist template GMRegistry* SkTRegistry::gHead; diff --git a/gm/gm.h b/gm/gm.h index a48976b..90de96f 100644 --- a/gm/gm.h +++ b/gm/gm.h @@ -96,8 +96,13 @@ namespace skiagm { // GM's getISize bounds. void drawSizeBounds(SkCanvas*, SkColor); - static void SetResourcePath(const char*); - static SkString GetResourcePath(); + static void SetResourcePath(const char* resourcePath) { + gResourcePath = resourcePath; + } + + static SkString& GetResourcePath() { + return gResourcePath; + } bool isCanvasDeferred() const { return fCanvasIsDeferred; } void setCanvasIsDeferred(bool isDeferred) { @@ -110,7 +115,7 @@ namespace skiagm { } protected: - static const char* gResourcePath; + static SkString gResourcePath; virtual void onOnceBeforeDraw() {} virtual void onDraw(SkCanvas*) = 0; diff --git a/tests/Test.cpp b/tests/Test.cpp index b904d5a..e57427a 100644 --- a/tests/Test.cpp +++ b/tests/Test.cpp @@ -1,10 +1,10 @@ + /* * Copyright 2011 Google Inc. * * Use of this source code is governed by a BSD-style license that can be * found in the LICENSE file. */ - #include "Test.h" #include "SkCommandLineFlags.h" @@ -41,8 +41,6 @@ void Reporter::endTest(Test* test) { /////////////////////////////////////////////////////////////////////////////// -const char* Test::gResourcePath; - Test::Test() : fReporter(NULL), fPassed(true) {} Test::~Test() { @@ -123,9 +121,8 @@ SkString Test::GetTmpDir() { return SkString(tmpDir); } -void Test::SetResourcePath(const char* resourcePath) { - gResourcePath = resourcePath; -} +static const char* gResourcePath = NULL; +void Test::SetResourcePath(const char* resourcePath) { gResourcePath = resourcePath; } SkString Test::GetResourcePath() { return SkString(gResourcePath); diff --git a/tests/Test.h b/tests/Test.h index 90d072e..4d2cf2b 100644 --- a/tests/Test.h +++ b/tests/Test.h @@ -73,8 +73,6 @@ namespace skiatest { virtual void onRun(Reporter*) = 0; private: - static const char* gResourcePath; - Reporter* fReporter; SkString fName; bool fPassed; -- 2.7.4