Fix GetFramebufferAttachmentParameteriv command handling.

Fix GetFramebufferAttachmentParameteriv command handling.

Blink side CL: https://codereview.chromium.org/1303343009/

BUG=527249
TEST=conformance2/state/gl-object-get-calls.html related parts pass
R=piman@chromium.org

Committed: https://crrev.com/67265f5b35173165472621909462d3cac4b500bd
Cr-Commit-Position: refs/heads/master@{#347844}

[Zhenyao Mo, 2015-09-04 00:23:44]

Patchset #1 (id:1) has been deleted

[Zhenyao Mo, 2015-09-04 00:24:15]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-04 00:24:40]

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

[Zhenyao Mo, 2015-09-04 00:24:50]

piman: PTAL
Others: FYI

[commit-bot: I haz the power, 2015-09-04 00:47:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 00:47:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[piman, 2015-09-04 01:18:03]

lgtm

[Zhenyao Mo, 2015-09-04 23:08:57]

Patchset #2 (id:40001) has been deleted

[Zhenyao Mo, 2015-09-04 23:10:39]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-04 23:11:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319453010/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319453010/60001

[Zhenyao Mo, 2015-09-04 23:38:11]

piman: the gl_tests caught a bug patch set 1 introduced.  Could you take another
look?

https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:6222: if (error == GL_NO_ERROR
&& pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) {
This is where the change is.

[piman, 2015-09-05 00:12:03]

https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:6222: if (error == GL_NO_ERROR
&& pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) {
For this pname, should we just always return the tracked attachment without
going to GL?

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

On 2015/09/05 00:12:03, piman (slow to review) wrote:
>
https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> 
>
https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:6222: if (error == GL_NO_ERROR
> && pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) {
> For this pname, should we just always return the tracked attachment without
> going to GL?

The thing is, in order to do that, I have to add the full validation for ES3,
which is moderately more complicated than ES2.  I am OK to do that if you think
that's a better path.

[commit-bot: I haz the power, 2015-09-05 00:32:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-05 00:32:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[piman, 2015-09-05 00:57:27]

On Fri, Sep 4, 2015 at 5:14 PM, <zmo@chromium.org> wrote:

> On 2015/09/05 00:12:03, piman (slow to review) wrote:
>
>
>
https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
>
>> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
>>
>
>
>
>
https://codereview.chromium.org/1319453010/diff/60001/gpu/command_buffer/serv...
>
>> gpu/command_buffer/service/gles2_cmd_decoder.cc:6222: if (error ==
>> GL_NO_ERROR
>> && pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) {
>> For this pname, should we just always return the tracked attachment
>> without
>> going to GL?
>>
>
> The thing is, in order to do that, I have to add the full validation for
> ES3,
> which is moderately more complicated than ES2.  I am OK to do that if you
> think
> that's a better path.
>

I'm not sure I follow. The no framebuffer case is handled above. What else
is there to do for that pname? Just check that
!backbuffer_attachment.IsValid(attachment)? That seems straightforward
enough...


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

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

[Zhenyao Mo, 2015-09-08 22:49:09]

Patchset #3 (id:80001) has been deleted

[Zhenyao Mo, 2015-09-08 22:50:29]

Revised, please take another look.

Note you only need to look at difference between patch set 3 and 4, as patch set
3 is pure rebasing.

[Zhenyao Mo, 2015-09-08 22:50:34]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-08 22:50:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319453010/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319453010/120001

[piman, 2015-09-08 23:03:25]

lgtm

[commit-bot: I haz the power, 2015-09-09 00:07:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-09 00:07:54]

Dry run: This issue passed the CQ dry run.

[Zhenyao Mo, 2015-09-09 00:15:17]

The CQ bit was checked by zmo@chromium.org

[commit-bot: I haz the power, 2015-09-09 00:15:43]

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

[commit-bot: I haz the power, 2015-09-09 00:27:01]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2015-09-09 00:27:46]

Patchset 4 (id:??) landed as
https://crrev.com/67265f5b35173165472621909462d3cac4b500bd
Cr-Commit-Position: refs/heads/master@{#347844}