command_buffer: Defer restoring of FBO bindings when changing virtual contexts

command_buffer: Defer restoring of FBO bindings when changing virtual contexts

Defer restoring of FBO bindings when changing virtual contexts.

Restoring FBO bindings during MakeCurrent of a virtual context consumes
significant amount of the MakeCurrent duration. For GPU rasterization,
this work is wasted, as Skia will rebind GL state, including FBOs, for
each tile rendered.

Defer the FBO binding only during the context state restore, not
when client calls BindFramebuffer. This is mainly because the
unit test expectations are hard to change correctly.

Following operations leave the FBO binding dirty:
* Decoder state restore (Virtual context MakeCurrent)
* Decoder MakeCurrent (Non-virtual context MakeCurrent)
* Client deleting an FBO that is bound
* Copying a texture with CopyTextureCHROMIUM
* ScopedResolvedFrameBufferBinder

Other operations will bind the correct
FBO eagerly:
* Client calling BindFramebuffer
* Decoder switching surfaces
* ...

These are left to bind eagerly, as the current test expectations are
laborous to modify. Later on, if the benefit in uniformity of the decoder code
or performance is seen as worth the complication in the test expectation setup,
these could be modified too.

All the operations that use the FBOs will now call 
GLES2DecoderImpl::ApplyFramebufferBindings before doing operations 
that need the FBO.

Changes to the tests:
* Deleting FBOs do not cause any BindFramebuffer calls, so remove the expectation.
* Using another FBO after FBO delete will cause BindFramebuffer call, so add the 
expectation at the use site.
* Few tests did bind the targeted framebuffer multiple times without reason.


BUG=603407
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

[Kimmo Kinnunen, 2016-04-27 13:18:02]

Description was changed from

==========
command_buffer: Defer restoring of FBO bindings when changing virtual contexts

Defer restoring of FBO bindings when changing virtual contexts.

Restoring FBO bindings during MakeCurrent of a virtual context consumes
significant amount of the MakeCurrent duration. For GPU rasterization,
this work is wasted, as Skia will rebind GL state, including FBOs, for
each tile rendered.

Defer the FBO binding only during the context state restore, not
when client calls BindFramebuffer. This is mainly because the
unit test expectations are hard to change correctly.

TODO: the build_gles2_cmd_buffer.py change related to state_flag
might need to be a bit nicer.

BUG=603407
==========

to

==========
command_buffer: Defer restoring of FBO bindings when changing virtual contexts

Defer restoring of FBO bindings when changing virtual contexts.

Restoring FBO bindings during MakeCurrent of a virtual context consumes
significant amount of the MakeCurrent duration. For GPU rasterization,
this work is wasted, as Skia will rebind GL state, including FBOs, for
each tile rendered.

Defer the FBO binding only during the context state restore, not
when client calls BindFramebuffer. This is mainly because the
unit test expectations are hard to change correctly.

TODO: the build_gles2_cmd_buffer.py change related to state_flag
might need to be a bit nicer.

BUG=603407
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
==========

[Kimmo Kinnunen, 2016-04-28 11:41:57]

Description was changed from

==========
command_buffer: Defer restoring of FBO bindings when changing virtual contexts

Defer restoring of FBO bindings when changing virtual contexts.

Restoring FBO bindings during MakeCurrent of a virtual context consumes
significant amount of the MakeCurrent duration. For GPU rasterization,
this work is wasted, as Skia will rebind GL state, including FBOs, for
each tile rendered.

Defer the FBO binding only during the context state restore, not
when client calls BindFramebuffer. This is mainly because the
unit test expectations are hard to change correctly.

TODO: the build_gles2_cmd_buffer.py change related to state_flag
might need to be a bit nicer.

BUG=603407
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

==========
command_buffer: Defer restoring of FBO bindings when changing virtual contexts

Defer restoring of FBO bindings when changing virtual contexts.

Restoring FBO bindings during MakeCurrent of a virtual context consumes
significant amount of the MakeCurrent duration. For GPU rasterization,
this work is wasted, as Skia will rebind GL state, including FBOs, for
each tile rendered.

Defer the FBO binding only during the context state restore, not
when client calls BindFramebuffer. This is mainly because the
unit test expectations are hard to change correctly.

Following operations leave the FBO binding dirty:
* Decoder state restore (Virtual context MakeCurrent)
* Decoder MakeCurrent (Non-virtual context MakeCurrent)
* Client deleting an FBO that is bound
* Copying a texture with CopyTextureCHROMIUM
* ScopedResolvedFrameBufferBinder

Other places, are left intact, e.g. sites will bind the correct
FBO eagerly:
* Client calling BindFramebuffer
* Decoder switching surfaces
* Others

These are left to bind eagerly, as the current test expectations are
laborous to modify. Later on, if the benefit in uniformity of the decoder code
or performance is seen as worth the complication in the test expectation setup,
these could be modified too.

All the operations that use the FBOs will now call 
GLES2DecoderImpl::ApplyFramebufferBindings before doing operations 
that need the FBO.

Changes to the tests:
* Deleting FBOs do not cause any BindFramebuffer calls, so remove the
expectation.
* Using another FBO after FBO delete will cause BindFramebuffer call, so add the

