Reland 'Add path utils, plus a test for it.'

Build SkPathJoin and SkBasename on windows also.

Previous CL did not build on Windows because the two functions were
accidentally placed inside an ifdef that did not include windows.
Move the functions to the top of the file, and add a comment by the
endif for clarity.

Previously reviewed at https://codereview.chromium.org/15747004/

Review URL: https://codereview.chromium.org/15740024

git-svn-id: http://skia.googlecode.com/svn/trunk@9295 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/gm/factory.cpp b/gm/factory.cpp
index 4538cda..a7356a9 100644 (file)
--- a/gm/factory.cpp
+++ b/gm/factory.cpp
@@ -11,6 +11,7 @@
 #include "SkData.h"
 #include "SkImageDecoder.h"
 #include "SkLruImageCache.h"
+#include "SkOSFile.h"
 #include "SkStream.h"
 
 namespace skiagm {
@@ -24,13 +25,9 @@ public:
 
 protected:
     virtual void onOnceBeforeDraw() SK_OVERRIDE {
-        SkString filename(INHERITED::gResourcePath);
-        if (!filename.endsWith("/") && !filename.endsWith("\\")) {
-            filename.append("/");
-        }
-
         // Copyright-free file from http://openclipart.org/detail/29213/paper-plane-by-ddoo
-        filename.append("plane.png");
+        SkString filename = SkOSPath::SkPathJoin(INHERITED::gResourcePath.c_str(),
+                                                 "plane.png");
 
         SkAutoTUnref<SkStream> stream(SkStream::NewFromFile(filename.c_str()));
         if (NULL != stream.get()) {

diff --git a/gm/gm_expectations.cpp b/gm/gm_expectations.cpp
index 08461bd..2f92120 100644 (file)
--- a/gm/gm_expectations.cpp
+++ b/gm/gm_expectations.cpp
@@ -37,15 +37,6 @@ namespace skiagm {
         va_end(args);
     }
 
-    SkString SkPathJoin(const char *rootPath, const char *relativePath) {
-        SkString result(rootPath);
-        if (!result.endsWith(SkPATH_SEPARATOR)) {
-            result.appendUnichar(SkPATH_SEPARATOR);
-        }
-        result.append(relativePath);
-        return result;
-    }
-
     Json::Value CreateJsonTree(Json::Value expectedResults,
                                Json::Value actualResultsFailed,
                                Json::Value actualResultsFailureIgnored,
@@ -194,7 +185,7 @@ namespace skiagm {
     // IndividualImageExpectationsSource class...
 
     Expectations IndividualImageExpectationsSource::get(const char *testName) {
-        SkString path = SkPathJoin(fRootDir.c_str(), testName);
+        SkString path = SkOSPath::SkPathJoin(fRootDir.c_str(), testName);
         SkBitmap referenceBitmap;
         bool decodedReferenceBitmap =
             SkImageDecoder::DecodeFile(path.c_str(), &referenceBitmap,

diff --git a/gm/gm_expectations.h b/gm/gm_expectations.h
index 8efb986..55122d4 100644 (file)
--- a/gm/gm_expectations.h
+++ b/gm/gm_expectations.h
@@ -32,16 +32,6 @@ namespace skiagm {
 
     void gm_fprintf(FILE *stream, const char format[], ...);
 
-    /**
-     * Assembles rootPath and relativePath into a single path, like this:
-     * rootPath/relativePath
-     *
-     * Uses SkPATH_SEPARATOR, to work on all platforms.
-     *
-     * TODO(epoger): This should probably move into SkOSFile.h
-     */
-    SkString SkPathJoin(const char *rootPath, const char *relativePath);
-
     Json::Value CreateJsonTree(Json::Value expectedResults,
                                Json::Value actualResultsFailed,
                                Json::Value actualResultsFailureIgnored,

diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 3481cf8..3c4f27d 100644 (file)
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -222,7 +222,7 @@ public:
         filename.append(renderModeDescriptor);
         filename.appendUnichar('.');
         filename.append(suffix);
-        return SkPathJoin(path, filename.c_str());
+        return SkOSPath::SkPathJoin(path, filename.c_str());
     }
 
     /* since PNG insists on unpremultiplying our alpha, we take no

diff --git a/gm/image.cpp b/gm/image.cpp
index 8578e51..71fe0e1 100644 (file)
--- a/gm/image.cpp
+++ b/gm/image.cpp
@@ -31,6 +31,9 @@ static SkData* fileToData(const char path[]) {
 }
 
 static void drawJpeg(SkCanvas* canvas, const SkISize& size) {
+    // TODO: Make this draw a file that is checked in, so it can
+    // be exercised on machines other than mike's. Will require a
+    // rebaseline.
     SkAutoDataUnref data(fileToData("/Users/mike/Downloads/skia.google.jpeg"));
     SkImage* image = SkImage::NewEncodedData(data);
     if (image) {

diff --git a/gyp/tests.gyp b/gyp/tests.gyp
index a15671d..10a4ba4 100644 (file)
--- a/gyp/tests.gyp
+++ b/gyp/tests.gyp
@@ -75,6 +75,7 @@
         '../tests/Matrix44Test.cpp',
         '../tests/MemsetTest.cpp',
         '../tests/MetaDataTest.cpp',
+        '../tests/OSPathTest.cpp',
         '../tests/PackBitsTest.cpp',
         '../tests/PaintTest.cpp',
         '../tests/ParsePathTest.cpp',

diff --git a/include/core/SkOSFile.h b/include/core/SkOSFile.h
index 257b66a..8564d43 100644 (file)
--- a/include/core/SkOSFile.h
+++ b/include/core/SkOSFile.h
@@ -105,4 +105,27 @@ private:
     uint16_t*   fStr;
 };
 
+/**
+ *  Functions for modifying SkStrings which represent paths on the filesystem.
+ */
+class SkOSPath {
+public:
+    /**
+     * Assembles rootPath and relativePath into a single path, like this:
+     * rootPath/relativePath
+     *
+     * Uses SkPATH_SEPARATOR, to work on all platforms.
+     */
+    static SkString SkPathJoin(const char *rootPath, const char *relativePath);
+
+    /**
+     *  Return the name of the file, ignoring the directory structure.
+     *  Behaves like python's os.path.basename. If the fullPath is
+     *  /dir/subdir/, an empty string is returned.
+     *  @param fullPath Full path to the file.
+     *  @return SkString The basename of the file - anything beyond the
+     *      final slash, or the full name if there is no slash.
+     */
+    static SkString SkBasename(const char* fullPath);
+};
 #endif

diff --git a/src/utils/SkOSFile.cpp b/src/utils/SkOSFile.cpp
index 478a0cc..0a40375 100644 (file)
--- a/src/utils/SkOSFile.cpp
+++ b/src/utils/SkOSFile.cpp
@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -7,6 +6,28 @@
  */
 #include "SkOSFile.h"
 
+SkString SkOSPath::SkPathJoin(const char *rootPath, const char *relativePath) {
+    SkString result(rootPath);
+    if (!result.endsWith(SkPATH_SEPARATOR)) {
+        result.appendUnichar(SkPATH_SEPARATOR);
+    }
+    result.append(relativePath);
+    return result;
+}
+
+SkString SkOSPath::SkBasename(const char* fullPath) {
+    if (!fullPath) {
+        return SkString();
+    }
+    const char* filename = strrchr(fullPath, SkPATH_SEPARATOR);
+    if (NULL == filename) {
+        filename = fullPath;
+    } else {
+        ++filename;
+    }
+    return SkString(filename);
+}
+
 #ifdef SK_BUILD_FOR_WIN
 
 static uint16_t* concat_to_16(const char src[], const char suffix[])
@@ -225,5 +246,4 @@ bool SkOSFile::Iter::next(SkString* name, bool getDir)
     }
     return false;
 }
-
-#endif
+#endif // if one of:SK_BUILD_FOR_MAC, SK_BUILD_FOR_UNIX, SK_BUILD_FOR_ANDROID,SK_BUILD_FOR_IOS

diff --git a/tests/OSPathTest.cpp b/tests/OSPathTest.cpp
new file mode 100644 (file)
index 0000000..96ff8a7
--- /dev/null
+++ b/tests/OSPathTest.cpp
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkString.h"
+#include "SkOSFile.h"
+#include "Test.h"
+
+/**
+ *  Test SkPathJoin and SkBasename.
+ *  Will use SkPathJoin to append filename to dir, test that it works correctly,
+ *  and tests using SkBasename on the result.
+ *  @param reporter Reporter for test conditions.
+ *  @param dir String representing the path to a folder. May or may not
+ *      end with SkPATH_SEPARATOR.
+ *  @param filename String representing the basename of a file. Must NOT
+ *      contain SkPATH_SEPARATOR.
+ */
+static void test_dir_with_file(skiatest::Reporter* reporter, SkString dir,
+                               SkString filename) {
+    // If filename contains SkPATH_SEPARATOR, the tests will fail.
+    SkASSERT(!filename.contains(SkPATH_SEPARATOR));
+
+    // Tests for SkOSPath::SkPathJoin and SkOSPath::SkBasename
+
+    // fullName should be "dir<SkPATH_SEPARATOR>file"
+    SkString fullName = SkOSPath::SkPathJoin(dir.c_str(), filename.c_str());
+
+    // fullName should be the combined size of dir and file, plus one if
+    // dir did not include the final path separator.
+    size_t expectedSize = dir.size() + filename.size();
+    if (!dir.endsWith(SkPATH_SEPARATOR)) {
+        expectedSize++;
+    }
+    REPORTER_ASSERT(reporter, fullName.size() == expectedSize);
+
+    SkString basename = SkOSPath::SkBasename(fullName.c_str());
+
+    // basename should be the same as filename
+    REPORTER_ASSERT(reporter, basename.equals(filename));
+
+    // basename will not contain a path separator
+    REPORTER_ASSERT(reporter, !basename.contains(SkPATH_SEPARATOR));
+
+    // Now take the basename of filename, which should be the same as filename.
+    basename = SkOSPath::SkBasename(filename.c_str());
+    REPORTER_ASSERT(reporter, basename.equals(filename));
+}
+
+static void test_os_path_utils_tests(skiatest::Reporter* reporter) {
+    SkString dir("dir");
+    SkString filename("file");
+    test_dir_with_file(reporter, dir, filename);
+
+    // Now make sure this works with a path separator at the end of dir.
+    dir.appendUnichar(SkPATH_SEPARATOR);
+    test_dir_with_file(reporter, dir, filename);
+
+    // Test with a sub directory.
+    dir.append("subDir");
+    test_dir_with_file(reporter, dir, filename);
+
+    // Basename of a directory with a path separator at the end is empty.
+    dir.appendUnichar(SkPATH_SEPARATOR);
+    SkString baseOfDir = SkOSPath::SkBasename(dir.c_str());
+    REPORTER_ASSERT(reporter, baseOfDir.size() == 0);
+
+    // Basename of NULL is an empty string.
+    SkString empty = SkOSPath::SkBasename(NULL);
+    REPORTER_ASSERT(reporter, empty.size() == 0);
+}
+
+#include "TestClassDef.h"
+DEFINE_TESTCLASS("OSPath", OSPathTestClass, test_os_path_utils_tests)

diff --git a/tools/skimage_main.cpp b/tools/skimage_main.cpp
index f52cf32..98cde50 100644 (file)
--- a/tools/skimage_main.cpp
+++ b/tools/skimage_main.cpp
@@ -69,27 +69,10 @@ static SkImageDecoder::Format guess_format_from_suffix(const char suffix[]) {
     return SkImageDecoder::kUnknown_Format;
 }
 
-/**
- *  Return the name of the file, ignoring the directory structure.
- *  Does not create a new string.
- *  @param fullPath Full path to the file.
- *  @return string The basename of the file - anything beyond the final slash, or the full name
- *      if there is no slash.
- *  TODO: Might this be useful as a utility function in SkOSFile? Would it be more appropriate to
- *  create a new string?
- */
-static const char* SkBasename(const char* fullPath) {
-    const char* filename = strrchr(fullPath, SkPATH_SEPARATOR);
-    if (NULL == filename || *++filename == '\0') {
-        filename = fullPath;
-    }
-    return filename;
-}
-
 static void make_outname(SkString* dst, const char outDir[], const char src[],
                          const char suffix[]) {
-    const char* basename = SkBasename(src);
-    dst->set(skiagm::SkPathJoin(outDir, basename));
+    SkString basename = SkOSPath::SkBasename(src);
+    dst->set(SkOSPath::SkPathJoin(outDir, basename.c_str()));
     if (!dst->endsWith(suffix)) {
         const char* cstyleDst = dst->c_str();
         const char* dot = strrchr(cstyleDst, '.');
@@ -272,8 +255,7 @@ static bool write_subset(const char* writePath, const char* filename, const char
     SkASSERT(bitmapFromDecodeSubset != NULL);
 
     // Create a subdirectory to hold the results of decodeSubset.
-    // TODO: Move SkPathJoin into SkOSFile.h
-    SkString dir = skiagm::SkPathJoin(writePath, "subsets");
+    SkString dir = SkOSPath::SkPathJoin(writePath, "subsets");
     if (!sk_mkdir(dir.c_str())) {
         gFailedSubsetDecodes.push_back().printf("Successfully decoded %s from %s, but failed to "
                                                 "create a directory to write to.", subsetDim,
@@ -298,7 +280,7 @@ static bool write_subset(const char* writePath, const char* filename, const char
         return false;
     }
 
-    SkString dirExtracted = skiagm::SkPathJoin(writePath, "extracted");
+    SkString dirExtracted = SkOSPath::SkPathJoin(writePath, "extracted");
     if (!sk_mkdir(dirExtracted.c_str())) {
         gFailedSubsetDecodes.push_back().printf("Successfully decoded %s from %s, but failed to "
                                                 "create a directory for extractSubset comparison.",
@@ -335,7 +317,8 @@ static void decodeFileAndWrite(const char srcPath[], const SkString* writePath)
     }
 
     // Create a string representing just the filename itself, for use in json expectations.
-    const char* filename = SkBasename(srcPath);
+    SkString basename = SkOSPath::SkBasename(srcPath);
+    const char* filename = basename.c_str();
 
     if (compare_to_expectations_if_necessary(bitmap, filename, &gDecodeFailures)) {
         gSuccessfulDecodes.push_back().printf("%s [%d %d]", srcPath, bitmap.width(),