Add a bunch of ES3 enums to the validator.

Add a bunch of ES3 enums to the validator.

BUG=429053
TEST=gpu_unittests
R=piman@chromium.org

Committed: https://crrev.com/bb77c86888ebea8e65aa5026486ed8f1d4cfd700
Cr-Commit-Position: refs/heads/master@{#329765}

[Zhenyao Mo, 2015-05-13 22:24:46]

piman: PTAL
others: FYI (feel free to review)

[piman, 2015-05-13 22:35:30]

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
File gpu/command_buffer/build_gles2_cmd_buffer.py (right):

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1114: 'GL_BUFFER_ACCESS_FLAGS',
note: because of the logic in GLES2DecoderImpl::HandleMapBufferRange, we have to
intercept the Get of this one to return the original value (which we may need to
track).

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1427: 'GL_UNPACK_SKIP_IMAGES',
most of these have an effect on the client only if a buffer isn't bound, so we
probably need to intercept them on the client.

[bajones, 2015-05-13 22:38:39]

bajones@chromium.org changed reviewers:
+ bajones@chromium.org

[bajones, 2015-05-13 22:38:40]

A couple of comments

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
File gpu/command_buffer/build_gles2_cmd_buffer.py (right):

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1179: #
'GL_PROGRAM_BINARY_RETRIEVABLE_HINT',
Probably don't want to leave this as a comment. If it's valid in ES3 (not
WebGL2) why not add it? If we're not supporting it, maybe add it explicitly to
the invalid list?

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1419: 'GL_PACK_IMAGE_HEIGHT',
As per conversation with Ken, this isn't in the ES3 spec any more.

[Zhenyao Mo, 2015-05-13 23:02:53]

revised. please take another look.

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
File gpu/command_buffer/build_gles2_cmd_buffer.py (right):

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1114: 'GL_BUFFER_ACCESS_FLAGS',
On 2015/05/13 22:35:29, piman (Very slow to review) wrote:
> note: because of the logic in GLES2DecoderImpl::HandleMapBufferRange, we have
to
> intercept the Get of this one to return the original value (which we may need
to
> track).

Acknowledged.

Will follow up on this: crbug.com/487834

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1179: #
'GL_PROGRAM_BINARY_RETRIEVABLE_HINT',
On 2015/05/13 22:38:40, bajones wrote:
> Probably don't want to leave this as a comment. If it's valid in ES3 (not
> WebGL2) why not add it? If we're not supporting it, maybe add it explicitly to
> the invalid list?

Done.

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1419: 'GL_PACK_IMAGE_HEIGHT',
On 2015/05/13 22:38:40, bajones wrote:
> As per conversation with Ken, this isn't in the ES3 spec any more.

Done.

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1427: 'GL_UNPACK_SKIP_IMAGES',
On 2015/05/13 22:35:29, piman (Very slow to review) wrote:
> most of these have an effect on the client only if a buffer isn't bound, so we
> probably need to intercept them on the client.

Right, we need to intercept all of them on the client side.

Created crbug.com/487841 to track this.

[piman, 2015-05-13 23:11:26]

lgtm

[Zhenyao Mo, 2015-05-13 23:39:21]

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
File gpu/command_buffer/build_gles2_cmd_buffer.py (right):

https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
gpu/command_buffer/build_gles2_cmd_buffer.py:1427: 'GL_UNPACK_SKIP_IMAGES',
Per offline discussion, these are still needed on the service side if service
side data are in use.

[Zhenyao Mo, 2015-05-13 23:39:28]

The CQ bit was checked by zmo@chromium.org

[Zhenyao Mo, 2015-05-13 23:39:28]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1134373002/#ps40001
(title: "final")

[commit-bot: I haz the power, 2015-05-13 23:41:25]

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

[piman, 2015-05-14 00:01:30]

On 2015/05/13 23:39:21, Zhenyao Mo wrote:
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> File gpu/command_buffer/build_gles2_cmd_buffer.py (right):
> 
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> gpu/command_buffer/build_gles2_cmd_buffer.py:1427: 'GL_UNPACK_SKIP_IMAGES',
> Per offline discussion, these are still needed on the service side if service
> side data are in use.

Well, if we cache them on the client side (even for the bound buffer case), we
can use that without round-tripping to the service.

[Zhenyao Mo, 2015-05-14 00:21:39]

On 2015/05/14 00:01:30, piman (Very slow to review) wrote:
> On 2015/05/13 23:39:21, Zhenyao Mo wrote:
> >
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> > File gpu/command_buffer/build_gles2_cmd_buffer.py (right):
> > 
> >
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> > gpu/command_buffer/build_gles2_cmd_buffer.py:1427: 'GL_UNPACK_SKIP_IMAGES',
> > Per offline discussion, these are still needed on the service side if
service
> > side data are in use.
> 
> Well, if we cache them on the client side (even for the bound buffer case), we
> can use that without round-tripping to the service.

But we still need to call glPixelStorei with them on the service side for the
bound buffer case, so they still need to be added to the validator.

[Zhenyao Mo, 2015-05-14 00:26:57]

On 2015/05/14 00:21:39, Zhenyao Mo wrote:
> On 2015/05/14 00:01:30, piman (Very slow to review) wrote:
> > On 2015/05/13 23:39:21, Zhenyao Mo wrote:
> > >
> >
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> > > File gpu/command_buffer/build_gles2_cmd_buffer.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> > > gpu/command_buffer/build_gles2_cmd_buffer.py:1427:
'GL_UNPACK_SKIP_IMAGES',
> > > Per offline discussion, these are still needed on the service side if
> service
> > > side data are in use.
> > 
> > Well, if we cache them on the client side (even for the bound buffer case),
we
> > can use that without round-tripping to the service.
> 
> But we still need to call glPixelStorei with them on the service side for the
> bound buffer case, so they still need to be added to the validator.

OK, I have to leave for SF for a 3D web graphics event.  If you think I am
wrong, please uncheck the commit button for me. Thank you.

[commit-bot: I haz the power, 2015-05-14 00:47:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-14 00:48:26]

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

[piman, 2015-05-14 00:55:27]

On Wed, May 13, 2015 at 5:21 PM, <zmo@chromium.org> wrote:

> On 2015/05/14 00:01:30, piman (Very slow to review) wrote:
>
>> On 2015/05/13 23:39:21, Zhenyao Mo wrote:
>> >
>>
>
>
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
>
>> > File gpu/command_buffer/build_gles2_cmd_buffer.py (right):
>> >
>> >
>>
>
>
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
>
>> > gpu/command_buffer/build_gles2_cmd_buffer.py:1427:
>> 'GL_UNPACK_SKIP_IMAGES',
>> > Per offline discussion, these are still needed on the service side if
>>
> service
>
>> > side data are in use.
>>
>
>  Well, if we cache them on the client side (even for the bound buffer
>> case), we
>> can use that without round-tripping to the service.
>>
>
> But we still need to call glPixelStorei with them on the service side for
> the
> bound buffer case, so they still need to be added to the validator.
>

Sure, but GL_UNPACK_SKIP_IMAGES etc. is only used for the Get (which
doesn't need a service-side component if we cache on the client).
We do need the PixelStorei command, but with careful management (so that a
client doesn't change the gl driver as it reads from shm buffers for the
no-buffer-bound case).


> https://codereview.chromium.org/1134373002/
>

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

[Zhenyao Mo, 2015-05-14 03:51:20]

On 2015/05/14 00:55:27, piman (Very slow to review) wrote:
> On Wed, May 13, 2015 at 5:21 PM, <mailto:zmo@chromium.org> wrote:
> 
> > On 2015/05/14 00:01:30, piman (Very slow to review) wrote:
> >
> >> On 2015/05/13 23:39:21, Zhenyao Mo wrote:
> >> >
> >>
> >
> >
> >
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> >
> >> > File gpu/command_buffer/build_gles2_cmd_buffer.py (right):
> >> >
> >> >
> >>
> >
> >
> >
>
https://codereview.chromium.org/1134373002/diff/1/gpu/command_buffer/build_gl...
> >
> >> > gpu/command_buffer/build_gles2_cmd_buffer.py:1427:
> >> 'GL_UNPACK_SKIP_IMAGES',
> >> > Per offline discussion, these are still needed on the service side if
> >>
> > service
> >
> >> > side data are in use.
> >>
> >
> >  Well, if we cache them on the client side (even for the bound buffer
> >> case), we
> >> can use that without round-tripping to the service.
> >>
> >
> > But we still need to call glPixelStorei with them on the service side for
> > the
> > bound buffer case, so they still need to be added to the validator.
> >
> 
> Sure, but GL_UNPACK_SKIP_IMAGES etc. is only used for the Get (which
> doesn't need a service-side component if we cache on the client).
> We do need the PixelStorei command, but with careful management (so that a
> client doesn't change the gl driver as it reads from shm buffers for the
> no-buffer-bound case).

Got it.  I'll revisit this carefully when I work around to fine tune the
PixelStorei command.

> 
> 
> > https://codereview.chromium.org/1134373002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.