Make drawIndexedInstances non-virtual and rewrite GrIODB's drawRect on top of drawIndexedInstances.

Make drawIndexedInstances non-virtual and rewrite GrIODB's drawRect on top of drawIndexedInstances.

R=robertphillips@google.com
Committed: https://code.google.com/p/skia/source/detail?r=7508

[robertphillips, 2013-01-31 18:48:57]

LGTM + mainly comment suggestions

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.cpp
File samplecode/SampleManyRects.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.c...
samplecode/SampleManyRects.cpp:15: 
overall comment on what this sample is intended to do?

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.c...
samplecode/SampleManyRects.cpp:39: SkRect clipRect = canvasRect;
comment on why doing the inset? Do we even use this value any more?

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.c...
samplecode/SampleManyRects.cpp:51: // that the rect is inside the clip so the
clip changes interrupt batching.
It seems like we should be able to determine the rect is still in the clip for
the majority of cases? Don't know if it would pay off time-wise though?

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrContext.cpp#newcod...
src/gpu/GrContext.cpp:103: delete fFontCache;
Why do we care about this?

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrDrawState.cpp#newc...
src/gpu/GrDrawState.cpp:548: 
is will -> will

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer.cpp
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer....
src/gpu/GrInOrderDrawBuffer.cpp:99: if (NULL != srcRects) {
I've seen this pattern a bunch - is it time for a inlineable helper method?

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer....
src/gpu/GrInOrderDrawBuffer.cpp:123:
combinedMatrix.postConcat(this->drawState()->getViewMatrix());
want -> want to

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer....
src/gpu/GrInOrderDrawBuffer.cpp:183: // Take into account that the viewport
edges may handle some clipping for us.
I think this needs to better explain why we are blasting out when the clip
matches the rendertarget size

[bsalomon, 2013-02-01 14:18:35]

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.cpp
File samplecode/SampleManyRects.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.c...
samplecode/SampleManyRects.cpp:15: 
On 2013/01/31 18:48:57, robertphillips wrote:
> overall comment on what this sample is intended to do?

Done.

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.c...
samplecode/SampleManyRects.cpp:39: SkRect clipRect = canvasRect;
On 2013/01/31 18:48:57, robertphillips wrote:
> comment on why doing the inset? Do we even use this value any more?

Removed

https://codereview.appspot.com/7221078/diff/2001/samplecode/SampleManyRects.c...
samplecode/SampleManyRects.cpp:51: // that the rect is inside the clip so the
clip changes interrupt batching.
On 2013/01/31 18:48:57, robertphillips wrote:
> It seems like we should be able to determine the rect is still in the clip for
> the majority of cases? Don't know if it would pay off time-wise though?

I think it would... This gets 12 to 16 times slower when the clip is rotated.

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrContext.cpp#newcod...
src/gpu/GrContext.cpp:103: delete fFontCache;
On 2013/01/31 18:48:57, robertphillips wrote:
> Why do we care about this?

Removed. I can't remember why I typed this since deleting null is fine.

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrDrawState.cpp#newc...
src/gpu/GrDrawState.cpp:548: 
On 2013/01/31 18:48:57, robertphillips wrote:
> is will -> will

Done.

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer.cpp
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer....
src/gpu/GrInOrderDrawBuffer.cpp:99: if (NULL != srcRects) {
On 2013/01/31 18:48:57, robertphillips wrote:
> I've seen this pattern a bunch - is it time for a inlineable helper method?

I think it appears two times, here and the base classes version of this
function.

It be consolidated but it'd be obsoleted by the stuff Jim is working on pretty
quickly. I don't know that we actually need an array of srcRects at all.

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer....
src/gpu/GrInOrderDrawBuffer.cpp:123:
combinedMatrix.postConcat(this->drawState()->getViewMatrix());
On 2013/01/31 18:48:57, robertphillips wrote:
> want -> want to

Done.

https://codereview.appspot.com/7221078/diff/2001/src/gpu/GrInOrderDrawBuffer....
src/gpu/GrInOrderDrawBuffer.cpp:183: // Take into account that the viewport
edges may handle some clipping for us.
On 2013/01/31 18:48:57, robertphillips wrote:
> I think this needs to better explain why we are blasting out when the clip
> matches the rendertarget size

Done.