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

Issue 6801044: Make the orientation of a texture accessible from and known by GrSurface. (Closed)

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

Description

Make the orientation of a texture accessible from and known by GrSurface. R=robertphillips@google.com Committed: https://code.google.com/p/skia/source/detail?r=6148

Patch Set 1 #

Patch Set 2 : #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -69 lines) Patch
M include/gpu/GrRenderTarget.h View 3 chunks +6 lines, -5 lines 0 comments Download
M include/gpu/GrSurface.h View 1 4 chunks +25 lines, -6 lines 0 comments Download
M include/gpu/GrTexture.h View 1 chunk +2 lines, -2 lines 0 comments Download
M include/gpu/GrTypes.h View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/SkLightingImageFilter.cpp View 2 chunks +2 lines, -3 lines 0 comments Download
M src/effects/SkMatrixConvolutionImageFilter.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/SkMorphologyImageFilter.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/effects/GrTextureDomainEffect.cpp View 1 chunk +1 line, -2 lines 0 comments Download
M src/gpu/gl/GrGLProgram.h View 1 chunk +2 lines, -3 lines 0 comments Download
M src/gpu/gl/GrGLProgram.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/gl/GrGLRenderTarget.h View 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/gl/GrGLRenderTarget.cpp View 2 chunks +5 lines, -2 lines 4 comments Download
M src/gpu/gl/GrGLTexture.h View 4 chunks +1 line, -17 lines 2 comments Download
M src/gpu/gl/GrGLTexture.cpp View 2 chunks +4 lines, -5 lines 2 comments Download
M src/gpu/gl/GrGpuGL.h View 1 chunk +1 line, -2 lines 0 comments Download
M src/gpu/gl/GrGpuGL.cpp View 4 chunks +4 lines, -5 lines 0 comments Download
M src/gpu/gl/GrGpuGL_program.cpp View 5 chunks +6 lines, -11 lines 0 comments Download

Messages

Total messages: 3
bsalomon
11 years, 10 months ago (2012-10-26 18:29:40 UTC) #1
robertphillips
LGTM + some questions https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLRenderTarget.cpp File src/gpu/gl/GrGLRenderTarget.cpp (right): https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLRenderTarget.cpp#newcode52 src/gpu/gl/GrGLRenderTarget.cpp:52: desc.fConfig, desc.fSampleCnt), indent? https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLRenderTarget.cpp#newcode75 src/gpu/gl/GrGLRenderTarget.cpp:75: ...
11 years, 10 months ago (2012-10-26 18:38:10 UTC) #2
bsalomon
11 years, 10 months ago (2012-10-26 18:49:47 UTC) #3
https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLRenderTarget.cpp
File src/gpu/gl/GrGLRenderTarget.cpp (right):

https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLRenderTarget....
src/gpu/gl/GrGLRenderTarget.cpp:52: desc.fConfig, desc.fSampleCnt),
On 2012/10/26 18:38:10, robertphillips wrote:
> indent?

It's a param to INHERITED() not to MakeDesc().

https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLRenderTarget....
src/gpu/gl/GrGLRenderTarget.cpp:75: desc.fConfig, desc.fSampleCnt),
On 2012/10/26 18:38:10, robertphillips wrote:
> indent?

ditto

https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLTexture.cpp
File src/gpu/gl/GrGLTexture.cpp (right):

https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLTexture.cpp#n...
src/gpu/gl/GrGLTexture.cpp:44: const Desc& textureDesc)
On 2012/10/26 18:38:10, robertphillips wrote:
> Why are we passing both the textureDesc and the fOrigin?

GrGLTexture::Desc derives from GrTextureDesc. GrTextureDesc doesn't know about
Origin; it's one of the things added by the subclass.

We don't want GrTextureDesc to know about origin because it is a public struct
and we want to control the origin assigned via the private constructors used
internally.

https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLTexture.h
File src/gpu/gl/GrGLTexture.h (right):

https://codereview.appspot.com/6801044/diff/2001/src/gpu/gl/GrGLTexture.h#new...
src/gpu/gl/GrGLTexture.h:61: bool            fOwnsID;
On 2012/10/26 18:38:10, robertphillips wrote:
> Should this move up to GrTextureDesc?

See answer to other Q... we could give clients the ability to control this but I
think we'd want to think through that a bit more. We'd probably want to have a
kDefault or kBackend to choose whatever the native origin is.
Sign in to reply to this message.

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