Paint buffered range closest to the current play position

Source/core/layout/LayoutMediaControls.cpp

 /*
  * Copyright (C) 2009 Apple Inc.
  * Copyright (C) 2009 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.
@@ skipping 189 matching lines @@
         return false;
 
     const LayoutStyle& style = object->styleRef();
     GraphicsContext* context = paintInfo.context;
 
     paintRoundedSliderBackground(rect, style, context);
 
     // Draw the buffered range. Since the element may have multiple buffered ranges and it'd be
     // distracting/'busy' to show all of them, show only the buffered range containing the current play head.
     RefPtrWillBeRawPtr<TimeRanges> bufferedTimeRanges = mediaElement->buffered();
-    float duration = mediaElement->duration();
-    float currentTime = mediaElement->currentTime();
-    if (std::isnan(duration) || std::isinf(duration) || !duration || std::isnan(currentTime))
+    double duration = mediaElement->duration();
+    double currentTime = mediaElement->currentTime();
+    if (std::isnan(duration) || std::isinf(duration) || !duration || std::isnan(currentTime)
+        || bufferedTimeRanges->length() == 0)
         return true;
 
-    for (unsigned i = 0; i < bufferedTimeRanges->length(); ++i) {
-        float start = bufferedTimeRanges->start(i, ASSERT_NO_EXCEPTION);
-        float end = bufferedTimeRanges->end(i, ASSERT_NO_EXCEPTION);
-        if (std::isnan(start) || std::isnan(end) || start > currentTime || end < currentTime)

[philipj_slow, 2015/03/06 03:16:17]

Could we fix the whole problem by tweaking this to use "start > currentTime +
some delta" instead? The delta could be a new
HTMLMediaElement::cachedTimeDelta() implemented as m_cachedTime -
webMediaPlayer()->currentTime(), which could find use in
HTMLMediaElement::currentTime() as well.

[landell, 2015/03/06 13:49:06]

I can look at such a solution. Don't know if the special handling for seeking
and default playback position might get in the way though.

-            continue;
-        int startPosition = int(start * rect.width() / duration);
-        int currentPosition = int(currentTime * rect.width() / duration);
-        int endPosition = int(end * rect.width() / duration);
+    unsigned nearestIdx = bufferedTimeRanges->nearestRange(currentTime);

[philipj_slow, 2015/03/06 03:16:17]

So I was thinking that we could have a getter for the nearest range, but the
ranges themselves aren't objects at all. This API ends up a bit cumbersome I
think.

[landell, 2015/03/06 13:49:06]

I agree. To me the problem is that the TimeRanges interface is kind of weird and
this was an attempt to play nicely with it. :)
I could change it to return a TimeRanges::Range instead if you think it is
better.

+    double start = bufferedTimeRanges->start(nearestIdx, ASSERT_NO_EXCEPTION);
+    double end = bufferedTimeRanges->end(nearestIdx, ASSERT_NO_EXCEPTION);
+    if (std::isnan(start) || std::isnan(end)) {
+        return true;
+    }
 
-        // Add half the thumb width proportionally adjusted to the current painting position.
-        int thumbCenter = mediaSliderThumbWidth / 2;
-        int addWidth = thumbCenter * (1.0 - 2.0 * currentPosition / rect.width());
-        currentPosition += addWidth;
+    int startPosition = int(start * rect.width() / duration);
+    int currentPosition = int(currentTime * rect.width() / duration);
+    int endPosition = int(end * rect.width() / duration);
+
+    // Add half the thumb width proportionally adjusted to the current painting position.
+    int thumbCenter = mediaSliderThumbWidth / 2;
+    int addWidth = thumbCenter * (1.0 - 2.0 * currentPosition / rect.width());
+    currentPosition += addWidth;
+
+    // Only paint ranges that will intersect with the slider thumb

[philipj_slow, 2015/03/06 03:16:17]

I see no explicit delta, so I guess the resolution of the slider is what serves
that role? Couldn't also break in edge cases just like today, where the
alignment of values is just unfortunate?

[landell, 2015/03/06 13:49:06]

My thinking was that it made sense to make the delta condition based on
visibility, i.e. we avoid painting ranges that would introduce gaps between the
thumb and range. It's kind of artificial though, since painting will use the
currentPosition at one end anyway. As you say, there are some corner cases here
but I only think it would be significant for short streams, assuming the diff is
small.

I could add some timeline based delta here or maybe switch to it entirely. This
solution will always have corner cases though but a delta based on timestamps is
probably a bit more robust.

+    if (currentPosition <= endPosition + thumbCenter
+        || currentPosition >= startPosition - thumbCenter) {
 
         // Draw white-ish highlight before current time.
         Color startColor = Color(195, 195, 195);
         Color endColor = Color(217, 217, 217);
         if (currentPosition > startPosition)
             paintSliderRangeHighlight(rect, style, context, startPosition, currentPosition, startColor, endColor);
 
         // Draw grey-ish highlight after current time.
         startColor = Color(60, 60, 60);
         endColor = Color(76, 76, 76);
 
         if (endPosition > currentPosition)
             paintSliderRangeHighlight(rect, style, context, currentPosition, endPosition, startColor, endColor);
-
-        return true;
     }
 
     return true;
 }
 
 static bool paintMediaSliderThumb(LayoutObject* object, const PaintInfo& paintInfo, const IntRect& rect)
 {
     if (!object->node())
         return false;
 
@@ skipping 213 matching lines @@
 {
     return formatChromiumMediaControlsTime(time, time);
 }
 
 String LayoutMediaControls::formatMediaControlsCurrentTime(float currentTime, float duration)
 {
     return formatChromiumMediaControlsTime(currentTime, duration);
 }
 
 } // namespace blink