[Command Buffer] enable/disable FRAMEBUFFER_SRGB only when sRGB image is active

gpu/command_buffer/service/context_state.cc

Index: gpu/command_buffer/service/context_state.cc
diff --git a/gpu/command_buffer/service/context_state.cc b/gpu/command_buffer/service/context_state.cc
index e9e4de8bb8174d729c271dd43a181791b408c174..c9e4199636ee5250fed5b3853b4439c6b97f855b 100644
--- a/gpu/command_buffer/service/context_state.cc
+++ b/gpu/command_buffer/service/context_state.cc
@@ -495,15 +495,13 @@ void ContextState::RestoreState(const ContextState* prev_state) {
   RestoreIndexedUniformBufferBindings(prev_state);
   RestoreGlobalState(prev_state);
 
-  if (!prev_state) {
-    if (feature_info_->feature_flags().desktop_srgb_support) {
-      framebuffer_srgb_ = false;
-      glDisable(GL_FRAMEBUFFER_SRGB);
-    }
-  } else if (framebuffer_srgb_ != prev_state->framebuffer_srgb_) {

[yunchao, 2016/08/24 14:08:51]

It is OK to keep Qiankun's code here. I just want to say the framebuffer_srgb
status is not context-aware at all. The only thing we need to make sure is: keep
the flag framebuffer_srgb_ to be consistent with what is really should be
(glDisable or glEnable FRAMEBUFFER_SRGB) during context init/restore.  

[qiankun, 2016/08/24 16:21:00]

Agree with you.

+  if (prev_state && framebuffer_srgb_ != prev_state->framebuffer_srgb_) {
     // FRAMEBUFFER_SRGB will be restored lazily at render time.

[qiankun, 2016/08/24 16:22:13]

This comment is wrong in your current implementation.

[yunchao, 2016/08/25 16:29:51]

That's true.

     framebuffer_srgb_ = prev_state->framebuffer_srgb_;
   }
+  if (feature_info_->feature_flags().desktop_srgb_support) {
+    EnableDisable(GL_FRAMEBUFFER_SRGB, framebuffer_srgb_);

[Zhenyao Mo, 2016/08/25 00:39:37]

That's what we try to avoid.  If we use virtual contexts, then every frame we
switching contexts multiple times, at least between WebGL and compositor. The
original design is to avoid enable/disable FRAMEBUFFER_SRGB per frame, but now
it is per frame.

[yunchao, 2016/08/25 16:29:51]

You are correct. The original code is better.

+  }
 }
 
 ErrorState* ContextState::GetErrorState() {