Avoid to resolve CMAA for read and draw without content changed or with content committed.

third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp

Index: third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
index 7a26d0f0102499c20e5b3afe6f6e882fb03cdeeb..e30fb8ecc684b3e67da55e829262206db3551a44 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
@@ -984,7 +984,7 @@ void DrawingBuffer::ResolveAndBindForReadAndDraw() {
 void DrawingBuffer::ResolveMultisampleFramebufferInternal() {
   DCHECK(state_restorer_);
   state_restorer_->SetFramebufferBindingDirty();
-  if (WantExplicitResolve() && !contents_change_resolved_) {
+  if (WantExplicitResolve()) {
     state_restorer_->SetClearStateDirty();
     gl_->BindFramebuffer(GL_READ_FRAMEBUFFER_ANGLE, multisample_fbo_);
     gl_->BindFramebuffer(GL_DRAW_FRAMEBUFFER_ANGLE, fbo_);
@@ -1017,7 +1017,7 @@ void DrawingBuffer::ResolveMultisampleFramebufferInternal() {
 }
 
 void DrawingBuffer::ResolveIfNeeded() {
-  if (anti_aliasing_mode_ != kNone)
+  if (anti_aliasing_mode_ != kNone && !contents_change_resolved_)

[Zhenyao Mo, 2017/04/13 17:36:06]

This is a behavioral change.  Because before even if contents_change_resolved_
is true, we still bind fbo_ (line $1019), but now we don't.

Is it OK? Please do a bit careful study and provide an answer (with explanation)
here.

[xinghua.cao, 2017/04/26 10:46:37]

Zhenyao, I had made a more carefully investigation on this issue. List my
comment as below,
I think DrawingBuffer::ResolveMultisampleFramebufferInternal is used to only
resolve the multisampled buffer into back_color_buffer_ texture.
ResolveIfNeeded function is called by
DrawingBuffer::PrepareTextureMailboxInternal,
DrawingBuffer::CopyToPlatformTexture and
DrawingBuffer::ResolveAndBindForReadAndDraw.
DrawingBuffer::PrepareTextureMailboxInternal and
DrawingBuffer::CopyToPlatformTexture only operate back_color_buffer_ texture, do
not operate fbo, and do not require bind to fbo_.
DrawingBuffer::ResolveAndBindForReadAndDraw had already binded fbo_ to
GL_FRAMEBUFFER target after resolving multisample, because it need to read and
draw fbo_.
In summary, I think binding fbo_ is only needed if we want to operate the fbo_.
if we only want to operate attached texture, does not need bind fbo_, and I
think only read and draw need to bind fbo_ to GL_FRAMEBUFFER after resolve
multisample. And if one function wants operate fbo_, it should explicitly bind
fbo_ in its body, like DrawingBuffer::ResolveAndBindForReadAndDraw. Is my
understanding right?
I think that binding fbo_ (line $1014) is only useful for
ApplyScreenSpaceAntialiasingCHROMIUM (CMAA operation, anti_aliasing_mode is
kScreenSpaceAntialiasing) if GL_FRAMEBUFFER target is currently binding to zero,
if not, it is already binding on fbo_.

By the way, I have another question. Why should bind to fbo_ after every resolve
multisample? If it was originally binding to multisample_fbo_, after resolve
multsample, it is binding to fbo_. Is that right?

[Ken Russell (switch to Gerrit), 2017/04/28 00:10:00]

This sounds correct. One note: DrawingBuffer::PrepareTextureMailboxInternal
calls ResolveIfNeeded and immediately calls FinishPrepareTextureMailboxGpu.
FinishPrepareTextureMailboxGpu calls AttachColorBufferToReadFramebuffer -- which
sounds like it's relying on the currently bound framebuffer -- but in fact it
binds fbo_ itself.
I agree. It looks like that could be done only if we're going to call
ApplyScreenSpaceAntialiasingCHROMIUM.
I don't see a good reason for this and think this could be changed.

[dshwang, 2017/04/28 02:08:29]

I think so.
I agree. as ResolveIfNeeded() is private method, public caller must take care of
fbo state.

[dshwang, 2017/04/28 02:48:42]

ResolveMultisampleFramebufferInternal() set fbo dirty to state_restorer_, so
it's ok to bind any FBO in this moment.

     ResolveMultisampleFramebufferInternal();
   contents_change_resolved_ = true;
 }