Improved UMA and extended support for overlays

Recent Finch data has indicated a large percentage of failing overlays.
In order to better understand why these overlays might be failing we add
a new fail enum that indicates that the overlay candidate failure was
a deliberate act by prioritization.

Also included in this cl is a minor fix for a specific case with low
latency surfaces. Some low latency surfaces (eg Ink) do not change
content Ids even when the pixel data changes. We now detect these
content changes by testing for a valid surface damage index. If the
surface has a damage index we can assume that the content has
changed.



Change-Id: I00b7015ee78a952105c7107eb4cedaf798f88b12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2614860Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Peter McNeeley <petermcneeley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842996}

components/viz/common/display/overlay_strategy.h

@@ -22,7 +22,8 @@ enum class OverlayStrategy {
   kUnderlay = 4,
   kUnderlayCast = 5,
   kNoStrategyAllFail = 6,
-  kMaxValue = kNoStrategyAllFail,
+  kNoStrategyFailMin = 7,
+  kMaxValue = kNoStrategyFailMin,
 };
 
 // Parses a comma separated list of overlay strategy types and returns a list

components/viz/service/display/overlay_candidate.cc

@@ -31,8 +31,6 @@ namespace viz {
 constexpr uint32_t OverlayCandidate::kInvalidDamageIndex;
 
 namespace {
-// Tolerance for considering axis vector elements to be zero.
-const SkScalar kEpsilon = std::numeric_limits<float>::epsilon();
 
 const gfx::BufferFormat kOverlayFormats[] = {
     gfx::BufferFormat::RGBX_8888, gfx::BufferFormat::RGBA_8888,
@@ -43,14 +41,14 @@ const gfx::BufferFormat kOverlayFormats[] = {
 enum Axis { NONE, AXIS_POS_X, AXIS_NEG_X, AXIS_POS_Y, AXIS_NEG_Y };
 
 Axis VectorToAxis(const gfx::Vector3dF& vec) {
-  if (std::abs(vec.z()) > kEpsilon)
+  if (!cc::MathUtil::IsWithinEpsilon(vec.z(), 0.f))
     return NONE;
-  const bool x_zero = (std::abs(vec.x()) <= kEpsilon);
-  const bool y_zero = (std::abs(vec.y()) <= kEpsilon);
+  const bool x_zero = cc::MathUtil::IsWithinEpsilon(vec.x(), 0.f);
+  const bool y_zero = cc::MathUtil::IsWithinEpsilon(vec.y(), 0.f);
   if (x_zero && !y_zero)
-    return (vec.y() > 0) ? AXIS_POS_Y : AXIS_NEG_Y;
+    return (vec.y() > 0.f) ? AXIS_POS_Y : AXIS_NEG_Y;
   else if (y_zero && !x_zero)
-    return (vec.x() > 0) ? AXIS_POS_X : AXIS_NEG_X;
+    return (vec.x() > 0.f) ? AXIS_POS_X : AXIS_NEG_X;
   else
     return NONE;
 }
@@ -64,7 +62,7 @@ gfx::OverlayTransform GetOverlayTransform(const gfx::Transform& quad_transform,
   gfx::Vector3dF x_axis = cc::MathUtil::GetXAxis(quad_transform);
   gfx::Vector3dF y_axis = cc::MathUtil::GetYAxis(quad_transform);
   if (y_flipped) {
-    y_axis.Scale(-1);
+    y_axis.Scale(-1.f);
   }
 
   Axis x_to = VectorToAxis(x_axis);
@@ -121,20 +119,7 @@ gfx::Rect GetDamageRect(const DrawQuad* quad,
 
 }  // namespace
 
-OverlayCandidate::OverlayCandidate()
-    : transform(gfx::OVERLAY_TRANSFORM_NONE),
-      format(gfx::BufferFormat::RGBA_8888),
-      uv_rect(0.f, 0.f, 1.f, 1.f),
-      is_clipped(false),
-      is_opaque(false),
-      resource_id(0),
-#if defined(OS_ANDROID)
-      is_backed_by_surface_texture(false),
-      is_promotable_hint(false),
-#endif
-      overlay_handled(false),
-      gpu_fence_id(0) {
-}
+OverlayCandidate::OverlayCandidate() = default;
 
 OverlayCandidate::OverlayCandidate(const OverlayCandidate& other) = default;
 
@@ -153,23 +138,26 @@ bool OverlayCandidate::FromDrawQuad(
   if (!output_color_matrix.isIdentity())
     return false;
 
+  const SharedQuadState* sqs = quad->shared_quad_state;
+
   // We don't support an opacity value different than one for an overlay plane.
-  if (quad->shared_quad_state->opacity != 1.f)
+  if (sqs->opacity != 1.f)
     return false;
 
-  candidate->overlay_damage_index =
-      quad->shared_quad_state->overlay_damage_index.value_or(
-          kInvalidDamageIndex);
-
   // We can't support overlays with mask filter.
-  if (!quad->shared_quad_state->mask_filter_info.IsEmpty())
+  if (!sqs->mask_filter_info.IsEmpty())
     return false;
   // We support only kSrc (no blending) and kSrcOver (blending with premul).
-  if (!(quad->shared_quad_state->blend_mode == SkBlendMode::kSrc ||
-        quad->shared_quad_state->blend_mode == SkBlendMode::kSrcOver)) {
+  if (!(sqs->blend_mode == SkBlendMode::kSrc ||
+        sqs->blend_mode == SkBlendMode::kSrcOver)) {
     return false;
   }
 
+  candidate->requires_overlay = OverlayCandidate::RequiresOverlay(quad);
+
+  candidate->overlay_damage_index =
+      sqs->overlay_damage_index.value_or(kInvalidDamageIndex);
+
   switch (quad->material) {
     case DrawQuad::Material::kTextureContent:
       return FromTextureQuad(resource_provider, surface_damage_rect_list,
@@ -192,14 +180,14 @@ bool OverlayCandidate::FromDrawQuad(
 // static
 bool OverlayCandidate::IsInvisibleQuad(const DrawQuad* quad) {
   float opacity = quad->shared_quad_state->opacity;
-  if (opacity < std::numeric_limits<float>::epsilon())
+  if (cc::MathUtil::IsWithinEpsilon(opacity, 0.f))
     return true;
   if (quad->material != DrawQuad::Material::kSolidColor)
     return false;
   const SkColor color = SolidColorDrawQuad::MaterialCast(quad)->color;
-  const float alpha = (SkColorGetA(color) * (1.0f / 255.0f)) * opacity;
+  const float alpha = (SkColorGetA(color) * (1.f / 255.f)) * opacity;
   return quad->ShouldDrawWithBlending() &&
-         alpha < std::numeric_limits<float>::epsilon();
+         cc::MathUtil::IsWithinEpsilon(alpha, 0.f);
 }
 
 // static
@@ -313,17 +301,18 @@ bool OverlayCandidate::FromDrawQuadResource(
   if (!base::Contains(kOverlayFormats, candidate->format))
     return false;
 
-  gfx::OverlayTransform overlay_transform = GetOverlayTransform(
-      quad->shared_quad_state->quad_to_target_transform, y_flipped);
+  const SharedQuadState* sqs = quad->shared_quad_state;
+  gfx::OverlayTransform overlay_transform =
+      GetOverlayTransform(sqs->quad_to_target_transform, y_flipped);
   if (overlay_transform == gfx::OVERLAY_TRANSFORM_INVALID)
     return false;
 
-  auto& transform = quad->shared_quad_state->quad_to_target_transform;
+  auto& transform = sqs->quad_to_target_transform;
   candidate->display_rect = gfx::RectF(quad->rect);
   transform.TransformRect(&candidate->display_rect);
 
-  candidate->clip_rect = quad->shared_quad_state->clip_rect;
-  candidate->is_clipped = quad->shared_quad_state->is_clipped;
+  candidate->clip_rect = sqs->clip_rect;
+  candidate->is_clipped = sqs->is_clipped;
   candidate->is_opaque = !quad->ShouldDrawWithBlending();
   // For underlays the function 'EstimateVisibleDamage()' is called to update
   // |damage_area_estimate| to more accurately reflect the actual visible
@@ -333,7 +322,7 @@ bool OverlayCandidate::FromDrawQuadResource(
   candidate->resource_id = resource_id;
   candidate->transform = overlay_transform;
   candidate->mailbox = resource_provider->GetMailbox(resource_id);
-  candidate->requires_overlay = OverlayCandidate::RequiresOverlay(quad);
+
   return true;
 }
 
@@ -359,7 +348,6 @@ bool OverlayCandidate::FromVideoHoleQuad(
   // damage.
   candidate->damage_area_estimate =
       GetDamageRect(quad, surface_damage_rect_list).size().GetArea();
-  candidate->requires_overlay = OverlayCandidate::RequiresOverlay(quad);
   return true;
 }
 
@@ -375,6 +363,7 @@ bool OverlayCandidate::FromTextureQuad(
       (quad->background_color != SK_ColorBLACK ||
        quad->ShouldDrawWithBlending()))
     return false;
+
   if (!FromDrawQuadResource(resource_provider, surface_damage_rect_list, quad,
                             quad->resource_id(), quad->y_flipped, candidate)) {
     return false;
@@ -446,15 +435,15 @@ void OverlayCandidate::HandleClipAndSubsampling(OverlayCandidate* candidate) {
       candidate->uv_rect, candidate->resource_size_in_pixels.width(),
       candidate->resource_size_in_pixels.height());
   // Make it an integral multiple of the subsampling factor.
-  constexpr int kSubsamplingFactor = 2;
-  src_rect.set_x(kSubsamplingFactor *
-                 (std::lround(src_rect.x()) / kSubsamplingFactor));
-  src_rect.set_y(kSubsamplingFactor *
-                 (std::lround(src_rect.y()) / kSubsamplingFactor));
-  src_rect.set_width(kSubsamplingFactor *
-                     (std::lround(src_rect.width()) / kSubsamplingFactor));
-  src_rect.set_height(kSubsamplingFactor *
-                      (std::lround(src_rect.height()) / kSubsamplingFactor));
+  auto subsample_round = [](float val) {
+    constexpr int kSubsamplingFactor = 2;
+    return (std::lround(val) / kSubsamplingFactor) * kSubsamplingFactor;
+  };
+
+  src_rect.set_x(subsample_round(src_rect.x()));
+  src_rect.set_y(subsample_round(src_rect.y()));
+  src_rect.set_width(subsample_round(src_rect.width()));
+  src_rect.set_height(subsample_round(src_rect.height()));
   // Scale it back into UV space and set it in the candidate.
   candidate->uv_rect = gfx::ScaleRect(
       src_rect, 1.0f / candidate->resource_size_in_pixels.width(),

components/viz/service/display/overlay_candidate.h

@@ -81,9 +81,9 @@ class VIZ_SERVICE_EXPORT OverlayCandidate {
   ~OverlayCandidate();
 
   // Transformation to apply to layer during composition.
-  gfx::OverlayTransform transform;
+  gfx::OverlayTransform transform = gfx::OVERLAY_TRANSFORM_NONE;
   // Format of the buffer to scanout.
-  gfx::BufferFormat format;
+  gfx::BufferFormat format = gfx::BufferFormat::RGBA_8888;
   // ColorSpace of the buffer for scanout.
   gfx::ColorSpace color_space;
   // Size of the resource, in pixels.
@@ -92,15 +92,15 @@ class VIZ_SERVICE_EXPORT OverlayCandidate {
   // to integer coordinates if setting |overlay_handled| to true.
   gfx::RectF display_rect;
   // Crop within the buffer to be placed inside |display_rect|.
-  gfx::RectF uv_rect;
+  gfx::RectF uv_rect = gfx::RectF(0.f, 0.f, 1.f, 1.f);
   // Clip rect in the target content space after composition.
   gfx::Rect clip_rect;
   // If the quad is clipped after composition.
-  bool is_clipped;
+  bool is_clipped = false;
   // If the quad doesn't require blending.
-  bool is_opaque;
+  bool is_opaque = false;
   // Texture resource to present in an overlay.
-  unsigned resource_id;
+  unsigned resource_id = 0;
   // Mailbox from resource_id. It is used by SkiaRenderer.
   gpu::Mailbox mailbox;
 
@@ -108,11 +108,7 @@ class VIZ_SERVICE_EXPORT OverlayCandidate {
   // For candidates from StreamVideoDrawQuads, this records whether the quad is
   // marked as being backed by a SurfaceTexture or not.  If so, it's not really
   // promotable to an overlay.
-  bool is_backed_by_surface_texture;
-
-  // Filled in by the OverlayCandidateValidator to indicate whether this is a
-  // promotable candidate or not.
-  bool is_promotable_hint;
+  bool is_backed_by_surface_texture = false;
 #endif
 
   // Stacking order of the overlay plane relative to the main surface,
@@ -121,20 +117,20 @@ class VIZ_SERVICE_EXPORT OverlayCandidate {
 
   // To be modified by the implementer if this candidate can go into
   // an overlay.
-  bool overlay_handled;
+  bool overlay_handled = false;
 
   // Gpu fence to wait for before overlay is ready for display.
-  unsigned gpu_fence_id;
+  unsigned gpu_fence_id = 0;
 
   // The total area in square pixels of damage for this candidate's quad. This
   // is an estimate when 'EstimateOccludedDamage' function is used.
   int damage_area_estimate = 0;
 
-  // Result of call to 'RequiresOverlay' function w/ associated quad.
   static constexpr uint32_t kInvalidDamageIndex = UINT_MAX;
+  // Damage index for |SurfaceDamageRectList|.
   uint32_t overlay_damage_index = kInvalidDamageIndex;
 
-  // Cached result of call to 'RequiresOverlay' function.
+  // Is true if an HW overlay is required for the quad content.
   bool requires_overlay = false;
 
  private:

components/viz/service/display/overlay_candidate_temporal_tracker.cc

@@ -41,8 +41,9 @@ void OverlayCandidateTemporalTracker::AddRecord(
     uint64_t curr_frame,
     float damage_area_ratio,
     unsigned resource_id,
-    const OverlayCandidateTemporalTracker::Config& config) {
-  if (prev_resource_id != resource_id &&
+    const OverlayCandidateTemporalTracker::Config& config,
+    bool force_resource_update) {
+  if ((prev_resource_id != resource_id || force_resource_update) &&
       frame_record[(next_index + kNumRecords - 1) % kNumRecords] !=
           curr_frame) {
     frame_record[next_index] = curr_frame;

components/viz/service/display/overlay_candidate_temporal_tracker.h

@@ -55,10 +55,16 @@ class VIZ_SERVICE_EXPORT OverlayCandidateTemporalTracker {
 
   // This function adds a new record to the tracker if the |resource_id| has
   // changed since last update.
+  // The |force_resource_update| flag has been added for the case when the
+  // resource has been updated but the |resource_id| has not changed. The case
+  // for when this occurs is a low latency surface (ink). Fortunately, we can
+  // use surface damage to ascertain when these surfaces have changed despite
+  // the |resource_id| remaining constant.
   void AddRecord(uint64_t curr_frame,
                  float damage_area_ratio,
                  unsigned resource_id,
-                 const Config& config);
+                 const Config& config,
+                 bool force_resource_update = false);
 
   // This function returns true when this tracker's 'AddRecord' was not called
   // in the previous frame. We require this behavior in order to know when an

components/viz/service/display/overlay_processor_using_strategy.cc

@@ -16,6 +16,7 @@
 #include "components/viz/common/quads/solid_color_draw_quad.h"
 #include "components/viz/service/display/display_resource_provider.h"
 #include "components/viz/service/display/output_surface.h"
+#include "components/viz/service/display/overlay_candidate.h"
 #include "components/viz/service/display/overlay_strategy_single_on_top.h"
 #include "components/viz/service/display/overlay_strategy_underlay.h"
 #include "ui/gfx/geometry/rect_conversions.h"
@@ -285,7 +286,9 @@ void OverlayProcessorUsingStrategy::SortProposedOverlayCandidatesPrioritized(
     track_data.AddRecord(
         frame_sequence_number_,
         static_cast<float>(it->candidate.damage_area_estimate) / display_area,
-        it->candidate.resource_id, tracker_config_);
+        it->candidate.resource_id, tracker_config_,
+        it->candidate.overlay_damage_index !=
+            OverlayCandidate::kInvalidDamageIndex);
 
     // Here a series of criteria are considered for wholesale rejection of a
     // candidate. The rational for rejection is usually power improvements but
@@ -357,7 +360,6 @@ bool OverlayProcessorUsingStrategy::AttemptWithStrategiesPrioritized(
     std::vector<gfx::Rect>* content_bounds,
     gfx::Rect* incoming_damage) {
   last_successful_strategy_ = nullptr;
-
   Strategy::OverlayProposedCandidateList proposed_candidates;
   for (const auto& strategy : strategies_) {
     strategy->ProposePrioritized(
@@ -366,13 +368,20 @@ bool OverlayProcessorUsingStrategy::AttemptWithStrategiesPrioritized(
         &proposed_candidates, content_bounds);
   }
 
+  size_t num_proposed_pre_sort = proposed_candidates.size();
   UMA_HISTOGRAM_COUNTS_1000(
       "Viz.DisplayCompositor.OverlayNumProposedCandidates",
-      proposed_candidates.size());
+      num_proposed_pre_sort);
 
   SortProposedOverlayCandidatesPrioritized(&proposed_candidates);
 
   for (auto&& candidate : proposed_candidates) {
+    // Underlays change the material so we save it here to record proper UMA.
+    DrawQuad::Material quad_material =
+        candidate.strategy->GetUMAEnum() != OverlayStrategy::kUnknown
+            ? candidate.quad_iter->material
+            : DrawQuad::Material::kInvalid;
+
     if (candidate.strategy->AttemptPrioritized(
             output_color_matrix, render_pass_backdrop_filters,
             resource_provider, render_pass_list, surface_damage_rect_list,
@@ -383,12 +392,16 @@ bool OverlayProcessorUsingStrategy::AttemptWithStrategiesPrioritized(
       LogStrategyEnumUMA(candidate.strategy->GetUMAEnum());
       last_successful_strategy_ = candidate.strategy;
       OnOverlaySwitchUMA(ToProposeKey(candidate));
+      UMA_HISTOGRAM_ENUMERATION("Viz.DisplayCompositor.OverlayQuadMaterial",
+                                quad_material);
       return true;
     }
   }
 
   if (proposed_candidates.size() == 0) {
-    LogStrategyEnumUMA(OverlayStrategy::kNoStrategyUsed);
+    LogStrategyEnumUMA(num_proposed_pre_sort != 0
+                           ? OverlayStrategy::kNoStrategyFailMin
+                           : OverlayStrategy::kNoStrategyUsed);
   } else {
     LogStrategyEnumUMA(OverlayStrategy::kNoStrategyAllFail);
   }

components/viz/service/display/overlay_unittest.cc

@@ -2555,6 +2555,28 @@ TEST_F(UnderlayTest, OverlayCandidateTemporalTracker) {
   wait_1_frame();
   tracker.AddRecord(frame_counter, 0.0f, get_id(), config);
   EXPECT_FALSE(tracker.IsAbsent());
+
+  // Tracker forced updates were added to support quads that change content but
+  // not resource ids (example here is low latency ink surface). Here we test
+  // this small feature by keeping the resource id constant but passing in true
+  // to the force update param.
+  static const float kDamageRatio = 0.7f;
+  static const int kFakeConstantResourceId = 13;
+  for (int i = 0; i < OverlayCandidateTemporalTracker::kNumRecords; i++) {
+    wait_1_frame();
+    tracker.AddRecord(frame_counter, kDamageRatio, kFakeConstantResourceId,
+                      config, true);
+  }
+  EXPECT_FLOAT_EQ(kDamageRatio, tracker.MeanFrameRatioRate(config));
+
+  // Now test the false case for the force update param.
+  for (int i = 0; i < OverlayCandidateTemporalTracker::kNumRecords; i++) {
+    wait_1_frame();
+    tracker.AddRecord(frame_counter, 0.9f, kFakeConstantResourceId, config,
+                      false);
+  }
+  // The damage should remain unchanged.
+  EXPECT_FLOAT_EQ(kDamageRatio, tracker.MeanFrameRatioRate(config));
 }
 
 TEST_F(UnderlayTest, UpdateDamageRectWhenNoPromotion) {

tools/metrics/histograms/enums.xml

@@ -56311,6 +56311,21 @@ Called by update_net_trust_anchors.py.-->
   <int value="20" label="Tap Suppress"/>
 </enum>
 
+<enum name="OverlayQuadMaterial">
+  <int value="0" label="Invalid"/>
+  <int value="1" label="DebugBorder"/>
+  <int value="2" label="PictureContent"/>
+  <int value="3" label="CompositorRenderPass"/>
+  <int value="4" label="AggregatedRenderPass"/>
+  <int value="5" label="SolidColor"/>
+  <int value="6" label="StreamVideoContent"/>
+  <int value="7" label="SurfaceContent"/>
+  <int value="8" label="TextureContent"/>
+  <int value="9" label="TiledContent"/>
+  <int value="10" label="YuvVideoContent"/>
+  <int value="11" label="VideoHole"/>
+</enum>
+
 <enum name="OverlayStrategies">
   <int value="0" label="Unknown"/>
   <int value="1" label="No overlay"/>
@@ -56319,6 +56334,7 @@ Called by update_net_trust_anchors.py.-->
   <int value="4" label="Underlay"/>
   <int value="5" label="Underlay Cast"/>
   <int value="6" label="All overlays failed"/>
+  <int value="7" label="Overlays failed min"/>
 </enum>
 
 <enum name="OverlaySupportFlag">

tools/metrics/histograms/histograms_xml/others/histograms.xml

@@ -17046,6 +17046,16 @@ regressions. -->
   </summary>
 </histogram>
 
+<histogram name="Viz.DisplayCompositor.OverlayQuadMaterial"
+    enum="OverlayQuadMaterial" expires_after="2021-07-01">
+  <owner>petermcneeley@chromium.org</owner>
+  <owner>dcastagna@chromium.org</owner>
+  <summary>
+    Quad material for current promoted overlay, per frame. Recorded every time a
+    frame is rendered by the display compositor.
+  </summary>
+</histogram>
+
 <histogram name="Viz.DisplayCompositor.OverlayStrategy"
     enum="OverlayStrategies" expires_after="2021-07-01">
   <owner>dcastagna@chromium.org</owner>