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

Issue 6449057: Renamed and made public SkGradientShaderBases's 'commonAsAGradient' to 'getGradientTableBitmap', an… (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 4 months ago by rileya
Modified:
12 years, 4 months ago
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Renamed and made public SkGradientShaderBases's 'commonAsAGradient' to 'getGradientTableBitmap', and use that instead of asABitmap in gradient custom stage setup. Also tidied up Gr gradient implementation constructors, to take the appropriate SkGradientShaderBase subclass, and where necessary (namely 2pt radial/conical) made them obtain extra parameters from that object, rather than passing them in in addition to it. Committed: https://code.google.com/p/skia/source/detail?r=4808

Patch Set 1 #

Patch Set 2 : Fix Windows line endings in SkGradientShaderPriv.h #

Total comments: 3

Patch Set 3 : GrScalar->SkScalar in Radial/Conical parameter getters. #

Patch Set 4 : Add comment about old GrGradientEffect constructor. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+318 lines, -314 lines) Patch
M src/effects/gradients/SkGradientShader.cpp View 2 chunks +7 lines, -7 lines 0 comments Download
M src/effects/gradients/SkGradientShaderPriv.h View 1 2 3 1 chunk +267 lines, -264 lines 0 comments Download
M src/effects/gradients/SkLinearGradient.h View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/gradients/SkLinearGradient.cpp View 2 chunks +4 lines, -4 lines 0 comments Download
M src/effects/gradients/SkRadialGradient.h View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/gradients/SkRadialGradient.cpp View 2 chunks +4 lines, -3 lines 0 comments Download
M src/effects/gradients/SkSweepGradient.h View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/gradients/SkSweepGradient.cpp View 2 chunks +3 lines, -2 lines 0 comments Download
M src/effects/gradients/SkTwoPointConicalGradient.h View 1 2 2 chunks +6 lines, -3 lines 0 comments Download
M src/effects/gradients/SkTwoPointConicalGradient.cpp View 4 chunks +9 lines, -13 lines 0 comments Download
M src/effects/gradients/SkTwoPointRadialGradient.h View 1 2 2 chunks +6 lines, -3 lines 0 comments Download
M src/effects/gradients/SkTwoPointRadialGradient.cpp View 3 chunks +9 lines, -12 lines 0 comments Download

Messages

Total messages: 6
rileya
http://codereview.appspot.com/6449057/diff/2001/src/effects/gradients/SkGradientShaderPriv.h File src/effects/gradients/SkGradientShaderPriv.h (right): http://codereview.appspot.com/6449057/diff/2001/src/effects/gradients/SkGradientShaderPriv.h#newcode225 src/effects/gradients/SkGradientShaderPriv.h:225: GrGradientEffect(GrTexture* texture); I wanted to completely remove the gradient ...
12 years, 4 months ago (2012-07-27 15:14:51 UTC) #1
bsalomon
We need to figure out a better way to test the shader generator... perhaps some ...
12 years, 4 months ago (2012-07-27 15:21:08 UTC) #2
rileya
On 2012/07/27 15:21:08, bsalomon wrote: > We need to figure out a better way to ...
12 years, 4 months ago (2012-07-27 15:32:44 UTC) #3
bsalomon
LGTM On 2012/07/27 15:32:44, rileya wrote: > On 2012/07/27 15:21:08, bsalomon wrote: > > We ...
12 years, 4 months ago (2012-07-27 15:34:34 UTC) #4
rileya
On 2012/07/27 15:34:34, bsalomon wrote: > LGTM > > On 2012/07/27 15:32:44, rileya wrote: > ...
12 years, 4 months ago (2012-07-27 15:44:50 UTC) #5
bsalomon
12 years, 4 months ago (2012-07-27 15:48:30 UTC) #6
On 2012/07/27 15:44:50, rileya wrote:
> On 2012/07/27 15:34:34, bsalomon wrote:
> > LGTM
> > 
> > On 2012/07/27 15:32:44, rileya wrote:
> > > On 2012/07/27 15:21:08, bsalomon wrote:
> > > > We need to figure out a better way to test the shader generator...
perhaps
> > > some
> > > > method for GrCustomStages to auto-register themselves to with the tester
> and
> > > > able to configure themselves randomly.
> > > 
> > > Is there anything I can do towards that end with this CL?
> > 
> > I think it's a separate task. Can you stick a comment on the texture cons
that
> > it is there for the unit test?
> > 
> > > 
> > >
> >
>
http://codereview.appspot.com/6449057/diff/2001/src/effects/gradients/SkTwoPo...
> > > File src/effects/gradients/SkTwoPointConicalGradient.h (right):
> > > 
> > >
> >
>
http://codereview.appspot.com/6449057/diff/2001/src/effects/gradients/SkTwoPo...
> > > src/effects/gradients/SkTwoPointConicalGradient.h:67: GrScalar
getCenterX1()
> > > const { return SkPoint::Distance(fCenter1, fCenter2); }
> > > On 2012/07/27 15:21:08, bsalomon wrote:
> > > > Let's call them SkScalar since they're in an Sk class.
> > > 
> > > Fixed.
> 
> Alright, added the comment.

LGTM
Sign in to reply to this message.

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