Add Intel macOS workaround for WebGL2 canvas_sub_rectangle tests

third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp

 /*
  * Copyright (C) 2012 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:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 171 matching lines @@
   }
   m_recordingPixelCount = 0;
 }
 
 void Canvas2DLayerBridge::setLoggerForTesting(std::unique_ptr<Logger> logger) {
   m_logger = std::move(logger);
 }
 
 bool Canvas2DLayerBridge::shouldAccelerate(AccelerationHint hint) const {
   bool accelerate;
-  if (m_softwareRenderingWhileHidden)
+  if (m_softwareRenderingWhileHidden) {
     accelerate = false;
-  else if (m_accelerationMode == ForceAccelerationForTesting)
+  } else if (m_accelerationMode == ForceAccelerationForTesting) {
     accelerate = true;
-  else if (m_accelerationMode == DisableAcceleration)
+  } else if (m_accelerationMode == DisableAcceleration) {
     accelerate = false;
-  else
+  } else {
     accelerate = hint == PreferAcceleration ||
-                 hint == PreferAccelerationAfterVisibilityChange;
+                 hint == PreferAccelerationAfterVisibilityChange ||
+                 hint == PreferAccelerationWithExtraSurfaceCopy;
+  }
 
   if (accelerate &&
       (!m_contextProvider ||
        m_contextProvider->contextGL()->GetGraphicsResetStatusKHR() !=
            GL_NO_ERROR))
     accelerate = false;
   return accelerate;
 }
 
 bool Canvas2DLayerBridge::isAccelerated() const {
@@ skipping 322 matching lines @@
     CanvasMetrics::countCanvasContextUsage(
         CanvasMetrics::GPUAccelerated2DCanvasSurfaceCreationFailed);
     m_surfaceCreationFailedAtLeastOnce = true;
   }
 }
 
 SkSurface* Canvas2DLayerBridge::getOrCreateSurface(AccelerationHint hint) {
   if (m_surface)
     return m_surface.get();
 
-  if (m_layer && !isHibernating() && hint == PreferAcceleration &&
+  if (m_layer && !isHibernating() &&
+      (hint == PreferAcceleration ||
+       hint == PreferAccelerationWithExtraSurfaceCopy) &&
       m_accelerationMode != DisableAcceleration) {
     return nullptr;  // re-creation will happen through restore()
   }
 
   bool wantAcceleration = shouldAccelerate(hint);
   if (CANVAS2D_BACKGROUND_RENDER_SWITCH_TO_CPU && isHidden() &&
       wantAcceleration) {
     wantAcceleration = false;
     m_softwareRenderingWhileHidden = true;
   }
@@ skipping 552 matching lines @@
     return nullptr;
   if (!getOrCreateSurface(hint))
     return nullptr;
   flush();
   // A readback operation may alter the texture parameters, which may affect
   // the compositor's behavior. Therefore, we must trigger copy-on-write
   // even though we are not technically writing to the texture, only to its
   // parameters.
   getOrCreateSurface()->notifyContentWillChange(
       SkSurface::kRetain_ContentChangeMode);
+
+  // Create a new SkSurface compatible with m_surface, and discard it
+  // immediately. This nonsense operation can suppress the flaky error, which
+  // will likely happen when later reading or peeking pixels back from the image
+  // snapshot, on Intel MacOS platform(crbug.com/665656).

[qiankun, 2016/12/05 08:25:31]

Brian & Eric, we found this workaround but we didn't root cause why result of
read back from canvas is wrong. Do you have any ideas?

[bsalomon, 2016/12/05 14:12:41]

Sorry, but I don't think we've encountered this particular quirk in Skia before.

+  if (hint == PreferAccelerationWithExtraSurfaceCopy)

[Justin Novosad, 2016/12/05 20:54:01]

This is a misuse of "hint". As it's name suggests, it is just a hint. The
purpose of the hint parameter is to indicate an opportunistic preference that
may or may not be ignored.  Using it for other purposes is confusing and bad for
readability.  Proof of confusingness: this code will be making an unnecessary
copy in cases where the canvas is not gpu accelerated (because the hint does not
force the canvas to be accelerated).  I think it would be cleaner to use the
SnapshotReason instead.  I have an idea how to do this cleanly.  Let me land a
small patch first.

+    m_surface->makeSurface(m_surface->getCanvas()->imageInfo()).reset();

[Zhenyao Mo, 2016/12/05 18:13:36]

This seems a big perf regression.  If we only encounter this bug when uploading
2d canvas to WebGL, should we only trigger this on-demand when we use this 2d
canvas as texture source in WebGL? (I haven't looked into how feasible it is
though, but doing it all the time on Intel Mac seems an overkill).

+
   return m_surface->makeImageSnapshot();
 }
 
 void Canvas2DLayerBridge::willOverwriteCanvas() {
   skipQueuedDrawCommands();
 }
 
 #if USE_IOSURFACE_FOR_2D_CANVAS
 Canvas2DLayerBridge::ImageInfo::ImageInfo(
     std::unique_ptr<gfx::GpuMemoryBuffer> gpuMemoryBuffer,
@@ skipping 15 matching lines @@
     default;
 
 void Canvas2DLayerBridge::Logger::reportHibernationEvent(
     HibernationEvent event) {
   DEFINE_STATIC_LOCAL(EnumerationHistogram, hibernationHistogram,
                       ("Canvas.HibernationEvents", HibernationEventCount));
   hibernationHistogram.count(event);
 }
 
 }  // namespace blink