Address senorblanco's comments on r600.

Don't inline constructors and destructors.
Include license in test file.
A few nits
Also, cleanup a couple compile warnings.

Review URL: http://codereview.appspot.com/2279043

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

diff --git a/include/pdf/SkPDFCatalog.h b/include/pdf/SkPDFCatalog.h
index fa15938..932f1c8 100644 (file)
--- a/include/pdf/SkPDFCatalog.h
+++ b/include/pdf/SkPDFCatalog.h
@@ -23,19 +23,15 @@
 
 /** \class SkPDFCatalog
 
-    The PDF catalog object manages object numbers and when emitted to the
-    PDF stream, indexes all the objects in the file by offset.
+    The PDF catalog manages object numbers and file offsets.  It is used
+    to create the PDF cross reference table.
 */
 class SkPDFCatalog {
 public:
     /** Create a PDF catalog.
      */
-    SkPDFCatalog()
-        : fNextObjNum(1),
-          fStartedAssigningObjNums(false),
-          fAssigningFirstPageObjNums(false) {
-    }
-    virtual ~SkPDFCatalog() {}
+    SkPDFCatalog();
+    ~SkPDFCatalog();
 
     /** Add the passed object to the catalog.
      *  @param obj        The object to add.

diff --git a/include/pdf/SkPDFTypes.h b/include/pdf/SkPDFTypes.h
index 247bd37..9324595 100644 (file)
--- a/include/pdf/SkPDFTypes.h
+++ b/include/pdf/SkPDFTypes.h
@@ -35,8 +35,8 @@ class SkPDFObject : public SkRefCnt {
 public:
     /** Create a PDF object.
      */
-    SkPDFObject() {}
-    virtual ~SkPDFObject() {}
+    SkPDFObject();
+    virtual ~SkPDFObject();
 
     /** Subclasses must implement this method to print the object to the
      *  PDF file.
@@ -76,8 +76,8 @@ public:
     /** Create a reference to an existing SkPDFObject.
      *  @param obj The object to reference.
      */
-    explicit SkPDFObjRef(SkPDFObject* obj) : fObj(obj) {}
-    virtual ~SkPDFObjRef() {}
+    explicit SkPDFObjRef(SkPDFObject* obj);
+    virtual ~SkPDFObjRef();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog,
@@ -97,8 +97,8 @@ public:
     /** Create a PDF integer (usually for indirect reference purposes).
      *  @param value An integer value between 2^31 - 1 and -2^31.
      */
-    SkPDFInt(int32_t value) : fValue(value) {}
-    virtual ~SkPDFInt() {}
+    explicit SkPDFInt(int32_t value);
+    virtual ~SkPDFInt();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog,
@@ -117,8 +117,8 @@ public:
     /** Create a PDF real number.
      *  @param value A real value.
      */
-    SkPDFScalar(SkScalar value) : fValue(value) {}
-    virtual ~SkPDFScalar() {}
+    explicit SkPDFScalar(SkScalar value);
+    virtual ~SkPDFScalar();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog,
@@ -137,9 +137,9 @@ public:
     /** Create a PDF string. Maximum length (in bytes) is 65,535.
      *  @param value A string value.
      */
-    SkPDFString(const char value[]);
-    SkPDFString(const SkString& value);
-    virtual ~SkPDFString() {}
+    explicit SkPDFString(const char value[]);
+    explicit SkPDFString(const SkString& value);
+    virtual ~SkPDFString();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog,
@@ -147,7 +147,7 @@ public:
     virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect);
 
 private:
-    static const int kMaxLen = 65535;
+    static const uint32_t kMaxLen = 65535;
 
     const SkString fValue;
 
@@ -163,9 +163,9 @@ public:
     /** Create a PDF name object. Maximum length is 127 bytes.
      *  @param value The name.
      */
-    SkPDFName(const char name[]);
-    SkPDFName(const SkString& name);
-    virtual ~SkPDFName() {}
+    explicit SkPDFName(const char name[]);
+    explicit SkPDFName(const SkString& name);
+    virtual ~SkPDFName();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog,
@@ -173,11 +173,11 @@ public:
     virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect);
 
 private:
-    static const int kMaxLen = 127;
+    static const uint32_t kMaxLen = 127;
 
     const SkString fValue;
 
-    SkString formatName(const SkString& input);
+    static SkString formatName(const SkString& input);
 };
 
 /** \class SkPDFArray
@@ -188,7 +188,7 @@ class SkPDFArray : public SkPDFObject {
 public:
     /** Create a PDF array. Maximum length is 8191.
      */
-    SkPDFArray() {}
+    SkPDFArray();
     virtual ~SkPDFArray();
 
     // The SkPDFObject interface.
