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

Issue 4631078: [PDF] Improve complex xfer mode support. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 9 months ago by Steve VanDeBogart
Modified:
6 years, 5 months ago
Reviewers:
edisonn, reed1
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlecode.com/svn/trunk
Visibility:
Public.

Description

[PDF] Improve complex xfer mode support. Xfer mode applies only to the shape of the source drawing, not everything in the clip as in currently implemented. It's just that the current gm examples draw a shape that fills the visible layer. R=edisonn@google.com, reed@google.com Committed: https://code.google.com/p/skia/source/detail?r=12034

Patch Set 1 #

Total comments: 18

Patch Set 2 : Rebase #

Patch Set 3 : Rebase #

Patch Set 4 : Add SrcATop DstATop and Modulate support #

Patch Set 5 : Add GM from 42473002 #

Patch Set 6 : Add other gm ignores and fix indent #

Total comments: 17

Patch Set 7 : Address comments #

Patch Set 8 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+386 lines, -164 lines) Patch
M expectations/gm/ignored-tests.txt View 1 2 3 4 5 6 7 1 chunk +9 lines, -0 lines 0 comments Download
M gm/xfermodes.cpp View 1 2 3 4 5 6 6 chunks +126 lines, -40 lines 0 comments Download
M include/pdf/SkPDFDevice.h View 1 2 3 3 chunks +6 lines, -5 lines 0 comments Download
M src/pdf/SkPDFDevice.cpp View 1 2 3 4 5 20 chunks +242 lines, -115 lines 0 comments Download
M src/pdf/SkPDFGraphicState.cpp View 1 2 3 2 chunks +3 lines, -4 lines 0 comments Download

Messages

Total messages: 15
Steve VanDeBogart
8 years, 9 months ago (2011-06-29 22:25:02 UTC) #1
Chris Guillory
I'm going to need an in person explanation of this change. http://codereview.appspot.com/4631078/diff/1/src/pdf/SkPDFDevice.cpp File src/pdf/SkPDFDevice.cpp (right): ...
8 years, 9 months ago (2011-07-01 00:28:13 UTC) #2
Steve VanDeBogart
http://codereview.appspot.com/4631078/diff/1/src/pdf/SkPDFDevice.cpp File src/pdf/SkPDFDevice.cpp (right): http://codereview.appspot.com/4631078/diff/1/src/pdf/SkPDFDevice.cpp#newcode500 src/pdf/SkPDFDevice.cpp:500: /* If the shape is different than the alpha ...
8 years, 9 months ago (2011-07-01 01:27:48 UTC) #3
Chris Guillory
I'd like to take another look at this. Thanks for explaining. http://codereview.appspot.com/4631078/diff/1/include/pdf/SkPDFDevice.h File include/pdf/SkPDFDevice.h (right): ...
8 years, 9 months ago (2011-07-01 01:30:39 UTC) #4
Steve VanDeBogart
Here are my replies to Chris's comments. https://codereview.appspot.com/4631078/diff/1/include/pdf/SkPDFDevice.h File include/pdf/SkPDFDevice.h (right): https://codereview.appspot.com/4631078/diff/1/include/pdf/SkPDFDevice.h#newcode192 include/pdf/SkPDFDevice.h:192: void drawFormXObjectWithMask(int ...
7 years, 5 months ago (2012-10-25 21:14:57 UTC) #5
Steve VanDeBogart
Mike, Edison, please review. I cleaned up the xfer mode improvements that were languishing. This ...
6 years, 5 months ago (2013-10-29 21:09:26 UTC) #6
reed1
On 2011/07/01 01:27:48, Steve VanDeBogart wrote: > http://codereview.appspot.com/4631078/diff/1/src/pdf/SkPDFDevice.cpp > File src/pdf/SkPDFDevice.cpp (right): > > http://codereview.appspot.com/4631078/diff/1/src/pdf/SkPDFDevice.cpp#newcode500 ...
6 years, 5 months ago (2013-10-30 13:27:07 UTC) #7
reed1
https://codereview.appspot.com/4631078/diff/90002/include/pdf/SkPDFDevice.h File include/pdf/SkPDFDevice.h (right): https://codereview.appspot.com/4631078/diff/90002/include/pdf/SkPDFDevice.h#newcode284 include/pdf/SkPDFDevice.h:284: SkPath* shape); can this guy be const?
6 years, 5 months ago (2013-10-30 13:31:28 UTC) #8
edisonn
LGTM - only nits, feel free to ignore them https://codereview.appspot.com/4631078/diff/90002/gm/xfermodes.cpp File gm/xfermodes.cpp (right): https://codereview.appspot.com/4631078/diff/90002/gm/xfermodes.cpp#newcode56 gm/xfermodes.cpp:56: ...
6 years, 5 months ago (2013-10-30 14:16:35 UTC) #9
edisonn
https://codereview.appspot.com/4631078/diff/90002/src/pdf/SkPDFDevice.cpp File src/pdf/SkPDFDevice.cpp (right): https://codereview.appspot.com/4631078/diff/90002/src/pdf/SkPDFDevice.cpp#newcode2109 src/pdf/SkPDFDevice.cpp:2109: // Assumes that xobject has been canonicalized (so we ...
6 years, 5 months ago (2013-10-30 14:50:03 UTC) #10
Steve VanDeBogart
On 2013/10/30 13:27:07, reed1 wrote: > On 2011/07/01 01:27:48, Steve VanDeBogart wrote: > > http://codereview.appspot.com/4631078/diff/1/src/pdf/SkPDFDevice.cpp ...
6 years, 5 months ago (2013-10-30 19:17:22 UTC) #11
edisonn
LGTM
6 years, 5 months ago (2013-10-30 19:32:57 UTC) #12
reed1
lgtm is this going to land, since it is not using the chromium codereview site?
6 years, 5 months ago (2013-10-30 20:08:09 UTC) #13
Steve VanDeBogart
On 2013/10/30 20:08:09, reed1 wrote: > lgtm > > is this going to land, since ...
6 years, 5 months ago (2013-10-30 20:24:30 UTC) #14
Steve VanDeBogart
6 years, 5 months ago (2013-10-30 20:48:12 UTC) #15
Message was sent while issue was closed.
Committed patchset #8 manually as r12034 (presubmit successful).
Sign in to reply to this message.

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