Move the GrGLGpu config stencil format index into ConfigTable

Move the GrGLGpu config stencil format index into ConfigTable
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1526253007

Committed: https://skia.googlesource.com/skia/+/62a627be6a7e3fdb4fe1c67de0c371a276c761f6

[bsalomon, 2015-12-16 21:39:25]

Description was changed from

==========
Move the GrGLGpu config stencil format index into ConfigTable
==========

to

==========
Move the GrGLGpu config stencil format index into ConfigTable
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2015-12-16 21:40:57]

bsalomon@google.com changed reviewers:
+ egdaniel@google.com

[egdaniel, 2015-12-16 21:52:11]

lgtm with the one question

https://codereview.chromium.org/1526253007/diff/20001/src/gpu/gl/GrGLGpu.h
File src/gpu/gl/GrGLGpu.h (right):

https://codereview.chromium.org/1526253007/diff/20001/src/gpu/gl/GrGLGpu.h#ne...
src/gpu/gl/GrGLGpu.h:156: // compatible stencil format, or negative if there is
no compatible stencil format.
Not a big issue, I can go either way, but would it make more sense to say:

"Given a GrPixelConfig return the index into the stencil format array on
GrGLCaps to a compatible stencil format, or
ConfigEntry::kUnsupported_StencilFormatIndex if there is no compatible stencil
format." We should never return kUnknown and this makes it very clear what
values can be returned from this function. If we go this route I would change
the if statement in createStencil back to what it was before.

[bsalomon, 2015-12-17 16:56:45]

https://codereview.chromium.org/1526253007/diff/20001/src/gpu/gl/GrGLGpu.h
File src/gpu/gl/GrGLGpu.h (right):

https://codereview.chromium.org/1526253007/diff/20001/src/gpu/gl/GrGLGpu.h#ne...
src/gpu/gl/GrGLGpu.h:156: // compatible stencil format, or negative if there is
no compatible stencil format.
On 2015/12/16 21:52:11, egdaniel wrote:
> Not a big issue, I can go either way, but would it make more sense to say:
> 
> "Given a GrPixelConfig return the index into the stencil format array on
> GrGLCaps to a compatible stencil format, or
> ConfigEntry::kUnsupported_StencilFormatIndex if there is no compatible stencil
> format." We should never return kUnknown and this makes it very clear what
> values can be returned from this function. If we go this route I would change
> the if statement in createStencil back to what it was before.


My feeling is that kUnsupported and kUnimplemented are implementation details
and that if this code was in another class we might not expose kUnsupported or
kUnimplemented to the caller. I suppose we could expose kUnsupported only but
since the return value is an index it seems straightforward to me to say
anything negative means we failed to find a compatible format.

[bsalomon, 2015-12-17 17:32:57]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2015-12-17 17:33:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1526253007/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1526253007/20001

[egdaniel, 2015-12-17 17:47:50]

On 2015/12/17 16:56:45, bsalomon wrote:
> https://codereview.chromium.org/1526253007/diff/20001/src/gpu/gl/GrGLGpu.h
> File src/gpu/gl/GrGLGpu.h (right):
> 
>
https://codereview.chromium.org/1526253007/diff/20001/src/gpu/gl/GrGLGpu.h#ne...
> src/gpu/gl/GrGLGpu.h:156: // compatible stencil format, or negative if there
is
> no compatible stencil format.
> On 2015/12/16 21:52:11, egdaniel wrote:
> > Not a big issue, I can go either way, but would it make more sense to say:
> > 
> > "Given a GrPixelConfig return the index into the stencil format array on
> > GrGLCaps to a compatible stencil format, or
> > ConfigEntry::kUnsupported_StencilFormatIndex if there is no compatible
stencil
> > format." We should never return kUnknown and this makes it very clear what
> > values can be returned from this function. If we go this route I would
change
> > the if statement in createStencil back to what it was before.
> 
> 
> My feeling is that kUnsupported and kUnimplemented are implementation details
> and that if this code was in another class we might not expose kUnsupported or
> kUnimplemented to the caller. I suppose we could expose kUnsupported only but
> since the return value is an index it seems straightforward to me to say
> anything negative means we failed to find a compatible format.

sounds good

[commit-bot: I haz the power, 2015-12-17 17:50:49]

Description was changed from

==========
Move the GrGLGpu config stencil format index into ConfigTable
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Move the GrGLGpu config stencil format index into ConfigTable
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/62a627be6a7e3fdb4fe1c67de0c371a276c761f6
==========

[commit-bot: I haz the power, 2015-12-17 17:50:50]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/62a627be6a7e3fdb4fe1c67de0c371a276c761f6