Apply FRAMEBUFFER_SRGB for blitFramebuffer

Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/framebufferblit/*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

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

[qiankun, 2016-06-28 07:38:21]

Description was changed from

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/negativebufferapi.html
==========

to

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/negativebufferapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[qiankun, 2016-06-28 09:26:06]

qiankun.miao@intel.com changed reviewers:
+ bajones@chromium.org, kbr@chromium.org, yunchao.he@intel.com, zmo@chromium.org

[qiankun, 2016-06-28 09:26:07]

PTAL.

[Zhenyao Mo, 2016-06-28 17:29:47]

On 2016/06/28 09:26:07, qiankun wrote:
> PTAL.

This is definitely the fix we need, but here we could be doing unnecessary
change of FRAMEBUFFER_SRGB in CheckBoundDrawFramebufferValid().

Can you think about a solution where we rid of that?

[qiankun, 2016-06-29 03:05:49]

https://codereview.chromium.org/2106703002/diff/1/gpu/command_buffer/service/...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/2106703002/diff/1/gpu/command_buffer/service/...
gpu/command_buffer/service/gles2_cmd_decoder.cc:3996: if (valid &&
feature_info_->feature_flags().desktop_srgb_support) {
zmo@ does it make sense to you if I check fun_name here to avoid duplicate
FRAMEBUFFER_SRGB setting for blitFramebuffer?

[Zhenyao Mo, 2016-06-29 05:03:19]

On 2016/06/29 03:05:49, qiankun wrote:
>
https://codereview.chromium.org/2106703002/diff/1/gpu/command_buffer/service/...
> File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> 
>
https://codereview.chromium.org/2106703002/diff/1/gpu/command_buffer/service/...
> gpu/command_buffer/service/gles2_cmd_decoder.cc:3996: if (valid &&
> feature_info_->feature_flags().desktop_srgb_support) {
> zmo@ does it make sense to you if I check fun_name here to avoid duplicate
> FRAMEBUFFER_SRGB setting for blitFramebuffer?

That's too "hacky" in my opinion.  Maybe add another function
CheckBoundFramebufferValid() that combines both Draw and Read, and then you will
only set FRAMEBUFFER_SRGB at most once.

[qiankun, 2016-06-29 06:04:55]

On 2016/06/29 05:03:19, Zhenyao Mo wrote:
> On 2016/06/29 03:05:49, qiankun wrote:
> >
>
https://codereview.chromium.org/2106703002/diff/1/gpu/command_buffer/service/...
> > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):
> > 
> >
>
https://codereview.chromium.org/2106703002/diff/1/gpu/command_buffer/service/...
> > gpu/command_buffer/service/gles2_cmd_decoder.cc:3996: if (valid &&
> > feature_info_->feature_flags().desktop_srgb_support) {
> > zmo@ does it make sense to you if I check fun_name here to avoid duplicate
> > FRAMEBUFFER_SRGB setting for blitFramebuffer?
> 
> That's too "hacky" in my opinion.  Maybe add another function
> CheckBoundFramebufferValid() that combines both Draw and Read, and then you
will
> only set FRAMEBUFFER_SRGB at most once.

Done.

[qiankun, 2016-06-29 06:12:37]

Description was changed from

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/negativebufferapi.html
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/framebufferblit/*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[qiankun, 2016-06-29 15:58:46]

It seems there is a bug for win_optional_gpu_tests_rel bot:
"AssertionError(u'Triggered same task twice: webgl2_conformance_tests on ATI GPU
on Windows".

[Zhenyao Mo, 2016-06-29 16:48:10]

lgtm

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

https://codereview.chromium.org/2106703002/diff/40001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:4020: // This is only used by
DoBlitFramebufferCHROMIUM which operates read/draw
comment should be added at function declaration, not function definition.

[Ken Russell (switch to Gerrit), 2016-06-29 19:15:04]

On 2016/06/29 15:58:46, qiankun wrote:
> It seems there is a bug for win_optional_gpu_tests_rel bot:
> "AssertionError(u'Triggered same task twice: webgl2_conformance_tests on ATI
GPU
> on Windows".

Sorry about that. It's just been fixed in http://crbug.com/624156 .

[qiankun, 2016-06-30 01:56:11]

The CQ bit was checked by qiankun.miao@intel.com

[qiankun, 2016-06-30 01:56:11]

The patchset sent to the CQ was uploaded after l-g-t-m from zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/2106703002/#ps60001
(title: "fix comments")

[commit-bot: I haz the power, 2016-06-30 01:56:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[qiankun, 2016-06-30 01:56:30]

Landing now.

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

https://codereview.chromium.org/2106703002/diff/40001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:4020: // This is only used by
DoBlitFramebufferCHROMIUM which operates read/draw
On 2016/06/29 16:48:10, Zhenyao Mo wrote:
> comment should be added at function declaration, not function definition.

Done.

[commit-bot: I haz the power, 2016-06-30 02:52:47]

Description was changed from

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/framebufferblit/*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/framebufferblit/*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-06-30 02:52:48]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-30 02:52:58]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 02:55:38]

Description was changed from

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/framebufferblit/*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Apply FRAMEBUFFER_SRGB for blitFramebuffer

When read framebuffer contains sRGB formatted attachment, FRAMEBUFFER_SRGB
should be enabled for blitFramebuffer api on top of desktop GL.

BUG=429053
TEST=deqp/functional/gles3/framebufferblit/*
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/ee62ba686f5053bced5ef548edbb16948e9c278a
Cr-Commit-Position: refs/heads/master@{#403074}
==========

[commit-bot: I haz the power, 2016-06-30 02:55:39]

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