Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(507)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 452823002: Separate GL path rendering state from GrGpuGL to GrGLPathRendering (Closed)

Created:
6 years, 4 months ago by Kimmo Kinnunen
Modified:
6 years, 4 months ago
Reviewers:
Chris Dalton, bsalomon
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@00xx-cherrypick-pathrendering-class
Project:
skia
Visibility:
Public.

Description

Separate GL path rendering state from GrGpuGL to GrGLPathRendering Separate GL path rendering state from GrGpuGL to GrGLPathRendering. This makes GrGpuGL code simpler. The intention is that while GrGpuGL represents the global environment for GL, the GrGLPathRendering represents the global environment for path rendering extension. Add GrPathRendering, a base class for path rendering, and inherit GrGLPathRendering from that. Move the path rendering virtual functions from GrGpu to GrPathRendering. Committed: https://skia.googlesource.com/skia/+/ccdaa0422501e5cbcba53d6bd19f2736f1beaef3

Patch Set 1 #

Patch Set 2 : beautify #

Total comments: 6

Patch Set 3 : address review comments #

Patch Set 4 : address review comment #

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+531 lines, -467 lines) Patch
M gyp/gpu.gypi View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/gpu/GrDrawTarget.h View 1 2 3 chunks +4 lines, -27 lines 0 comments Download
M src/gpu/GrGpu.h View 1 2 5 chunks +7 lines, -10 lines 0 comments Download
M src/gpu/GrGpu.cpp View 1 4 chunks +5 lines, -6 lines 0 comments Download
M src/gpu/GrInOrderDrawBuffer.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
A src/gpu/GrPathRendering.h View 1 2 1 chunk +75 lines, -0 lines 0 comments Download
M src/gpu/GrStencilAndCoverTextContext.cpp View 1 2 6 chunks +6 lines, -6 lines 0 comments Download
M src/gpu/gl/GrGLPath.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M src/gpu/gl/GrGLPathRange.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M src/gpu/gl/GrGLPathRendering.h View 1 2 3 3 chunks +53 lines, -11 lines 0 comments Download
M src/gpu/gl/GrGLPathRendering.cpp View 1 2 3 5 chunks +323 lines, -13 lines 0 comments Download
M src/gpu/gl/GrGLProgram.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/gl/GrGLProgramEffects.cpp View 1 chunk +8 lines, -6 lines 0 comments Download
M src/gpu/gl/GrGLUtil.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/gpu/gl/GrGLUtil.cpp View 1 chunk +25 lines, -0 lines 0 comments Download
M src/gpu/gl/GrGpuGL.h View 9 chunks +6 lines, -36 lines 0 comments Download
M src/gpu/gl/GrGpuGL.cpp View 1 2 3 4 11 chunks +3 lines, -341 lines 0 comments Download
M src/gpu/gl/GrGpuGL_program.cpp View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Kimmo Kinnunen
The issue that came up in review of GL ES patch. At least the class ...
6 years, 4 months ago (2014-08-08 07:55:04 UTC) #1
bsalomon
I'd like see Chris's thoughts on the specifics but I like the direction of this. ...
6 years, 4 months ago (2014-08-08 13:36:45 UTC) #2
Kimmo Kinnunen
Chris, ping. These are blocking on your input.
6 years, 4 months ago (2014-08-14 11:38:27 UTC) #3
Chris Dalton
On 2014/08/14 11:38:27, Kimmo Kinnunen wrote: > Chris, ping. These are blocking on your input. ...
6 years, 4 months ago (2014-08-14 16:21:16 UTC) #4
Chris Dalton
https://chromiumcodereview.appspot.com/452823002/diff/20001/src/gpu/GrPathRendering.h File src/gpu/GrPathRendering.h (right): https://chromiumcodereview.appspot.com/452823002/diff/20001/src/gpu/GrPathRendering.h#newcode26 src/gpu/GrPathRendering.h:26: * It is expected that there is 1:1 relation ...
6 years, 4 months ago (2014-08-14 16:22:10 UTC) #5
Kimmo Kinnunen
Thanks! How about now? https://chromiumcodereview.appspot.com/452823002/diff/20001/src/gpu/GrPathRendering.h File src/gpu/GrPathRendering.h (right): https://chromiumcodereview.appspot.com/452823002/diff/20001/src/gpu/GrPathRendering.h#newcode26 src/gpu/GrPathRendering.h:26: * It is expected that ...
6 years, 4 months ago (2014-08-15 06:19:26 UTC) #6
Chris Dalton
lgtm
6 years, 4 months ago (2014-08-15 18:04:27 UTC) #7
bsalomon
lgtm w/ a minor ask: Can you change the name of onResetContext to resetContext()? We ...
6 years, 4 months ago (2014-08-19 21:29:54 UTC) #8
Kimmo Kinnunen
On 2014/08/19 21:29:54, bsalomon wrote: > lgtm w/ a minor ask: Can you change the ...
6 years, 4 months ago (2014-08-20 08:28:14 UTC) #9
Kimmo Kinnunen
The CQ bit was checked by kkinnunen@nvidia.com
6 years, 4 months ago (2014-08-20 08:28:38 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/kkinnunen@nvidia.com/452823002/80001
6 years, 4 months ago (2014-08-20 08:29:30 UTC) #11
commit-bot: I haz the power
6 years, 4 months ago (2014-08-20 08:36:32 UTC) #12
Message was sent while issue was closed. 
Committed patchset #5 (80001) as ccdaa0422501e5cbcba53d6bd19f2736f1beaef3

Powered by Google App Engine
This is Rietveld 408576698