expectation at the use site.
* Few tests did bind the targeted framebuffer multiple times without reason.


BUG=603407
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
==========

[Kimmo Kinnunen, 2016-04-28 11:45:26]

kkinnunen@nvidia.com changed reviewers:
+ vmiura@chromium.org, zmo@google.com

[Kimmo Kinnunen, 2016-04-28 11:45:27]

This is part of the work discussed a while ago (see the bug).

This seems to cause non-trivial performance increase, even without any other
state deferral.

The question I can see coming is wrt. how to ensure that this catches all the
relevant FBO use sites, and how to enforce correctness also after commit. Do you
guys see this as a problem, or can you just review this sort of stuff and see if
it's correct or not? If there's a problem, should I invent some more elaborate
RAII scheme in order for this to be accepted?

https://codereview.chromium.org/1925663002/diff/20001/gpu/config/gpu_driver_b...
File gpu/config/gpu_driver_bug_list_json.cc (left):

https://codereview.chromium.org/1925663002/diff/20001/gpu/config/gpu_driver_b...
gpu/config/gpu_driver_bug_list_json.cc:808: "description": "Use virtual contexts
on NVIDIA with GLES 3.1",
just for telemetry perf testing

[Kimmo Kinnunen, 2016-04-28 11:55:19]

Description was changed from

==========
command_buffer: Defer restoring of FBO bindings when changing virtual contexts

Defer restoring of FBO bindings when changing virtual contexts.

Restoring FBO bindings during MakeCurrent of a virtual context consumes
significant amount of the MakeCurrent duration. For GPU rasterization,
this work is wasted, as Skia will rebind GL state, including FBOs, for
each tile rendered.

Defer the FBO binding only during the context state restore, not
when client calls BindFramebuffer. This is mainly because the
unit test expectations are hard to change correctly.

Following operations leave the FBO binding dirty:
* Decoder state restore (Virtual context MakeCurrent)
* Decoder MakeCurrent (Non-virtual context MakeCurrent)
* Client deleting an FBO that is bound
* Copying a texture with CopyTextureCHROMIUM
* ScopedResolvedFrameBufferBinder

Other places, are left intact, e.g. sites will bind the correct
FBO eagerly:
* Client calling BindFramebuffer
* Decoder switching surfaces
* Others

These are left to bind eagerly, as the current test expectations are
laborous to modify. Later on, if the benefit in uniformity of the decoder code
or performance is seen as worth the complication in the test expectation setup,
these could be modified too.

All the operations that use the FBOs will now call 
GLES2DecoderImpl::ApplyFramebufferBindings before doing operations 
that need the FBO.

Changes to the tests:
* Deleting FBOs do not cause any BindFramebuffer calls, so remove the
expectation.
* Using another FBO after FBO delete will cause BindFramebuffer call, so add the

expectation at the use site.
* Few tests did bind the targeted framebuffer multiple times without reason.


BUG=603407
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

==========
command_buffer: Defer restoring of FBO bindings when changing virtual contexts

Defer restoring of FBO bindings when changing virtual contexts.

Restoring FBO bindings during MakeCurrent of a virtual context consumes
significant amount of the MakeCurrent duration. For GPU rasterization,
this work is wasted, as Skia will rebind GL state, including FBOs, for
each tile rendered.

Defer the FBO binding only during the context state restore, not
when client calls BindFramebuffer. This is mainly because the
unit test expectations are hard to change correctly.

Following operations leave the FBO binding dirty:
* Decoder state restore (Virtual context MakeCurrent)
* Decoder MakeCurrent (Non-virtual context MakeCurrent)
* Client deleting an FBO that is bound
* Copying a texture with CopyTextureCHROMIUM
* ScopedResolvedFrameBufferBinder

Other operations will bind the correct
FBO eagerly:
* Client calling BindFramebuffer
* Decoder switching surfaces
* ...

These are left to bind eagerly, as the current test expectations are
laborous to modify. Later on, if the benefit in uniformity of the decoder code
or performance is seen as worth the complication in the test expectation setup,
these could be modified too.

All the operations that use the FBOs will now call 
GLES2DecoderImpl::ApplyFramebufferBindings before doing operations 
that need the FBO.

Changes to the tests:
* Deleting FBOs do not cause any BindFramebuffer calls, so remove the
expectation.
* Using another FBO after FBO delete will cause BindFramebuffer call, so add the

expectation at the use site.
* Few tests did bind the targeted framebuffer multiple times without reason.


BUG=603407
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
==========

[vmiura, 2016-04-28 22:28:20]

Some sites in ::GetHelper may also need to have the FBOs applied.

Good question about how to check if we have full coverage of sites.

Side note, why don't drivers update the FBO lazily themselves?

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

https://codereview.chromium.org/1925663002/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:4732:
MarkFramebufferBindingsChanged();
Based on the previous comment here, we have to ensure the frame buffer is bound
before restoring Viewport settings on some drivers.

Perhaps this could be handled similar to the "restore_scissor_on_fbo_change"
workaround.

https://codereview.chromium.org/1925663002/diff/20001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:6218: if (attachment ==
GL_DEPTH_STENCIL_ATTACHMENT) {
Also needs ApplyFramebufferBindings(target) ?