[Command buffer] It is unnecessary to set clear_state_dirty when image content changed

[Command buffer] It is unnecessary to set clear_state_dirty when image content changed
by TexStorage2D, TexImage2D, or CompressedTexImge2D. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set clear_state_dirty 
in the caller(TexStorage2D, TexImage2D, CompressedTexImage2D).

BUG=678637
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

[yunchao, 2017-01-05 06:32:02]

Description was changed from

==========
disable clear_state_dirty for image content change


remove unnecessary clear_state_dirty

BUG=
==========

to

==========
disable clear_state_dirty for image content change


remove unnecessary clear_state_dirty

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2017-01-05 15:03:37]

The CQ bit was checked by yunchao.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 15:03:48]

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

[yunchao, 2017-01-05 15:10:30]

Description was changed from

==========
disable clear_state_dirty for image content change


remove unnecessary clear_state_dirty

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear(), that function
should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2017-01-05 15:44:09]

Description was changed from

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear(), that function
should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2017-01-05 16:19:30]

Description was changed from

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty.

BUG=678637
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2017-01-05 16:19:30]

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

[yunchao, 2017-01-05 16:19:55]

PTAL. Thanks!

[Ken Russell (switch to Gerrit), 2017-01-05 16:23:34]

Yunchao, could you please add a unit test for this change?

[commit-bot: I haz the power, 2017-01-05 16:24:05]

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

[commit-bot: I haz the power, 2017-01-05 16:24:05]

Dry run: This issue passed the CQ dry run.

[yunchao, 2017-01-05 16:26:58]

On 2017/01/05 16:23:34, Ken Russell wrote:
> Yunchao, could you please add a unit test for this change?

Thanks for your kind reminder, Ken. I know that we should add unittest. But
seems that it is not possible to add appropirate unittest for code removal?

[Ken Russell (switch to Gerrit), 2017-01-05 16:29:10]

On 2017/01/05 16:26:58, yunchao wrote:
> On 2017/01/05 16:23:34, Ken Russell wrote:
> > Yunchao, could you please add a unit test for this change?
> 
> Thanks for your kind reminder, Ken. I know that we should add unittest. But
> seems that it is not possible to add appropirate unittest for code removal?

I haven't studied this code in detail but think it would be possible. The unit
test would run the appropriate command and make sure that
framebuffer_state_.clear_state_dirty was not set to true.

[yunchao, 2017-01-05 16:38:55]

On 2017/01/05 16:29:10, Ken Russell wrote:
> On 2017/01/05 16:26:58, yunchao wrote:
> > On 2017/01/05 16:23:34, Ken Russell wrote:
> > > Yunchao, could you please add a unit test for this change?
> > 
> > Thanks for your kind reminder, Ken. I know that we should add unittest. But
> > seems that it is not possible to add appropirate unittest for code removal?
> 
> I haven't studied this code in detail but think it would be possible. The unit
> test would run the appropriate command and make sure that
> framebuffer_state_.clear_state_dirty was not set to true.

OK, I will try to add unittest for this. However, we still need to make sure the
logic itself is correct. You know, sometimes, unit test is well designed to pass
 and prevent regression only. It may not expose the logic error in current
change.

[yunchao, 2017-01-05 16:42:23]

Description was changed from

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty.

BUG=678637
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty in the caller.

BUG=678637
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[yunchao, 2017-01-05 16:44:06]

Description was changed from

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty in the caller.

BUG=678637
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
[Command buffer] It is unnecessary to set clear_state_dirty when image content
changed
by TexStorage2D, TexImage2D, or CompressedTexImge2D. 

If the image is cleared to 0 by glColorMask() and glClear() in its callee, 
the callee should call RestoreClearState by itself. It is uncessary to set
clear_state_dirty 
in the caller(TexStorage2D, TexImage2D, CompressedTexImage2D).

BUG=678637
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Zhenyao Mo, 2017-01-05 18:24:12]

On 2017/01/05 16:38:55, yunchao wrote:
> On 2017/01/05 16:29:10, Ken Russell wrote:
> > On 2017/01/05 16:26:58, yunchao wrote:
> > > On 2017/01/05 16:23:34, Ken Russell wrote:
> > > > Yunchao, could you please add a unit test for this change?
> > > 
> > > Thanks for your kind reminder, Ken. I know that we should add unittest.
But
> > > seems that it is not possible to add appropirate unittest for code
removal?
> > 
> > I haven't studied this code in detail but think it would be possible. The
unit
> > test would run the appropriate command and make sure that
> > framebuffer_state_.clear_state_dirty was not set to true.
> 
> OK, I will try to add unittest for this. However, we still need to make sure
the
> logic itself is correct. You know, sometimes, unit test is well designed to
pass
>  and prevent regression only. It may not expose the logic error in current
> change.

not lgtm

See my comment in https://bugs.chromium.org/p/chromium/issues/detail?id=678637