Simplify rate limiter logic in Canvas2DLayerBridge

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 123 matching lines @@
       m_logger(WTF::wrapUnique(new Logger)),
       m_weakPtrFactory(this),
       m_imageBuffer(0),
       m_msaaSampleCount(msaaSampleCount),
       m_bytesAllocated(0),
       m_haveRecordedDrawCommands(false),
       m_destructionInProgress(false),
       m_filterQuality(kLow_SkFilterQuality),
       m_isHidden(false),
       m_isDeferralEnabled(true),
-      m_isRegisteredTaskObserver(false),
-      m_renderingTaskCompletedForCurrentFrame(false),
+      m_previousFrameWasPresented(

[xlai (Olivia), 2017/02/17 19:39:53]

Will it be better to rename this as "m_nextFrameNeedsToWait" and initialize it
to "false"? Or perhaps other better name.

[Justin Novosad, 2017/02/21 15:49:26]

Acknowledged.

+          true),  // true to prevent rate limiting at startup
       m_softwareRenderingWhileHidden(false),
       m_lastImageId(0),
       m_lastFilter(GL_LINEAR),
       m_accelerationMode(accelerationMode),
       m_opacityMode(opacityMode),
       m_size(size),
       m_colorSpace(colorSpace),
       m_skSurfacesUseColorSpace(skSurfacesUseColorSpace),
       m_colorType(colorType) {
   DCHECK(m_contextProvider);
@@ skipping 334 matching lines @@
     m_logger->reportHibernationEvent(HibernationAbortedDueGpuContextLoss);
     return;
   }
 
   if (!isAccelerated()) {
     m_logger->reportHibernationEvent(
         HibernationAbortedDueToSwitchToUnacceleratedRendering);
     return;
   }
 
-  TRACE_EVENT0("cc", "Canvas2DLayerBridge::hibernate");
+  TRACE_EVENT0("blink", "Canvas2DLayerBridge::hibernate");
   sk_sp<PaintSurface> tempHibernationSurface =
       PaintSurface::MakeRasterN32Premul(m_size.width(), m_size.height());
   if (!tempHibernationSurface) {
     m_logger->reportHibernationEvent(HibernationAbortedDueToAllocationFailure);
     return;
   }
   // No HibernationEvent reported on success. This is on purppose to avoid
   // non-complementary stats. Each HibernationScheduled event is paired with
   // exactly one failure or exit event.
   flushRecordingOnly();
@@ skipping 151 matching lines @@
     return;
   if (isHibernating())
     m_logger->reportHibernationEvent(HibernationEndedWithTeardown);
   m_hibernationImage.reset();
   m_recorder.reset();
   m_imageBuffer = nullptr;
   m_destructionInProgress = true;
   setIsHidden(true);
   m_surface.reset();
 
-  unregisterTaskObserver();
-
   if (m_layer && m_accelerationMode != DisableAcceleration) {
     GraphicsLayer::unregisterContentsLayer(m_layer->layer());
     m_layer->clearTexture();
     // Orphaning the layer is required to trigger the recration of a new layer
     // in the case where destruction is caused by a canvas resize. Test:
     // virtual/gpu/fast/canvas/canvas-resize-after-paint-without-layout.html
     m_layer->layer()->removeFromParent();
   }
 
   DCHECK(!m_bytesAllocated);
 }
 
