gpu: Add TransformFeedback & Uniform Buffer related consts

gpu: Add TransformFeedback & Uniform Buffer related consts

Added max_transform_feedback_separate_attribs, max_uniform_buffer_bindings,
and uniform_buffer_offset_alignment to Capabilities.
These were introducted in GLES 3.0.

BUG=429053
TEST=gpu_unittests

Committed: https://crrev.com/8f143294c4f57bb04e170283a917a577fadb40c8
Cr-Commit-Position: refs/heads/master@{#314725}

[hj.r.chung, 2015-01-29 12:56:10]

heejin.r.chung@samsung.com changed reviewers:
+ piman@chromium.org, vmiura@chromium.org

[hj.r.chung, 2015-02-02 06:09:52]

ptal!

[hj.r.chung, 2015-02-02 06:09:55]

ptal!

[piman, 2015-02-04 00:43:19]

TBH, I'm not sure this is really worth it.
The current bug means that after an invalid client operation, we may not vend
some ids in GenBuffers etc. which is essentially harmless.
However to solve it we're adding some amount of overhead / duplicated checking

https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/client/gl...
File gpu/command_buffer/client/gles2_implementation.cc (left):

https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/client/gl...
gpu/command_buffer/client/gles2_implementation.cc:2822: // used. even though
it's marked it as used here.
note: this is still true. For example, we don't know if GL_TEXTURE_EXTERNAL_OES
is supported or not (since it's an extension).

https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/common/ca...
File gpu/command_buffer/common/capabilities.h (right):

https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/common/ca...
gpu/command_buffer/common/capabilities.h:80: int
uniform_buffer_offset_alignment;
These need to be added to gpu/ipc/gpu_command_buffer_traits_multi.h

[hj.r.chung, 2015-02-04 01:21:26]

On 2015/02/04 00:43:19, piman (Very slow to review) wrote:
> TBH, I'm not sure this is really worth it.
> The current bug means that after an invalid client operation, we may not vend
> some ids in GenBuffers etc. which is essentially harmless.
> However to solve it we're adding some amount of overhead / duplicated checking

Yeah, I agree that the current bug is essentially harmless.
I was thinking there could be a trade-off between doing some checking in client
side vs.
sending an erroneous command to the server. But I guess the possibility of doing
a 
glBind call with the wrong parameters won't be that high :)

So, should I leave the capabilities & getHelpers part (with the fix you
mentioned) changes,
or do you think that part is also unnecessary?


> 
>
https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/client/gl...
> File gpu/command_buffer/client/gles2_implementation.cc (left):
> 
>
https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/client/gl...
> gpu/command_buffer/client/gles2_implementation.cc:2822: // used. even though
> it's marked it as used here.
> note: this is still true. For example, we don't know if
GL_TEXTURE_EXTERNAL_OES
> is supported or not (since it's an extension).
> 
>
https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/common/ca...
> File gpu/command_buffer/common/capabilities.h (right):
> 
>
https://codereview.chromium.org/885013002/diff/1/gpu/command_buffer/common/ca...
> gpu/command_buffer/common/capabilities.h:80: int
> uniform_buffer_offset_alignment;
> These need to be added to gpu/ipc/gpu_command_buffer_traits_multi.h

[piman, 2015-02-04 01:29:43]

On Tue, Feb 3, 2015 at 5:21 PM, <heejin.r.chung@samsung.com> wrote:

> On 2015/02/04 00:43:19, piman (Very slow to review) wrote:
>
>> TBH, I'm not sure this is really worth it.
>> The current bug means that after an invalid client operation, we may not
>> vend
>> some ids in GenBuffers etc. which is essentially harmless.
>> However to solve it we're adding some amount of overhead / duplicated
>> checking
>>
>
> Yeah, I agree that the current bug is essentially harmless.
> I was thinking there could be a trade-off between doing some checking in
> client
> side vs.
> sending an erroneous command to the server. But I guess the possibility of
> doing
> a
> glBind call with the wrong parameters won't be that high :)
>
> So, should I leave the capabilities & getHelpers part (with the fix you
> mentioned) changes,
> or do you think that part is also unnecessary?


Those are useful. I'm ok if you repurpose this CL, maybe just change the
description/summary?

Antoine


>
>
>
>
>
> https://codereview.chromium.org/885013002/diff/1/gpu/
> command_buffer/client/gles2_implementation.cc
>
>> File gpu/command_buffer/client/gles2_implementation.cc (left):
>>
>
>
> https://codereview.chromium.org/885013002/diff/1/gpu/
> command_buffer/client/gles2_implementation.cc#oldcode2822
>
>> gpu/command_buffer/client/gles2_implementation.cc:2822: // used. even
>> though
>> it's marked it as used here.
>> note: this is still true. For example, we don't know if
>>
> GL_TEXTURE_EXTERNAL_OES
>
>> is supported or not (since it's an extension).
>>
>
>
> https://codereview.chromium.org/885013002/diff/1/gpu/
> command_buffer/common/capabilities.h
>
>> File gpu/command_buffer/common/capabilities.h (right):
>>
>
>
> https://codereview.chromium.org/885013002/diff/1/gpu/
> command_buffer/common/capabilities.h#newcode80
>
>> gpu/command_buffer/common/capabilities.h:80: int
>> uniform_buffer_offset_alignment;
>> These need to be added to gpu/ipc/gpu_command_buffer_traits_multi.h
>>
>
>
>
> https://codereview.chromium.org/885013002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[hj.r.chung, 2015-02-04 02:50:50]

On 2015/02/04 01:29:43, piman (Very slow to review) wrote:
> On Tue, Feb 3, 2015 at 5:21 PM, <mailto:heejin.r.chung@samsung.com> wrote:
> 
> > On 2015/02/04 00:43:19, piman (Very slow to review) wrote:
> >
> >> TBH, I'm not sure this is really worth it.
> >> The current bug means that after an invalid client operation, we may not
> >> vend
> >> some ids in GenBuffers etc. which is essentially harmless.
> >> However to solve it we're adding some amount of overhead / duplicated
> >> checking
> >>
> >
> > Yeah, I agree that the current bug is essentially harmless.
> > I was thinking there could be a trade-off between doing some checking in
> > client
> > side vs.
> > sending an erroneous command to the server. But I guess the possibility of
> > doing
> > a
> > glBind call with the wrong parameters won't be that high :)
> >
> > So, should I leave the capabilities & getHelpers part (with the fix you
> > mentioned) changes,
> > or do you think that part is also unnecessary?
> 
> 
> Those are useful. I'm ok if you repurpose this CL, maybe just change the
> description/summary?
> 
> Antoine
> 
> 
> >
> >
> >
> >
> >
> > https://codereview.chromium.org/885013002/diff/1/gpu/
> > command_buffer/client/gles2_implementation.cc
> >
> >> File gpu/command_buffer/client/gles2_implementation.cc (left):
> >>
> >
> >
> > https://codereview.chromium.org/885013002/diff/1/gpu/
> > command_buffer/client/gles2_implementation.cc#oldcode2822
> >
> >> gpu/command_buffer/client/gles2_implementation.cc:2822: // used. even
> >> though
> >> it's marked it as used here.
> >> note: this is still true. For example, we don't know if
> >>
> > GL_TEXTURE_EXTERNAL_OES
> >
> >> is supported or not (since it's an extension).
> >>
> >
> >
> > https://codereview.chromium.org/885013002/diff/1/gpu/
> > command_buffer/common/capabilities.h
> >
> >> File gpu/command_buffer/common/capabilities.h (right):
> >>
> >
> >
> > https://codereview.chromium.org/885013002/diff/1/gpu/
> > command_buffer/common/capabilities.h#newcode80
> >
> >> gpu/command_buffer/common/capabilities.h:80: int
> >> uniform_buffer_offset_alignment;
> >> These need to be added to gpu/ipc/gpu_command_buffer_traits_multi.h
> >>
> >
> >
> >
> > https://codereview.chromium.org/885013002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

I left the early returns in switch(){default: } cases 
and updated the comment about the bug just in case there was someone like me :)
PTAL!

[piman, 2015-02-05 00:29:26]

Actually, I don't think we should early-return.
The switch statements only care about the state that we want to shadow
client-side, but there may be extensions (e.g. 3d textures) that we don't
shadow, but we may still want to allow (if es3 is enabled, or extension is
present).
So I think it's safer not to add the early returns.

[hj.r.chung, 2015-02-05 00:36:55]

On 2015/02/05 00:29:26, piman (Very slow to review) wrote:
> Actually, I don't think we should early-return.
> The switch statements only care about the state that we want to shadow
> client-side, but there may be extensions (e.g. 3d textures) that we don't
> shadow, but we may still want to allow (if es3 is enabled, or extension is
> present).
> So I think it's safer not to add the early returns.

thanks for the input! I removed the early returns

[piman, 2015-02-05 00:38:31]

lgtm

[hj.r.chung, 2015-02-05 00:45:30]

The CQ bit was checked by heejin.r.chung@samsung.com

[commit-bot: I haz the power, 2015-02-05 00:47:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/885013002/40001

[commit-bot: I haz the power, 2015-02-05 02:44:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-05 02:45:34]

Patchset 3 (id:??) landed as
https://crrev.com/8f143294c4f57bb04e170283a917a577fadb40c8
Cr-Commit-Position: refs/heads/master@{#314725}