pdfviewer: more code comments + concat the pdf matrix with the existing matrix in canvas, instead of reseting it.

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

git-svn-id: http://skia.googlecode.com/svn/trunk@11735 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/experimental/PdfViewer/SkPdfFont.h b/experimental/PdfViewer/SkPdfFont.h
index 12cc899..85ce651 100644
--- a/experimental/PdfViewer/SkPdfFont.h
+++ b/experimental/PdfViewer/SkPdfFont.h
@@ -5,6 +5,10 @@
  * found in the LICENSE file.
  */
 
+// TODO(edisonn): this file not commented much on purpose.
+// It will probably need heavy refactoring soon anyway to support all encodings, fonts and
+// proper text sizing and spacing
+
 #ifndef SkPdfFont_DEFINED
 #define SkPdfFont_DEFINED
 
diff --git a/experimental/PdfViewer/SkPdfGraphicsState.cpp b/experimental/PdfViewer/SkPdfGraphicsState.cpp
index 9f3917b..2a01b14 100644
--- a/experimental/PdfViewer/SkPdfGraphicsState.cpp
+++ b/experimental/PdfViewer/SkPdfGraphicsState.cpp
@@ -33,8 +33,7 @@
     // TODO(edisonn): miter, ...
     if (stroking) {
         paint->setStrokeWidth(SkDoubleToScalar(fLineWidth));
-        // TODO(edisonn): perf, avoid allocs allocs
-        // of the intervals
+        // TODO(edisonn): perf, avoid allocs of the intervals
         if (fDashArrayLength > 0) {
             paint->setPathEffect(new SkDashPathEffect(fDashArray,
                                                       fDashArrayLength,
diff --git a/experimental/PdfViewer/SkPdfGraphicsState.h b/experimental/PdfViewer/SkPdfGraphicsState.h
index 656ee8f..094b794 100644
--- a/experimental/PdfViewer/SkPdfGraphicsState.h
+++ b/experimental/PdfViewer/SkPdfGraphicsState.h
@@ -13,34 +13,39 @@
 #include "SkPdfConfig.h"
 #include "SkPdfUtils.h"
 
+class SkPdfAllocator;
 class SkPdfFont;
 class SkPdfDoc;
+class SkPdfNativeDoc;
 class SkPdfNativeObject;
 class SkPdfResourceDictionary;
 class SkPdfSoftMaskDictionary;
+#include "SkTypes.h"
 
-class SkPdfNativeDoc;
-class SkPdfAllocator;
 
-// TODO(edisonn): move this class in include/core?
-// Ref objects can't be dealt unless we use a specific class initialization
-// The difference between SkTDStackNew and SkTDStack is that SkTDStackNew uses new/delete
-// to be a manage c++ stuff (like initializations)
+// TODO(edisonn): move SkTDStackNester class in its own private file
 
 // Adobe limits it to 28, so 256 should be more than enough
 #define MAX_NESTING 256
 
-#include "SkTypes.h"
-template <typename T> class SkTDStackNew : SkNoncopyable {
+/** \class SkTDStackNester
+ *
+ * The difference between SkTDStackNester and SkTDStack is that:
+ *   - SkTDStackNester uses new/delete to manage initializations
+ *   - Supports nest/unnest which simulates a stack of stack. unnest will pop all the
+ *     objects pushed since the last nest
+ */
+
+template <typename T> class SkTDStackNester : SkNoncopyable {
 public:
-    SkTDStackNew() : fCount(0), fTotalCount(0), fLocalCount(0) {
+    SkTDStackNester() : fCount(0), fTotalCount(0), fLocalCount(0) {
         fInitialRec.fNext = NULL;
         fRec = &fInitialRec;
 
     //  fCount = kSlotCount;
     }
 
-    ~SkTDStackNew() {
+    ~SkTDStackNester() {
         Rec* rec = fRec;
         while (rec != &fInitialRec) {
             Rec* next = rec->fNext;
@@ -157,7 +162,9 @@
     int     fNestingLevel;
 };
 
-// TODO(edisonn): better class design.
+/** \class SkTDStackNester
+ *   Operates on stroking or non-stroking properties.
+ */
 class SkPdfColorOperator {
 
     /*
@@ -168,9 +175,7 @@
      */
 
     // TODO(edisonn): implement the array part too
-    // does not own the char*
 // TODO(edisonn): remove this public, let fields be private
-// TODO(edisonn): make color space an enum!
 public:
     NotOwnedString fColorSpace;
     SkPdfNativeObject* fPattern;
@@ -187,15 +192,14 @@
     SkColor fColor;
     double fOpacity;  // ca or CA
 
-    // TODO(edisonn): add here other color space options.
-
 public:
     void setRGBColor(SkColor color) {
         // TODO(edisonn): ASSERT DeviceRGB is the color space.
         fPattern = NULL;
         fColor = color;
     }
-    // TODO(edisonn): double check the default values for all fields.
+
+    // TODO(edisonn): implement the default values for all fields.
     SkPdfColorOperator() : fPattern(NULL), fColor(SK_ColorBLACK), fOpacity(1) {
         NotOwnedString::init(&fColorSpace, "DeviceRGB");
     }
@@ -216,7 +220,9 @@
     }
 };
 
-// TODO(edisonn): better class design.
+/** \class SkTDStackNester
+ *   Operates on stroking or non-stroking properties.
+ */
 struct SkPdfGraphicsState {
     // TODO(edisonn): deprecate and remove these!
     double              fCurPosX;
@@ -235,9 +241,8 @@
     SkPdfResourceDictionary* fResources;
 
 
-    // TODO(edisonn): move most of these in canvas/paint?
-    // we could have some in canvas (matrixes?),
-    // some in 2 paints (stroking paint and non stroking paint)
+    // TODO(edisonn): Can we move most of these in canvas/paint?
+    // Might need to strore some properties in 2 paints (stroking paint and non stroking paint)
 
 //    TABLE 4.2 Device-independent graphics state parameters
 /*
@@ -457,10 +462,7 @@
                                           installation-dependent.
  */
 
-
-
     // TODO(edisonn): some defaults are contextual, they could on colorspace, pdf version, ...
-
     SkPdfGraphicsState() {
         fCurPosX      = 0.0;
         fCurPosY      = 0.0;
@@ -493,14 +495,15 @@
     void applyGraphicsState(SkPaint* paint, bool stroking);
 };
 
+/** \class SkPdfContext
+ *   The context of the drawing. The document we draw from, the current stack of objects, ...
+ */
 class SkPdfContext {
 public:
-    SkTDStackNew<SkPdfNativeObject*>  fObjectStack;
-    SkTDStackNew<SkPdfGraphicsState>  fStateStack;
+    SkTDStackNester<SkPdfNativeObject*>  fObjectStack;
+    SkTDStackNester<SkPdfGraphicsState>  fStateStack;
     SkPdfGraphicsState              fGraphicsState;
     SkPdfNativeDoc*                 fPdfDoc;
-    // TODO(edisonn): the allocator, could be freed after the page is done drawing, so we have the
-    // pixels on the screen asap.
     SkPdfAllocator*                 fTmpPageAllocator;
     SkMatrix                        fOriginalMatrix;
 
diff --git a/experimental/PdfViewer/SkPdfRenderer.cpp b/experimental/PdfViewer/SkPdfRenderer.cpp
index 2a78765..6c6c666 100644
--- a/experimental/PdfViewer/SkPdfRenderer.cpp
+++ b/experimental/PdfViewer/SkPdfRenderer.cpp
@@ -3062,6 +3062,8 @@
     SkAssertResult(pdfContext.fOriginalMatrix.setPolyToPoly(pdfSpace, skiaSpace, 4));
     SkTraceMatrix(pdfContext.fOriginalMatrix, "Original matrix");
 
+    pdfContext.fOriginalMatrix.postConcat(canvas->getTotalMatrix());
+
     pdfContext.fGraphicsState.fCTM = pdfContext.fOriginalMatrix;
     pdfContext.fGraphicsState.fContentStreamMatrix = pdfContext.fOriginalMatrix;
     pdfContext.fGraphicsState.fMatrixTm = pdfContext.fGraphicsState.fCTM;
@@ -3071,7 +3073,7 @@
     canvas->clipRect(dst, SkRegion::kIntersect_Op, true);
 #endif
 
-    canvas->setMatrix(pdfContext.fOriginalMatrix);
+    canvas->concat(pdfContext.fOriginalMatrix);
 
     doPage(&pdfContext, canvas, fPdfDoc->page(page));
 
diff --git a/experimental/PdfViewer/SkPdfRenderer.h b/experimental/PdfViewer/SkPdfRenderer.h
index c4cfe23..51b5649 100644
--- a/experimental/PdfViewer/SkPdfRenderer.h
+++ b/experimental/PdfViewer/SkPdfRenderer.h
@@ -19,11 +19,62 @@
 class SkStream;
 class SkString;
 
+// What kind of content to render.
 enum SkPdfContent {
     kNoForms_SkPdfContent,
     kAll_SkPdfContent,
 };
 
+/** \class SkPdfRenderer
+ *
+ *  The SkPdfRenderer class is used to render a PDF into canvas.
+ *
+ */
+class SkPdfRenderer {
+public:
+    SkPdfRenderer() : fPdfDoc(NULL) {}
+    virtual ~SkPdfRenderer() {unload();}
+
+    // Render a specific page into the canvas, in a specific rectangle.
+    bool renderPage(int page, SkCanvas* canvas, const SkRect& dst) const;
+
+    // TODO(edisonn): deprecated, should be removed!
+    bool load(const SkString inputFileName);
+
+    // TODO(edisonn): replace it with a SkSmartStream which would know to to efficiently
+    // deal with a HTTP stream.
+    bool load(SkStream* stream);
+
+    // Unloads the pdf document.
+    void unload();
+
+    // Returns true if we succesfully loaded a document.
+    bool loaded() const {return fPdfDoc != NULL && pages() > 0;}
+
+    // Returns the number of pages in the loaded pdf.
+    int pages() const;
+
+    // Returns the MediaBox of a page. Can be used by client to crate a canvas.
+    SkRect MediaBox(int page) const;
+
+    // TODO(edisonn): for testing only, probably it should be removed, unless some client wants to
+    // let users know how much memory the PDF needs.
+    size_t bytesUsed() const;
+
+private:
+    SkPdfNativeDoc* fPdfDoc;
+};
+
+// For testing only, reports stats about rendering, like how many operations failed, or are NYI, ...
+void reportPdfRenderStats();
+
+// Renders a page of a pdf in a bitmap.
+bool SkPDFNativeRenderToBitmap(SkStream* stream,
+                               SkBitmap* output,
+                               int page = 0,
+                               SkPdfContent content = kAll_SkPdfContent,
+                               double dpi = 72.0);
+
 // TODO(edisonn): add options to render forms, checkboxes, ...
 // TODO(edisonn): Add API for Forms viewing and editing
 // e.g. SkBitmap getPage(int page);
@@ -31,38 +82,4 @@
 //      SkForm getForm(int formID); // SkForm(SkRect, .. other data)
 // TODO (edisonn): Add intend when loading pdf, for example: for viewing, for parsing content, ...
 
-class SkPdfRenderer {
-    SkPdfNativeDoc* fPdfDoc;
-public:
-    SkPdfRenderer() : fPdfDoc(NULL) {}
-    virtual ~SkPdfRenderer() {unload();}
-
-    bool renderPage(int page, SkCanvas* canvas, const SkRect& dst) const;
-
-    // TODO(edisonn): deprecated, should be removed!
-    bool load(const SkString inputFileName);
-
-    bool load(SkStream* stream);
-
-    void unload();
-
-    bool loaded() const {return fPdfDoc != NULL;}
-
-    int pages() const;
-
-    SkRect MediaBox(int page) const;
-
-    // TODO(edisonn): for testing only, probably it should be removed, unless some client wants to
-    // let users know how much memory the PDF needs.
-    size_t bytesUsed() const;
-};
-
-void reportPdfRenderStats();
-
-bool SkPDFNativeRenderToBitmap(SkStream* stream,
-                               SkBitmap* output,
-                               int page = 0,
-                               SkPdfContent content = kAll_SkPdfContent,
-                               double dpi = 72.0);
-
 #endif  // SkPdfRenderer_DEFINED
diff --git a/experimental/PdfViewer/SkPdfReporter.cpp b/experimental/PdfViewer/SkPdfReporter.cpp
index 4506c33..68b4600 100644
--- a/experimental/PdfViewer/SkPdfReporter.cpp
+++ b/experimental/PdfViewer/SkPdfReporter.cpp
@@ -28,6 +28,10 @@
 }
 
 // TODO(edisonn): add a flag to set the minimum warning level
+// TODO(edisonn): get the address in the file, and report it.
+// TODO(edisonn): build a html file based on warnings which would showe the original pdf
+//                content, with tooltips where warnings/errors were reported.
+
 
 #ifdef PDF_REPORT
 void SkPdfReport(SkPdfIssueSeverity sev, SkPdfIssue issue,
diff --git a/experimental/PdfViewer/SkPdfReporter.h b/experimental/PdfViewer/SkPdfReporter.h
index 26c97cd..be93959 100644
--- a/experimental/PdfViewer/SkPdfReporter.h
+++ b/experimental/PdfViewer/SkPdfReporter.h
@@ -16,6 +16,8 @@
 
 // TODO(edisonn): ability to turn on asserts for known good files
 
+// Severity of the issue, if it something interesting info, the result of an NYI feature,
+// sme ignorable defect in pdf or a major issue.
 enum SkPdfIssueSeverity {
     kInfo_SkPdfIssueSeverity,
     kCodeWarning_SkPdfIssueSeverity, // e.g. like NYI, PDF file is Ok.
@@ -27,6 +29,7 @@
     _kCount__SkPdfIssueSeverity
 };
 
+// The type of the issue.
 enum SkPdfIssue {
     kNoIssue_SkPdfIssue,
 
@@ -58,19 +61,30 @@
 
 #ifdef PDF_REPORT
 
+// Calls SkPdfReport(...) if report is true.
 void SkPdfReportIf(bool report,
                    SkPdfIssueSeverity sev, SkPdfIssue issue,
                    const char* context,
                    const SkPdfNativeObject* obj,
                    SkPdfContext* pdfContext);
+
+// Reports an issue, along with information where it happened, for example obj can be used to report
+// where exactly in th pdf there is a corruption
+// TODO(edisonn): add ability to report the callstack
 void SkPdfReport(SkPdfIssueSeverity sev, SkPdfIssue issue,
                  const char* context,
                  const SkPdfNativeObject* obj,
                  SkPdfContext* pdfContext);
+
+// Reports that an object does not have the expected type
+// TODO(edisonn): replace with SkPdfReportIfUnexpectedType() to simplify the callers?
+// TODO(edisonn): pass the keyword/operator too which triggers the issue.
 void SkPdfReportUnexpectedType(SkPdfIssueSeverity sev,
                                const char* context,
                                const SkPdfNativeObject* obj, int anyOfTypes,
                                SkPdfContext* pdfContext);
+
+// Code only in builds with reporting turn on.
 #define SkPdfREPORTCODE(code) code
 
 #else  // !PDF_REPORT
diff --git a/experimental/PdfViewer/SkPdfUtils.h b/experimental/PdfViewer/SkPdfUtils.h
index d05580f..87ed459 100644
--- a/experimental/PdfViewer/SkPdfUtils.h
+++ b/experimental/PdfViewer/SkPdfUtils.h
@@ -30,6 +30,8 @@
     kCount_SkPdfResult
 };
 
+// In order to operate fast, when we parse the pdf, we try not to allocate many new strings,
+// and if possible we refer the string in the pdf stream.
 struct NotOwnedString {
     const unsigned char* fBuffer;
     // TODO(edisonn): clean up, the last two bytes are used to signal if compression is used
diff --git a/experimental/PdfViewer/SkTrackDevice.h b/experimental/PdfViewer/SkTrackDevice.h
index 20f9e68..6018fcd 100644
--- a/experimental/PdfViewer/SkTrackDevice.h
+++ b/experimental/PdfViewer/SkTrackDevice.h
@@ -16,6 +16,11 @@
  *   A Track Device is used to track that callstack of an operation that affected some pixels.
  *   It can be used with SampleApp to investigate bugs (CL not checked in yet).
  *
+ *   every drawFoo is implemented as such:
+ *      before();   // - collects state of interesting pixels
+ *      INHERITED::drawFoo(...);
+ *      after();  // - checks if pixels of interest, and issue a breakpoint.
+ *
  */
 class SkTrackDevice : public SkBitmapDevice {
 public:
@@ -39,6 +44,8 @@
 
     virtual ~SkTrackDevice() {}
 
+    // Install a tracker - we can reuse the tracker between multiple devices, and the state of the
+    // tracker is preserved - number and location of poinbts, ...
     void installTracker(SkTracker* tracker) {
         fTracker = tracker;
         fTracker->newFrame();
diff --git a/experimental/PdfViewer/SkTracker.h b/experimental/PdfViewer/SkTracker.h
index 2f10fed..ba368f3 100644
--- a/experimental/PdfViewer/SkTracker.h
+++ b/experimental/PdfViewer/SkTracker.h
@@ -22,6 +22,18 @@
  *   A Tracker can be attached to a SkTrackDevice and it will store the track pixels.
  *   It can be used with SampleApp to investigate bugs (CL not checked in yet).
  *
+ *   The Tracker tracks 2 sets of points
+ *     A) one which is expected to issue breackpoints if the pixels are changes
+ *     B) one which if changes will disable the breackpoint
+ *   For point in A) there are two modes:
+ *     A.1) a breackpoint require that any of the points is changed
+ *     A.2) a breackpoint require that all of the points is changed
+ *   Points in B are allways in "any mode" - chaning the value of any pixel, will disable
+ *     the breackpoint
+ *
+ *   Point in A) are used to show what are the areas of interest, while poit in B are used to
+ *     disable breackpoints which would be issued in background change.
+ *
  */
 class SkTracker {
 public:
@@ -33,47 +45,60 @@
 
     virtual ~SkTracker() {}
 
+    // Clears all the points, but preserves the break mode.
     void clearPoints() {
         fCntExpectedTouched = 0;
         fCntExpectedUntouched = 0;
     }
 
+    // Enable the breackpoints.
     void enableTracking(bool b) {
         fEnabled = b;
     }
 
+    // Returns true if breackpoints are enabled.
     bool trackingEnabled() {
         return fEnabled;
     }
 
+    // Puts the tracker in Any mode.
     void any() {
         fBreakOnAny = true;
     }
 
+    // Puts the tracker in Any mode.
     void all() {
         fBreakOnAny = false;
     }
 
+    // returns true in in All mode. False for Any mode.
     bool requireAllExpectedTouched() {
         return !fBreakOnAny;
     }
 
+    // Returns the numbers of points in which if touched, would trigger a breackpoint.
     int cntExpectedTouched() {
         return fCntExpectedTouched;
     }
 
+    // Returns the points which if touched, would trigger a breackpoint.
+    // the Tracker owns the array
     const SkIPoint* expectedTouched() {
         return fExpectedTouched;
     }
 
+    // Returns the numbers of points in which if touched, would disable a breackpoint.
     int cntExpectedUntouched() {
         return fCntExpectedUntouched;
     }
 
+    // Returns the points which if touched, would disable a breackpoint.
+    // the Tracker owns the array
     const SkIPoint* expectedUntouched() {
         return fExpectedUntouched;
     }
 
+    // Adds a point which if changes in a drawFoo operation, would trigger a breakpoint.
     bool addExpectTouch(int x, int y) {
         if (fCntExpectedTouched >= MAX_TRACKING_POINTS) {
             return false;
@@ -86,6 +111,7 @@
         return true;
     }
 
+    // Adds a point which if changes in a drawFoo operation, would disable a breakpoint.
     bool addExpectUntouch(int x, int y) {
         if (fCntExpectedUntouched >= MAX_TRACKING_POINTS) {
             return false;
@@ -98,14 +124,17 @@
         return true;
     }
 
+    // Starts a new rendering session - reset the number of hits.
     void newFrame() {
         fHits = 0;
     }
 
+    // returns the number of breackpoints issues in this rendering session.
     int hits() {
         return fHits;
     }
 
+    // Called before drawFoo to store the state of the pixels
     void before(const SkBitmap& bitmap) {
         if (fCntExpectedTouched == 0) {
             return;
@@ -120,6 +149,7 @@
         }
     }
 
+    // Called after drawFoo to evaluate what pixels have changed, it could issue a breakpoint.
     // any/all of the expected touched has to be changed, and all expected untouched must be intact
     void after(const SkBitmap& bitmap) {
         if (fCntExpectedTouched == 0) {