@@ -234,7 +234,7 @@ class SkPDFDict : public SkPDFObject {
 public:
     /** Create a PDF dictionary. Maximum number of entries is 4095.
      */
-    SkPDFDict() {}
+    SkPDFDict();
     virtual ~SkPDFDict();
 
     // The SkPDFObject interface.

diff --git a/src/pdf/SkPDFCatalog.cpp b/src/pdf/SkPDFCatalog.cpp
index dd3cd7e..1349da1 100644 (file)
--- a/src/pdf/SkPDFCatalog.cpp
+++ b/src/pdf/SkPDFCatalog.cpp
@@ -18,6 +18,14 @@
 #include "SkPDFTypes.h"
 #include "SkStream.h"
 
+SkPDFCatalog::SkPDFCatalog()
+    : fNextObjNum(1),
+      fStartedAssigningObjNums(false),
+      fAssigningFirstPageObjNums(false) {
+}
+
+SkPDFCatalog::~SkPDFCatalog() {}
+
 void SkPDFCatalog::addObject(SkPDFObject* obj, bool onFirstPage) {
     SkASSERT(findObjectIndex(obj) == -1);
     SkASSERT(!fStartedAssigningObjNums);

diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index 83f133e..f5f23ca 100644 (file)
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -18,6 +18,9 @@
 #include "SkPDFTypes.h"
 #include "SkStream.h"
 
+SkPDFObject::SkPDFObject() {}
+SkPDFObject::~SkPDFObject() {}
+
 size_t SkPDFObject::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
     SkDynamicMemoryWStream buffer;
     emitObject(&buffer, catalog, indirect);
@@ -31,6 +34,9 @@ void SkPDFObject::emitIndirectObject(SkWStream* stream, SkPDFCatalog* catalog) {
     stream->writeText("\nendobj\n");
 }
 
+SkPDFObjRef::SkPDFObjRef(SkPDFObject* obj) : fObj(obj) {}
+SkPDFObjRef::~SkPDFObjRef() {}
+
 size_t SkPDFObject::getIndirectOutputSize(SkPDFCatalog* catalog) {
     return catalog->getObjectNumberSize(this) + strlen(" obj\n") +
         this->getOutputSize(catalog, false) + strlen("\nendobj\n");
@@ -48,6 +54,9 @@ size_t SkPDFObjRef::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
     return catalog->getObjectNumberSize(fObj.get()) + strlen(" R");
 }
 
+SkPDFInt::SkPDFInt(int32_t value) : fValue(value) {}
+SkPDFInt::~SkPDFInt() {}
+
 void SkPDFInt::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
                           bool indirect) {
     if (indirect)
@@ -55,6 +64,9 @@ void SkPDFInt::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
     stream->writeDecAsText(fValue);
 }
 
+SkPDFScalar::SkPDFScalar(SkScalar value) : fValue(value) {}
+SkPDFScalar::~SkPDFScalar() {}
+
 void SkPDFScalar::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
                              bool indirect) {
     if (indirect)
@@ -70,6 +82,8 @@ SkPDFString::SkPDFString(const SkString& value)
     : fValue(formatString(value)) {
 }
 
+SkPDFString::~SkPDFString() {}
+
 void SkPDFString::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
                              bool indirect) {
     if (indirect)
@@ -90,7 +104,7 @@ SkString SkPDFString::formatString(const SkString& input) {
     // a 7-bit clean string should require little escaping.
     bool sevenBitClean = true;
     for (size_t i = 0; i < input.size(); i++) {
-        if (input[i] > 0x7F || input[i] < ' ') {
+        if (input[i] & 0x80 || input[i] < ' ') {
             sevenBitClean = false;
             break;
         }
@@ -117,6 +131,7 @@ SkString SkPDFString::formatString(const SkString& input) {
 
 SkPDFName::SkPDFName(const char name[]) : fValue(formatName(SkString(name))) {}
 SkPDFName::SkPDFName(const SkString& name) : fValue(formatName(name)) {}
+SkPDFName::~SkPDFName() {}
 
 void SkPDFName::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
                            bool indirect) {
@@ -129,12 +144,13 @@ size_t SkPDFName::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
     return fValue.size();
 }
 
+// static
 SkString SkPDFName::formatName(const SkString& input) {
     SkASSERT(input.size() <= kMaxLen);
 
     SkString result("/");
     for (size_t i = 0; i < input.size(); i++) {
-        if (input[i] > 0x7F || input[i] < '!' || input[i] == '#') {
+        if (input[i] & 0x80 || input[i] < '!' || input[i] == '#') {
             result.append("#");
             result.appendHex(input[i], 2);
         } else {
@@ -145,6 +161,7 @@ SkString SkPDFName::formatName(const SkString& input) {
     return result;
 }
 
+SkPDFArray::SkPDFArray() {}
 SkPDFArray::~SkPDFArray() {
     fValue.safeUnrefAll();
 }
@@ -193,6 +210,7 @@ void SkPDFArray::append(SkPDFObject* value) {
     fValue.push(value);
 }
 
+SkPDFDict::SkPDFDict() {}
 SkPDFDict::~SkPDFDict() {
     for (int i = 0; i < fValue.count(); i++) {
         SkSafeUnref(fValue[i].key);

diff --git a/tests/PDFPrimitivesTest.cpp b/tests/PDFPrimitivesTest.cpp
index 81d8c0a..7e4465b 100644 (file)
--- a/tests/PDFPrimitivesTest.cpp
+++ b/tests/PDFPrimitivesTest.cpp
@@ -1,3 +1,19 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 #include <string>
 
 #include "Test.h"