Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(266)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 4662054: [PDF] Update PDF shader code in the same way as r1684.

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 6 months ago by Steve VanDeBogart
Modified:
14 years, 6 months ago
Reviewers:
reed1
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlecode.com/svn/trunk
Visibility:
Public.
[PDF] Update PDF shader code in the same way as r1684.

Patch Set 1 #

Patch Set 2 : Address comments #

Created: 14 years, 6 months ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -18 lines) Patch
M src/pdf/SkPDFShader.cpp View 1 2 chunks +24 lines, -18 lines 0 comments Download
Total messages: 6
|
Steve VanDeBogart
Mike, how did you test your change in 1684? Will setting both radii in gm/gradients.cpp ...
14 years, 6 months ago (2011年06月25日 01:31:01 UTC) #1
Mike, how did you test your change in 1684? Will setting both radii in
gm/gradients.cpp trigger the condition? I could slog through the math again,
but if you remember....
Sign in to reply to this message.
reed1
Perhaps we can keep the check for a the samein both cases: either both test ...
14 years, 6 months ago (2011年06月27日 13:14:48 UTC) #2
Perhaps we can keep the check for a the samein both cases: either both test a ==
0 or both tests a != 0, just to make it easier to see that its really 2
different equations (quad and linear), but with common code in the middle.
2. Try using string.appendf("%d %d roll min max etc");
This will allow you to put more of the logic in a single line, making it easier
to follow sometimes.
Sign in to reply to this message.
Steve VanDeBogart
On 2011年06月27日 13:14:48, reed1 wrote: > Perhaps we can keep the check for a the ...
14 years, 6 months ago (2011年06月27日 17:37:53 UTC) #3
On 2011年06月27日 13:14:48, reed1 wrote:
> Perhaps we can keep the check for a the samein both cases: either both test a
==
> 0 or both tests a != 0, just to make it easier to see that its really 2
Done.
> different equations (quad and linear), but with common code in the middle.
> 
> 2. Try using string.appendf("%d %d roll min max etc");
> 
> This will allow you to put more of the logic in a single line, making it
easier
> to follow sometimes.
Done.
I just have have to figure out a test...
Sign in to reply to this message.
reed1
I'll submit a test soon (need it for the gpu case as well) On Mon, ...
14 years, 6 months ago (2011年06月27日 17:41:21 UTC) #4
I'll submit a test soon (need it for the gpu case as well)
On Mon, Jun 27, 2011 at 1:37 PM, <vandebo@chromium.org> wrote:
> On 2011年06月27日 13:14:48, reed1 wrote:
>>
>> Perhaps we can keep the check for a the samein both cases: either both
>
> test a ==
>>
>> 0 or both tests a != 0, just to make it easier to see that its really
>
> 2
>
> Done.
>
>> different equations (quad and linear), but with common code in the
>
> middle.
>
>> 2. Try using string.appendf("%d %d roll min max etc");
>
>> This will allow you to put more of the logic in a single line, making
>
> it easier
>>
>> to follow sometimes.
>
> Done.
>
> I just have have to figure out a test...
>
> http://codereview.appspot.com/4662054/
>
Sign in to reply to this message.
reed1
gm/gradients.cpp has been updated with gradients_degenerate_2pt On Mon, Jun 27, 2011 at 1:40 PM, Mike ...
14 years, 6 months ago (2011年06月27日 18:12:05 UTC) #5
gm/gradients.cpp has been updated with gradients_degenerate_2pt
On Mon, Jun 27, 2011 at 1:40 PM, Mike Reed <reed@google.com> wrote:
> I'll submit a test soon (need it for the gpu case as well)
>
> On Mon, Jun 27, 2011 at 1:37 PM,  <vandebo@chromium.org> wrote:
>> On 2011年06月27日 13:14:48, reed1 wrote:
>>>
>>> Perhaps we can keep the check for a the samein both cases: either both
>>
>> test a ==
>>>
>>> 0 or both tests a != 0, just to make it easier to see that its really
>>
>> 2
>>
>> Done.
>>
>>> different equations (quad and linear), but with common code in the
>>
>> middle.
>>
>>> 2. Try using string.appendf("%d %d roll min max etc");
>>
>>> This will allow you to put more of the logic in a single line, making
>>
>> it easier
>>>
>>> to follow sometimes.
>>
>> Done.
>>
>> I just have have to figure out a test...
>>
>> http://codereview.appspot.com/4662054/
>>
>
Sign in to reply to this message.
Steve VanDeBogart
On 2011年06月27日 18:12:05, reed1 wrote: > gm/gradients.cpp has been updated with gradients_degenerate_2pt Thanks, that helps. ...
14 years, 6 months ago (2011年06月27日 20:19:02 UTC) #6
On 2011年06月27日 18:12:05, reed1 wrote:
> gm/gradients.cpp has been updated with gradients_degenerate_2pt
Thanks, that helps. For some reason the PDF code has the colors backward and I
don't see why. I have to come back to this later.
Sign in to reply to this message.
|
Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /