Prevent implicit framebuffer clear from clobbering alpha in emulated RGB

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

 /*
  * Copyright (c) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 969 matching lines @@
 
     // On old AMD GPUs on OS X, glColorMask doesn't work correctly for
     // multisampled renderbuffers and the alpha channel can be overwritten.
     // Clear the alpha channel of |m_fbo|.
     if (defaultBufferRequiresAlphaChannelToBePreserved() &&
         contextProvider()
             ->getCapabilities()
             .disable_multisampling_color_mask_usage) {
       m_gl->ClearColor(0, 0, 0, 1);
       m_gl->ColorMask(false, false, false, true);
+      m_gl->Clear(GL_COLOR_BUFFER_BIT);
+      m_gl->ColorMask(true, true, true, false);

[Ken Russell (switch to Gerrit), 2016/12/01 07:20:43]

While I'm pretty sure this was just an omission, this code path isn't tested on
any of our waterfall machines. Could you do at least a manual test where you
modify your test to use the AA path and run the browser with the command line
argument --disable_multisampling_color_mask_usage ? (GPU driver bug workarounds
can be specified on the command line.) Let's make sure this code path is taken
in that case, and that it still renders correctly. (We should also manually test
PlayCanvas's test case with this command line argument.)

[erikchen, 2016/12/01 16:04:11]

This was a relatively recent regression from:
https://chromiumcodereview.appspot.com/2402273002/diff/140001/third_party/Web...

Can we use a nested instance of m_stateRestorer [not sure if it supports nesting
off the top of my head]

[ccameron, 2016/12/01 19:25:59]

Gah, sorry!!!

Shouldn't we be doing
  m_client->DrawingBufferClientRestoreMaskAndClearValues();
to restore the values that the client expects (that's what it was before my
patch chopped out the Clear by accident).

[Kai Ninomiya, 2016/12/01 23:30:20]

I see, it looks like the `m_stateRestorer->setClearStateDirty();` added in your
patch ought to take care of this.

In that case, I should only need to add in the Clear, right?

     }
   }
 
   m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_fbo);
   if (m_antiAliasingMode == ScreenSpaceAntialiasing)
     m_gl->ApplyScreenSpaceAntialiasingCHROMIUM();
   m_contentsChangeCommitted = true;
 }
 
 void DrawingBuffer::restoreFramebufferBindings() {
@@ skipping 261 matching lines @@
   if (m_pixelUnpackBufferBindingDirty)
     client->DrawingBufferClientRestorePixelUnpackBufferBinding();
 }
 
 bool DrawingBuffer::shouldUseChromiumImage() {
   return RuntimeEnabledFeatures::webGLImageChromiumEnabled() &&
          m_chromiumImageUsage == AllowChromiumImage;
 }
 
 }  // namespace blink