Add an option to have a PdfViewer in SampleApp. Add a parameter --pdfDir to pass the dir with pdfs.…

Add an option to have a PdfViewer in SampleApp. Add a parameter --pdfDir to pass the dir with pdfs. Add flag --sort to sort samples acording to title, so foo.skp follows foo.pdf and we can quickly compare diffs.
The option needs to be enabled manually by developer to be compiled locally, since the pdf viewer is work in progress.

R=reed@google.com

Committed: https://code.google.com/p/skia/source/detail?r=10004

[vandebo (ex-Chrome), 2013-06-26 18:45:26]

https://codereview.chromium.org/17904006/diff/1/gyp/SampleApp.gyp
File gyp/SampleApp.gyp (right):

https://codereview.chromium.org/17904006/diff/1/gyp/SampleApp.gyp#newcode5
gyp/SampleApp.gyp:5: # podofo dependency has plentry od warnings in the header
files that will be errors otherwise
typo: od => of

https://codereview.chromium.org/17904006/diff/1/gyp/SampleApp.gyp#newcode6
gyp/SampleApp.gyp:6: #'skia_warnings_as_errors': 0,
Can't you just set this in the podofo build target?

[edisonn, 2013-06-26 18:52:30]

https://codereview.chromium.org/17904006/diff/1/gyp/SampleApp.gyp
File gyp/SampleApp.gyp (right):

https://codereview.chromium.org/17904006/diff/1/gyp/SampleApp.gyp#newcode5
gyp/SampleApp.gyp:5: # podofo dependency has plentry od warnings in the header
files that will be errors otherwise
On 2013/06/26 18:45:26, vandebo wrote:
> typo: od => of

Done.

https://codereview.chromium.org/17904006/diff/1/gyp/SampleApp.gyp#newcode6
gyp/SampleApp.gyp:6: #'skia_warnings_as_errors': 0,
On 2013/06/26 18:45:26, vandebo wrote:
> Can't you just set this in the podofo build target?

it is done, but there are warnings in the public .h headers, so any project that
uses them will end up with a warning

I will try to get rid of podofo in the next week

[vandebo (ex-Chrome), 2013-06-26 21:58:36]

Code seems fine to me. I'll let Mike give the final ok.

https://codereview.chromium.org/17904006/diff/6001/gyp/SampleApp.gyp
File gyp/SampleApp.gyp (right):

https://codereview.chromium.org/17904006/diff/6001/gyp/SampleApp.gyp#newcode6
gyp/SampleApp.gyp:6: #'skia_warnings_as_errors': 0,
use a direct_dependent_settings section in order to propagate this up.

https://codereview.chromium.org/17904006/diff/6001/samplecode/SampleApp.cpp
File samplecode/SampleApp.cpp (right):

https://codereview.chromium.org/17904006/diff/6001/samplecode/SampleApp.cpp#n...
samplecode/SampleApp.cpp:62: SkString fFilename;
private?

https://codereview.chromium.org/17904006/diff/6001/samplecode/SamplePdfFileVi...
File samplecode/SamplePdfFileViewer.cpp (right):

https://codereview.chromium.org/17904006/diff/6001/samplecode/SamplePdfFileVi...
samplecode/SamplePdfFileViewer.cpp:41: SkPdfDoc doc(path);
We now have SkPDFDocument and SkPdfDoc?  That's not at all confusing ;-)  Maybe
SkPDFDoc should be SkParsedPDF or something along those lines?

[reed1, 2013-06-26 22:10:46]

There is likely a lot of naming/API/hiding/etc. to be done to the "public"
headers for this experiment. SkPdfDoc is certainly confusing given the current
SkPDFDocument.

For just wacking this into SampleApp, not sure how much cleanup we need
initially. Possibly the easiest route is to in fact create a wrapper class that
is the temporary "public" face for all of this. Something very minimal with a
name like SkParsedPDF.

e.g.

class SkParsedPDF {
public:
    static SkParsedPDF* CreateFromFile(const char path[]);

    SkSize getSize() const;
    int countPages() const;
    void drawPage(int page, SkCanvas*);

private:
    .... * fImpl;
};

This would allow this *new* header to have a minimal footprint, basically hiding
everything behind an impl.

[edisonn, 2013-07-02 23:20:33]

https://codereview.chromium.org/17904006/diff/6001/gyp/SampleApp.gyp
File gyp/SampleApp.gyp (right):

https://codereview.chromium.org/17904006/diff/6001/gyp/SampleApp.gyp#newcode6
gyp/SampleApp.gyp:6: #'skia_warnings_as_errors': 0,
fixed (removed)
On 2013/06/26 21:58:36, vandebo wrote:
> use a direct_dependent_settings section in order to propagate this up.

https://codereview.chromium.org/17904006/diff/6001/samplecode/SampleApp.cpp
File samplecode/SampleApp.cpp (right):

https://codereview.chromium.org/17904006/diff/6001/samplecode/SampleApp.cpp#n...
samplecode/SampleApp.cpp:62: SkString fFilename;
On 2013/06/26 21:58:36, vandebo wrote:
> private?

Done.

https://codereview.chromium.org/17904006/diff/6001/samplecode/SamplePdfFileVi...
File samplecode/SamplePdfFileViewer.cpp (right):

https://codereview.chromium.org/17904006/diff/6001/samplecode/SamplePdfFileVi...
samplecode/SamplePdfFileViewer.cpp:41: SkPdfDoc doc(path);
Renamed to SkPodofoParsedPDF

There will be a SkNativeParsedPDF soon too

On 2013/06/26 21:58:36, vandebo wrote:
> We now have SkPDFDocument and SkPdfDoc?  That's not at all confusing ;-) 
Maybe
> SkPDFDoc should be SkParsedPDF or something along those lines?

[edisonn, 2013-07-03 11:23:09]

On 2013/06/26 22:10:46, reed1 wrote:
> There is likely a lot of naming/API/hiding/etc. to be done to the "public"
> headers for this experiment. SkPdfDoc is certainly confusing given the current
> SkPDFDocument.
> 
> For just wacking this into SampleApp, not sure how much cleanup we need
> initially. Possibly the easiest route is to in fact create a wrapper class
that
> is the temporary "public" face for all of this. Something very minimal with a
> name like SkParsedPDF.
> 
> e.g.
> 
> class SkParsedPDF {
> public:
>     static SkParsedPDF* CreateFromFile(const char path[]);
> 
>     SkSize getSize() const;
>     int countPages() const;
>     void drawPage(int page, SkCanvas*);
> 
> private:
>     .... * fImpl;
> };
> 
> This would allow this *new* header to have a minimal footprint, basically
hiding
> everything behind an impl.

done, podofo usage is behinf impl now

[reed1, 2013-07-10 18:45:53]

lgtm

[edisonn, 2013-07-11 14:28:08]

Committed patchset #8 manually as r10004 (presubmit successful).