Issue 441623005: Remove unused matrix param from GrContext/GrDrawTarget rect drawing functions.

Issue 441623005: Remove unused matrix param from GrContext/GrDrawTarget rect drawing functions. (Closed)

Created:
6 years, 4 months ago by bsalomon

Modified:
6 years, 4 months ago

Reviewers:
robertphillips

CC:
reviews_skia.org

Base URL:
https://skia.googlesource.com/skia.git@master

Visibility:
Public.

More Reviews

Description

Remove unused matrix param from GrContext/GrDrawTarget rect drawing functions. Committed: https://skia.googlesource.com/skia/+/01c8da1eef36570374f7e8764a38b25bf16ab7a6

Patch Set 2 : comment change #

Total comments: 8

Patch Set 3 : Address comment #

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+37 lines, -71 lines)			Patch
M	include/gpu/GrContext.h	View	1	1 chunk	+4 lines, -11 lines	0 comments	Download
M	src/gpu/GrClipMaskManager.cpp	View		5 chunks	+5 lines, -5 lines	0 comments	Download
M	src/gpu/GrContext.cpp	View		10 chunks	+11 lines, -22 lines	0 comments	Download
M	src/gpu/GrDefaultPathRenderer.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/gpu/GrDrawTarget.h	View	1 2	3 chunks	+5 lines, -8 lines	0 comments	Download
M	src/gpu/GrDrawTarget.cpp	View		1 chunk	+0 lines, -6 lines	0 comments	Download
M	src/gpu/GrInOrderDrawBuffer.h	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	src/gpu/GrInOrderDrawBuffer.cpp	View		3 chunks	+3 lines, -9 lines	0 comments	Download
M	src/gpu/GrOvalRenderer.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/gpu/GrSoftwarePathRenderer.cpp	View		1 chunk	+4 lines, -4 lines	0 comments	Download
M	src/gpu/SkGpuDevice.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/gpu/gl/GrGpuGL.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

bsalomon

This feature of GrContext/GrDrawTarget::drawRect(ToRect) isn't being used and it probably shouldn't be. When there are ...

6 years, 4 months ago (2014-08-04 14:51:46 UTC) #1

robertphillips

lgtm + questions & suggestions https://codereview.chromium.org/441623005/diff/20001/include/gpu/GrContext.h File include/gpu/GrContext.h (right): https://codereview.chromium.org/441623005/diff/20001/include/gpu/GrContext.h#newcode485 include/gpu/GrContext.h:485: * @param localRect rect ...

6 years, 4 months ago (2014-08-04 15:29:16 UTC) #2

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/bsalomon@google.com/441623005/40001

6 years, 4 months ago (2014-08-04 16:11:18 UTC) #4

commit-bot: I haz the power

Change committed as 01c8da1eef36570374f7e8764a38b25bf16ab7a6

6 years, 4 months ago (2014-08-04 16:21:41 UTC) #5

bsalomon

6 years, 4 months ago (2014-08-05 14:04:56 UTC) #6

Message was sent while issue was closed.

https://codereview.chromium.org/441623005/diff/20001/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/441623005/diff/20001/include/gpu/GrContext.h#...
include/gpu/GrContext.h:485: * @param localRect     rect of local coordinates to
be mapped onto dstRect
On 2014/08/04 15:29:15, robertphillips wrote:
> Do we get any mileage out of 'localMatrix'? Can we remove it too?

It is kind of useful... and it doesn't mess with batching.

https://codereview.chromium.org/441623005/diff/20001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/441623005/diff/20001/src/gpu/GrContext.cpp#ne...
src/gpu/GrContext.cpp:798: SkScalar width = NULL == strokeInfo ? -1 :
strokeInfo->getStrokeRec().getWidth();
On 2014/08/04 15:29:15, robertphillips wrote:
> Can this be "const SkMatrix& matrix = ..." now ?

We do an AutoViewMatrixRestore in some cases below, which will modify the draw
state's matrix.

https://codereview.chromium.org/441623005/diff/20001/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (right):

https://codereview.chromium.org/441623005/diff/20001/src/gpu/GrDrawTarget.h#n...
src/gpu/GrDrawTarget.h:391: * @param rect        the rect to draw
On 2014/08/04 15:29:16, robertphillips wrote:
> Remove this @param ?

Done.

https://codereview.chromium.org/441623005/diff/20001/src/gpu/GrInOrderDrawBuf...
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.chromium.org/441623005/diff/20001/src/gpu/GrInOrderDrawBuf...
src/gpu/GrInOrderDrawBuffer.cpp:168: // Go to device coords to allow batching
across matrix changes
On 2014/08/04 15:29:16, robertphillips wrote:
> const SkMatrix& ?

The avmr a few lines down will modify it.

Expand Messages | Collapse Messages