Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(233)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 7124058: Let them eat GrEffectRef. (Closed)

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

Description

Let them eat GrEffectRef. Changes the remaining existing code that operates on naked GrEffects to GrEffectRef. Committed: https://code.google.com/p/skia/source/detail?r=7321

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : Rebase and update bicubic effect #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+307 lines, -291 lines) Patch
M include/gpu/GrBackendEffectFactory.h View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M include/gpu/GrEffect.h View 1 2 6 chunks +46 lines, -12 lines 4 comments Download
M include/gpu/GrEffectStage.h View 1 2 3 chunks +2 lines, -10 lines 0 comments Download
M include/gpu/GrTBackendEffectFactory.h View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M src/effects/SkBicubicImageFilter.cpp View 1 2 6 chunks +8 lines, -11 lines 0 comments Download
M src/effects/SkBlendImageFilter.cpp View 1 2 7 chunks +9 lines, -9 lines 0 comments Download
M src/effects/SkColorMatrixFilter.cpp View 1 2 4 chunks +5 lines, -6 lines 0 comments Download
M src/effects/SkDisplacementMapEffect.cpp View 1 2 7 chunks +15 lines, -14 lines 0 comments Download
M src/effects/SkLightingImageFilter.cpp View 1 2 18 chunks +30 lines, -34 lines 0 comments Download
M src/effects/SkMagnifierImageFilter.cpp View 1 2 7 chunks +14 lines, -17 lines 2 comments Download
M src/effects/SkMatrixConvolutionImageFilter.cpp View 1 2 7 chunks +16 lines, -19 lines 0 comments Download
M src/effects/SkMorphologyImageFilter.cpp View 1 2 7 chunks +9 lines, -11 lines 0 comments Download
M src/effects/SkTableColorFilter.cpp View 1 2 5 chunks +6 lines, -8 lines 0 comments Download
M src/effects/gradients/SkGradientShader.cpp View 1 2 3 chunks +4 lines, -4 lines 0 comments Download
M src/effects/gradients/SkGradientShaderPriv.h View 1 2 2 chunks +1 line, -2 lines 0 comments Download
M src/effects/gradients/SkLinearGradient.cpp View 1 2 2 chunks +2 lines, -3 lines 0 comments Download
M src/effects/gradients/SkRadialGradient.cpp View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M src/effects/gradients/SkSweepGradient.cpp View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M src/effects/gradients/SkTwoPointConicalGradient.cpp View 1 2 7 chunks +9 lines, -11 lines 0 comments Download
M src/effects/gradients/SkTwoPointRadialGradient.cpp View 1 2 7 chunks +9 lines, -12 lines 0 comments Download
M src/gpu/GrContext.cpp View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M src/gpu/GrDrawTarget.cpp View 1 2 3 chunks +5 lines, -5 lines 0 comments Download
M src/gpu/SkGpuDevice.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/effects/GrConfigConversionEffect.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/effects/GrConfigConversionEffect.cpp View 1 2 7 chunks +28 lines, -29 lines 0 comments Download
M src/gpu/effects/GrConvolutionEffect.h View 1 2 2 chunks +13 lines, -13 lines 0 comments Download
M src/gpu/effects/GrConvolutionEffect.cpp View 1 2 6 chunks +8 lines, -10 lines 0 comments Download
M src/gpu/effects/GrSimpleTextureEffect.h View 1 2 2 chunks +5 lines, -5 lines 0 comments Download
M src/gpu/effects/GrSimpleTextureEffect.cpp View 1 2 2 chunks +3 lines, -5 lines 0 comments Download
M src/gpu/effects/GrTextureDomainEffect.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/effects/GrTextureDomainEffect.cpp View 1 2 7 chunks +12 lines, -15 lines 0 comments Download
M src/gpu/gl/GrGLEffect.h View 1 2 1 chunk +22 lines, -1 line 0 comments Download
M src/gpu/gl/GrGLEffect.cpp View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M src/gpu/gl/GrGLProgram.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/gl/GrGLShaderBuilder.cpp View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M src/gpu/gl/GrGpuGL_program.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M tests/GLProgramsTest.cpp View 1 2 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 3
bsalomon
More cookie-cutter fun. (I'm leaving the office in 5 mins. I don't plan to commit ...
11 years, 9 months ago (2013-01-17 19:53:45 UTC) #1
robertphillips
LGTM + nits/questions https://codereview.appspot.com/7124058/diff/6008/include/gpu/GrEffect.h File include/gpu/GrEffect.h (right): https://codereview.appspot.com/7124058/diff/6008/include/gpu/GrEffect.h#newcode64 include/gpu/GrEffect.h:64: */ Maybe a comment as to ...
11 years, 9 months ago (2013-01-22 19:36:45 UTC) #2
bsalomon
11 years, 9 months ago (2013-01-22 19:55:59 UTC) #3 
https://codereview.appspot.com/7124058/diff/6008/include/gpu/GrEffect.h
File include/gpu/GrEffect.h (right):

https://codereview.appspot.com/7124058/diff/6008/include/gpu/GrEffect.h#newco...
include/gpu/GrEffect.h:64: */
On 2013/01/22 19:36:45, robertphillips wrote:
> Maybe a comment as to what private inheritance accomplishes here (i.e., //
hide
> ref & unref methods)

Done.

https://codereview.appspot.com/7124058/diff/6008/include/gpu/GrEffect.h#newco...
include/gpu/GrEffect.h:159: 
On 2013/01/22 19:36:45, robertphillips wrote:
> Would making AutoEffectUnref a friend be better?

We will call these directly in the future (when saving a draw state).

https://codereview.appspot.com/7124058/diff/6008/src/effects/SkMagnifierImage...
File src/effects/SkMagnifierImageFilter.cpp (right):

https://codereview.appspot.com/7124058/diff/6008/src/effects/SkMagnifierImage...
src/effects/SkMagnifierImageFilter.cpp:232: bool
GrMagnifierEffect::onIsEqual(const GrEffectRef& sBase) const {
On 2013/01/22 19:36:45, robertphillips wrote:
> extra space?

Done.
Sign in to reply to this message.

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