Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(611)

Issue 4631067: Alternate approach to enabling PDF in gm (gyp changes) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 5 months ago by epoger
Modified:
13 years, 5 months ago
Reviewers:
Steve VanDeBogart
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Steve- Here is an alternate approach to http://codereview.appspot.com/4626071/ . Highlights include: - deletes skia_pdf_support flag: if your target depends on pdf.gyp, you get pdf support - pdf.gyp now sets SK_PDF_SUPPORT and adds include/pdf dir for all targets that depend on it - makes gypfiles smaller instead of bigger Changes you made in http://codereview.appspot.com/4626071/ that are NOT included in this CL: (should we add them?) - addition of zlib stuff - change to SkPDFDevice::drawTextOnPath() Tested as follows on both Mac and Linux, without any apparent problems: make clean make out/Debug/tests out/Debug/gm [and ran SampleApp]

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -34 lines) Patch
M gyp/gm.gyp View 2 chunks +1 line, -7 lines 0 comments Download
M gyp/pdf.gyp View 1 chunk +5 lines, -0 lines 0 comments Download
M gyp/tests.gyp View 2 chunks +1 line, -27 lines 0 comments Download

Messages

Total messages: 3
epoger
13 years, 5 months ago (2011-06-24 15:07:13 UTC) #1
Steve VanDeBogart
LGTM Should SampleApp.gyp also depend on PDF? None of the generated PDFs will have compressed ...
13 years, 5 months ago (2011-06-24 19:01:26 UTC) #2
epoger
13 years, 5 months ago (2011-06-24 19:10:08 UTC) #3
On 2011/06/24 19:01:26, Steve VanDeBogart wrote:
> LGTM
> 
> Should SampleApp.gyp also depend on PDF?

It already does...

> 
> None of the generated PDFs will have compressed streams.  If you look in
> src/core/SkFlate.cpp, if SK_ZLIB_INCLUDE isn't defined, compression isn't
done. 
> But currently, PDF is the only thing using zlib, so it only makes sense to
> enable it (and link to it) when doing PDF.  I'll work on another CL to add a
> SkFlate target and make pdf depend on that.

Sounds good.

Committed as r1713
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b