-void Canvas2DLayerBridge::unregisterTaskObserver() {
-  if (m_isRegisteredTaskObserver) {
-    Platform::current()->currentThread()->removeTaskObserver(this);
-    m_isRegisteredTaskObserver = false;
-  }
-}
-
 void Canvas2DLayerBridge::setFilterQuality(SkFilterQuality filterQuality) {
   DCHECK(!m_destructionInProgress);
   m_filterQuality = filterQuality;
   if (m_layer)
     m_layer->setNearestNeighbor(m_filterQuality == kNone_SkFilterQuality);
 }
 
 void Canvas2DLayerBridge::setIsHidden(bool hidden) {
   bool newHiddenValue = hidden || m_destructionInProgress;
   if (m_isHidden == newHiddenValue)
@@ skipping 62 matching lines @@
 }
 
 void Canvas2DLayerBridge::skipQueuedDrawCommands() {
   if (m_haveRecordedDrawCommands) {
     m_recorder->finishRecordingAsPicture();
     startRecording();
     m_haveRecordedDrawCommands = false;
   }
 
   if (m_isDeferralEnabled) {
-    unregisterTaskObserver();
     if (m_rateLimiter)
       m_rateLimiter->reset();
   }
 }
 
 void Canvas2DLayerBridge::flushRecordingOnly() {
   DCHECK(!m_destructionInProgress);
 
   if (m_haveRecordedDrawCommands && getOrCreateSurface()) {
     TRACE_EVENT0("cc", "Canvas2DLayerBridge::flushRecordingOnly");
@@ skipping 107 matching lines @@
     std::unique_ptr<cc::SingleReleaseCallback>* outReleaseCallback) {
   if (m_destructionInProgress) {
     // It can be hit in the following sequence.
     // 1. Canvas draws something.
     // 2. The compositor begins the frame.
     // 3. Javascript makes a context be lost.
     // 4. Here.
     return false;
   }
 
+  m_previousFrameWasPresented = true;
+  if (m_rateLimiter) {
+    m_rateLimiter->reset();

[xlai (Olivia), 2017/02/17 19:39:53]

Are you sure that this is the only place when the animation is consumed and
committed to the compositor?

[Justin Novosad, 2017/02/21 15:49:26]

For non-offscreen gpu-accelerated 2D canvases, it is.

+  }
+
   // If the context is lost, we don't know if we should be producing GPU or
   // software frames, until we get a new context, since the compositor will
   // be trying to get a new context and may change modes.
   if (!m_contextProvider ||
       m_contextProvider->contextGL()->GetGraphicsResetStatusKHR() !=
           GL_NO_ERROR)
     return false;
 
   DCHECK(isAccelerated() || isHibernating() || m_softwareRenderingWhileHidden);
 
@@ skipping 115 matching lines @@
   if (m_isDeferralEnabled) {
     m_haveRecordedDrawCommands = true;
     IntRect pixelBounds = enclosingIntRect(rect);
     m_recordingPixelCount += pixelBounds.width() * pixelBounds.height();
     if (m_recordingPixelCount >=
         (m_size.width() * m_size.height() *
          ExpensiveCanvasHeuristicParameters::ExpensiveOverdrawThreshold)) {
       disableDeferral(DisableDeferralReasonExpensiveOverdrawHeuristic);
     }
   }
-  if (!m_isRegisteredTaskObserver) {
-    Platform::current()->currentThread()->addTaskObserver(this);
-    m_isRegisteredTaskObserver = true;
-  }
   m_didDrawSinceLastFlush = true;
   m_didDrawSinceLastGpuFlush = true;
 }
 
 void Canvas2DLayerBridge::finalizeFrame() {
+  TRACE_EVENT0("blink", "Canvas2DLayerBridge::finalizeFrame");
   DCHECK(!m_destructionInProgress);
 
   // Make sure surface is ready for painting: fix the rendering mode now
   // because it will be too late during the paint invalidation phase.
   getOrCreateSurface(PreferAcceleration);
 
-  if (m_rateLimiter)
-    m_rateLimiter->reset();
-  m_renderingTaskCompletedForCurrentFrame = false;
-}
-
-void Canvas2DLayerBridge::doPaintInvalidation(const FloatRect& dirtyRect) {
-  DCHECK(!m_destructionInProgress);
-  if (m_layer && m_accelerationMode != DisableAcceleration)
-    m_layer->layer()->invalidateRect(enclosingIntRect(dirtyRect));
-}
-
-void Canvas2DLayerBridge::didProcessTask() {
-  TRACE_EVENT0("cc", "Canvas2DLayerBridge::didProcessTask");
-  DCHECK(m_isRegisteredTaskObserver);
-  // If m_renderTaskProcessedForCurrentFrame is already set to true,
-  // it means that rendering tasks are not synchronized with the compositor
-  // (i.e. not using requestAnimationFrame), so we are at risk of posting
-  // a multi-frame backlog to the GPU
-  if (m_renderingTaskCompletedForCurrentFrame) {
+  if (!m_previousFrameWasPresented) {
     if (isAccelerated()) {
       flushGpu();
       if (!m_rateLimiter) {
         m_rateLimiter =
             SharedContextRateLimiter::create(MaxCanvasAnimationBacklog);
       }
     } else {
       flush();
     }
+  } else {
+    m_previousFrameWasPresented = false;

[xlai (Olivia), 2017/02/17 19:39:53]

If the previous frame was successfully committed (and is true), why calling
another finalizeFrame() will make it false?  Also the initial call to
finalizeFrame() also make it false?

[Justin Novosad, 2017/02/21 15:49:26]

The basic idea is to activate rate limiting when finalize frame gets called more
than once between commits.  I'm going to rename things and add comments to make
this more readable.

   }
 
   if (m_rateLimiter) {
     m_rateLimiter->tick();
   }
-
-  m_renderingTaskCompletedForCurrentFrame = true;
-  unregisterTaskObserver();
 }
 
-void Canvas2DLayerBridge::willProcessTask() {
-  NOTREACHED();
+void Canvas2DLayerBridge::doPaintInvalidation(const FloatRect& dirtyRect) {
+  DCHECK(!m_destructionInProgress);
+  if (m_layer && m_accelerationMode != DisableAcceleration)
+    m_layer->layer()->invalidateRect(enclosingIntRect(dirtyRect));
 }
 
 sk_sp<SkImage> Canvas2DLayerBridge::newImageSnapshot(AccelerationHint hint,
                                                      SnapshotReason) {
   if (isHibernating())
     return m_hibernationImage;
   if (!checkSurfaceValid())
     return nullptr;
   if (!getOrCreateSurface(hint))
     return nullptr;
@@ skipping 32 matching lines @@
     default;
 
 void Canvas2DLayerBridge::Logger::reportHibernationEvent(
     HibernationEvent event) {
   DEFINE_STATIC_LOCAL(EnumerationHistogram, hibernationHistogram,
                       ("Canvas.HibernationEvents", HibernationEventCount));
   hibernationHistogram.count(event);
 }
 
 }  // namespace blink