Add support for GL_NV_framebuffer_multisample_coverage

[bsalomon, 2012-04-11 14:21:52]

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (left):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGpuGL.cpp#oldcod...
src/gpu/gl/GrGpuGL.cpp:1131: if (samples > 1) {
Using 1 rather than 0 was a bug.

[robertphillips, 2012-04-11 14:46:47]

LGTM w/ minor comments

[robertphillips, 2012-04-11 14:47:03]

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp#newco...
src/gpu/gl/GrGLCaps.cpp:218: int max = (fMSAACoverageModes.end() -
1)->fCoverageSampleCnt;
I'm surprised we don't have front & back members in SkTDArray.h

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h#newcode65
src/gpu/gl/GrGLCaps.h:65: enum CoverageAAType {
Do we want an Sk or Gr prefix here?

[bsalomon, 2012-04-11 15:01:18]

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp#newco...
src/gpu/gl/GrGLCaps.cpp:218: int max = (fMSAACoverageModes.end() -
1)->fCoverageSampleCnt;
On 2012/04/11 14:47:03, robertphillips wrote:
> I'm surprised we don't have front & back members in SkTDArray.h

There is a top() that gets count-1  but this isn't really being used as a stack
so it seemed misleading to call it.

In general we need to tackle merging SkTDArray and SkTArray (the latter has
front() and back() but not begin(), top(), or end() <sigh>).

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h#newcode65
src/gpu/gl/GrGLCaps.h:65: enum CoverageAAType {
On 2012/04/11 14:47:03, robertphillips wrote:
> Do we want an Sk or Gr prefix here?

For nested names we don't require it (and I prefer dropping it in that case).

[TomH, 2012-04-11 15:39:59]

LGTM. The coalescence of the several RenderbufferStorageMultisample calls is
appreciated.

No mac-specific NativeInterface support?

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp#newco...
src/gpu/gl/GrGLCaps.cpp:165: #include "SkTSearch.h"
Is there a reason not to have this at the top of the file?

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h#newcode36
src/gpu/gl/GrGLCaps.h:36: // Color samples are the subset of coverage samples
that have data:
This comment isn't quite clear.

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h#newcode65
src/gpu/gl/GrGLCaps.h:65: enum CoverageAAType {
SGTM

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h#newcode139
src/gpu/gl/GrGLCaps.h:139: * Reports the type of coverage sample AA support.
Tautological comment?

[bsalomon, 2012-04-11 18:16:59]

The mac doesn't support this extension (even on 10.7 with the 3.2 core profile).

Closed with r3651

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.cpp#newco...
src/gpu/gl/GrGLCaps.cpp:165: #include "SkTSearch.h"
On 2012/04/11 15:39:59, TomH wrote:
> Is there a reason not to have this at the top of the file?

I suppose not... I'll move it up before check in.

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h#newcode36
src/gpu/gl/GrGLCaps.h:36: // Color samples are the subset of coverage samples
that have data:
On 2012/04/11 15:39:59, TomH wrote:
> This comment isn't quite clear.
(Tom and I discussed a rewrite in person).

http://codereview.appspot.com/6005043/diff/3011/src/gpu/gl/GrGLCaps.h#newcode139
src/gpu/gl/GrGLCaps.h:139: * Reports the type of coverage sample AA support.
On 2012/04/11 15:39:59, TomH wrote:
> Tautological comment?
yes