Fix compositor ScrollTimeline handling of writing-modes

Previously we would get writing-modes/directionality completely
wrong for composited ScrollTimelines. This CL teaches cc::ScrollTimeline
about 'reverse' axes, without exposing the concepts of writing-mode or
directionality to it.

Bug: 906115
Change-Id: Id843668956b0766469148c1ae6ad33fca582ba63
Reviewed-on: https://chromium-review.googlesource.com/c/1310567
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608987}

cc/animation/animation_host_unittest.cc

@@ -328,8 +328,9 @@ TEST_F(AnimationHostTest, LayerTreeMutatorUpdateReflectsScrollAnimations) {
 
   // Create scroll timeline that links scroll animation and worklet animation
   // together. Use timerange so that we have 1:1 time & scroll mapping.
-  auto scroll_timeline = std::make_unique<ScrollTimeline>(
-      element_id, ScrollTimeline::Vertical, base::nullopt, base::nullopt, 100);
+  auto scroll_timeline =
+      std::make_unique<ScrollTimeline>(element_id, ScrollTimeline::ScrollDown,
+                                       base::nullopt, base::nullopt, 100);
 
   // Create a worklet animation that is bound to the scroll timeline.
   scoped_refptr<WorkletAnimation> worklet_animation(

cc/animation/scroll_timeline.cc

@@ -11,14 +11,26 @@
 
 namespace cc {
 
+namespace {
+bool IsVertical(ScrollTimeline::ScrollDirection direction) {
+  return direction == ScrollTimeline::ScrollUp ||
+         direction == ScrollTimeline::ScrollDown;
+}
+
+bool IsReverse(ScrollTimeline::ScrollDirection direction) {
+  return direction == ScrollTimeline::ScrollUp ||
+         direction == ScrollTimeline::ScrollLeft;
+}
+}  // namespace
+
 ScrollTimeline::ScrollTimeline(base::Optional<ElementId> scroller_id,
-                               ScrollDirection orientation,
+                               ScrollDirection direction,
                                base::Optional<double> start_scroll_offset,
                                base::Optional<double> end_scroll_offset,
                                double time_range)
     : active_id_(),
       pending_id_(scroller_id),
-      orientation_(orientation),
+      direction_(direction),
       start_scroll_offset_(start_scroll_offset),
       end_scroll_offset_(end_scroll_offset),
       time_range_(time_range) {}
@@ -26,7 +38,7 @@ ScrollTimeline::ScrollTimeline(base::Optional<ElementId> scroller_id,
 ScrollTimeline::~ScrollTimeline() {}
 
 std::unique_ptr<ScrollTimeline> ScrollTimeline::CreateImplInstance() const {
-  return std::make_unique<ScrollTimeline>(pending_id_, orientation_,
+  return std::make_unique<ScrollTimeline>(pending_id_, direction_,
                                           start_scroll_offset_,
                                           end_scroll_offset_, time_range_);
 }
@@ -58,11 +70,15 @@ double ScrollTimeline::CurrentTime(const ScrollTree& scroll_tree,
   gfx::ScrollOffset scroll_dimensions =
       scroll_tree.MaxScrollOffset(scroll_node->id);
 
-  double current_offset = (orientation_ == Vertical) ? offset.y() : offset.x();
-  double max_offset = (orientation_ == Vertical) ? scroll_dimensions.y()
-                                                 : scroll_dimensions.x();
-  DCHECK_GE(current_offset, 0);
+  double max_offset =
+      IsVertical(direction_) ? scroll_dimensions.y() : scroll_dimensions.x();
+  double current_physical_offset =
+      IsVertical(direction_) ? offset.y() : offset.x();
+  double current_offset = IsReverse(direction_)
+                              ? max_offset - current_physical_offset
+                              : current_physical_offset;
   DCHECK_GE(max_offset, 0);
+  DCHECK_GE(current_offset, 0);
 
   double resolved_start_scroll_offset = start_scroll_offset_.value_or(0);
   double resolved_end_scroll_offset = end_scroll_offset_.value_or(max_offset);

cc/animation/scroll_timeline.h

@@ -17,18 +17,18 @@ class ScrollTree;
 // progress of scrolling in some scroll container.
 //
 // This is the compositor-side representation of the web concept expressed in
-// https://wicg.github.io/scroll-animations/#scrolltimeline-interface. There are
-// differences between this class and the web definition of a ScrollTimeline.
-// For example the compositor does not know (or care) about 'writing modes', so
-// this class only tracks whether the ScrollTimeline orientation is horizontal
-// or vertical. Blink is expected to resolve any such 'web' requirements and
-// construct/update the compositor ScrollTimeline accordingly.
+// https://wicg.github.io/scroll-animations/#scrolltimeline-interface.
 class CC_ANIMATION_EXPORT ScrollTimeline {
  public:
-  enum ScrollDirection { Horizontal, Vertical };
+  enum ScrollDirection {
+    ScrollUp,
+    ScrollDown,
+    ScrollLeft,
+    ScrollRight,
+  };
 
   ScrollTimeline(base::Optional<ElementId> scroller_id,
-                 ScrollDirection orientation,
+                 ScrollDirection direction,
                  base::Optional<double> start_scroll_offset,
                  base::Optional<double> end_scroll_offset,
                  double time_range);
@@ -60,9 +60,9 @@ class CC_ANIMATION_EXPORT ScrollTimeline {
   base::Optional<ElementId> active_id_;
   base::Optional<ElementId> pending_id_;
 
-  // The orientation of the ScrollTimeline indicates which axis of the scroller
-  // it should base its current time on - either the horizontal or vertical.
-  ScrollDirection orientation_;
+  // The direction of the ScrollTimeline indicates which axis of the scroller
+  // it should base its current time on, and where the origin point is.
+  ScrollDirection direction_;
 
   base::Optional<double> start_scroll_offset_;
   base::Optional<double> end_scroll_offset_;

cc/animation/scroll_timeline_unittest.cc

@@ -98,9 +98,9 @@ TEST_F(ScrollTimelineTest, BasicCurrentTimeCalculations) {
   // just compute one edge and use it for vertical/horizontal.
   double time_range = content_size().height() - container_size().height();
 
-  ScrollTimeline vertical_timeline(scroller_id(), ScrollTimeline::Vertical,
+  ScrollTimeline vertical_timeline(scroller_id(), ScrollTimeline::ScrollDown,
                                    base::nullopt, base::nullopt, time_range);
-  ScrollTimeline horizontal_timeline(scroller_id(), ScrollTimeline::Horizontal,
+  ScrollTimeline horizontal_timeline(scroller_id(), ScrollTimeline::ScrollRight,
                                      base::nullopt, base::nullopt, time_range);
 
   // Unscrolled, both timelines should read a current time of 0.
@@ -120,7 +120,7 @@ TEST_F(ScrollTimelineTest, BasicCurrentTimeCalculations) {
 TEST_F(ScrollTimelineTest, CurrentTimeIsAdjustedForTimeRange) {
   // Here we set a time range to 100, which gives the current time the form of
   // 'percentage scrolled'.
-  ScrollTimeline timeline(scroller_id(), ScrollTimeline::Vertical,
+  ScrollTimeline timeline(scroller_id(), ScrollTimeline::ScrollDown,
                           base::nullopt, base::nullopt, 100);
 
   double halfwayY = (content_size().height() - container_size().height()) / 2.;
@@ -153,7 +153,7 @@ TEST_F(ScrollTimelineTest, ActiveTimeIsSetOnlyAfterPromotion) {
   double halfwayY = (content_size().height() - container_size().height()) / 2.;
   SetScrollOffset(&pending_tree, scroller_id, gfx::ScrollOffset(0, halfwayY));
 
-  ScrollTimeline main_timeline(scroller_id, ScrollTimeline::Vertical,
+  ScrollTimeline main_timeline(scroller_id, ScrollTimeline::ScrollDown,
                                base::nullopt, base::nullopt, 100);
 
   // Now create an impl version of the ScrollTimeline. Initilly this should only
@@ -180,7 +180,7 @@ TEST_F(ScrollTimelineTest, ActiveTimeIsSetOnlyAfterPromotion) {
 
 TEST_F(ScrollTimelineTest, CurrentTimeIsAdjustedForPixelSnapping) {
   double time_range = content_size().height() - container_size().height();
-  ScrollTimeline timeline(scroller_id(), ScrollTimeline::Vertical,
+  ScrollTimeline timeline(scroller_id(), ScrollTimeline::ScrollDown,
                           base::nullopt, base::nullopt, time_range);
 
   SetScrollOffset(&property_trees(), scroller_id(), gfx::ScrollOffset(0, 50));
@@ -197,7 +197,7 @@ TEST_F(ScrollTimelineTest, CurrentTimeIsAdjustedForPixelSnapping) {
 TEST_F(ScrollTimelineTest, CurrentTimeHandlesStartScrollOffset) {
   double time_range = content_size().height() - container_size().height();
   const double start_scroll_offset = 20;
-  ScrollTimeline timeline(scroller_id(), ScrollTimeline::Vertical,
+  ScrollTimeline timeline(scroller_id(), ScrollTimeline::ScrollDown,
                           start_scroll_offset, base::nullopt, time_range);
 
   // Unscrolled, the timeline should read a current time of unresolved, since
@@ -222,7 +222,7 @@ TEST_F(ScrollTimelineTest, CurrentTimeHandlesStartScrollOffset) {
 TEST_F(ScrollTimelineTest, CurrentTimeHandlesEndScrollOffset) {
   double time_range = content_size().height() - container_size().height();
   const double end_scroll_offset = time_range - 20;
-  ScrollTimeline timeline(scroller_id(), ScrollTimeline::Vertical,
+  ScrollTimeline timeline(scroller_id(), ScrollTimeline::ScrollDown,
                           base::nullopt, end_scroll_offset, time_range);
 
   SetScrollOffset(&property_trees(), scroller_id(),
@@ -249,7 +249,7 @@ TEST_F(ScrollTimelineTest, CurrentTimeHandlesCombinedStartAndEndScrollOffset) {
   double time_range = content_size().height() - container_size().height();
   double start_scroll_offset = 20;
   double end_scroll_offset = time_range - 50;
-  ScrollTimeline timeline(scroller_id(), ScrollTimeline::Vertical,
+  ScrollTimeline timeline(scroller_id(), ScrollTimeline::ScrollDown,
                           start_scroll_offset, end_scroll_offset, time_range);
   SetScrollOffset(&property_trees(), scroller_id(),
                   gfx::ScrollOffset(0, time_range - 150));
@@ -260,7 +260,7 @@ TEST_F(ScrollTimelineTest, CurrentTimeHandlesCombinedStartAndEndScrollOffset) {
 
 TEST_F(ScrollTimelineTest, CurrentTimeHandlesEqualStartAndEndScrollOffset) {
   double time_range = content_size().height() - container_size().height();
-  ScrollTimeline timeline(scroller_id(), ScrollTimeline::Vertical, 20, 20,
+  ScrollTimeline timeline(scroller_id(), ScrollTimeline::ScrollDown, 20, 20,
                           time_range);
   SetScrollOffset(&property_trees(), scroller_id(), gfx::ScrollOffset(0, 150));
   EXPECT_TRUE(std::isnan(timeline.CurrentTime(scroll_tree(), false)));
@@ -269,7 +269,7 @@ TEST_F(ScrollTimelineTest, CurrentTimeHandlesEqualStartAndEndScrollOffset) {
 TEST_F(ScrollTimelineTest,
        CurrentTimeHandlesStartOffsetLargerThanEndScrollOffset) {
   double time_range = content_size().height() - container_size().height();
-  ScrollTimeline timeline(scroller_id(), ScrollTimeline::Vertical, 50, 10,
+  ScrollTimeline timeline(scroller_id(), ScrollTimeline::ScrollDown, 50, 10,
                           time_range);
   SetScrollOffset(&property_trees(), scroller_id(), gfx::ScrollOffset(0, 150));
   EXPECT_TRUE(std::isnan(timeline.CurrentTime(scroll_tree(), false)));

cc/animation/worklet_animation_unittest.cc

@@ -50,7 +50,7 @@ class MockScrollTimeline : public ScrollTimeline {
  public:
   MockScrollTimeline()
       : ScrollTimeline(ElementId(),
-                       ScrollTimeline::Vertical,
+                       ScrollTimeline::ScrollDown,
                        base::nullopt,
                        base::nullopt,
                        0) {}

third_party/WebKit/LayoutTests/animations/animationworklet/resources/animation-worklet-tests.js

@@ -4,14 +4,22 @@ function runInAnimationWorklet(code) {
   );
 }
 
+function waitForAnimationFrames(count, callback) {
+  function rafCallback() {
+    if (count <= 0) {
+      callback();
+    } else {
+      count -= 1;
+      window.requestAnimationFrame(rafCallback);
+    }
+  }
+  rafCallback();
+};
+
 // Wait for two main thread frames to guarantee that compositor has produced
 // at least one frame.
-function waitTwoAnimationFrames(callback){
-  window.requestAnimationFrame( _=> {
-    window.requestAnimationFrame( _ => {
-      callback();
-    })
-  });
+function waitTwoAnimationFrames(callback) {
+  waitForAnimationFrames(2, callback);
 };
 
 // Load test cases in worklet context in sequence and wait until they are resolved.

third_party/WebKit/LayoutTests/animations/animationworklet/scroll-timeline-writing-modes.html

+<!DOCTYPE html>
+<title>Tests that ScrollTimeline works properly with writing mode and directionality</title>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+
+<script id="worklet_code"  type="text/worklet">
+registerAnimator("test_animator", class {
+  animate(currentTime, effect) {
+    effect.localTime = currentTime;
+  }
+});
+</script>
+
+<script src="resources/animation-worklet-tests.js"></script>
+<script>
+// Creates a DOM structure like:
+//   - container
+//     - box {100x100}
+//     - scroller {100x100, writing-mode, direction}
+//       - contents
+function createTestDOM(x_scroll_axis, writing_mode, direction) {
+  const elements = {};
+
+  elements.container = document.createElement('div');
+
+  elements.box = document.createElement('div');
+  elements.box.style.height = '100px';
+  elements.box.style.width = '100px';
+
+  elements.scroller = document.createElement('div');
+  elements.scroller.style.height = '100px';
+  elements.scroller.style.width = '100px';
+  if (x_scroll_axis)
+    elements.scroller.style.overflowX = 'scroll';
+  else
+    elements.scroller.style.overflowY = 'scroll';
+  elements.scroller.style.direction = direction;
+  elements.scroller.style.writingMode = writing_mode;
+
+  // Callers don't need access to this.
+  const contents = document.createElement('div');
+  contents.style.height = x_scroll_axis ? '100%' : '1000px';
+  contents.style.width = x_scroll_axis ? '1000px' : '100%';
+
+  elements.scroller.appendChild(contents);
+  elements.container.appendChild(elements.box);
+  elements.container.appendChild(elements.scroller);
+  document.body.appendChild(elements.container);
+
+  return elements;
+}
+
+function createAndPlayTestAnimation(elements, timeline_orientation) {
+  const effect = new KeyframeEffect(
+      elements.box,
+      [{transform: 'translateY(0)'}, {transform: 'translateY(200px)'}], {
+        duration: 1000,
+      });
+
+  const timeline = new ScrollTimeline({
+    scrollSource: elements.scroller,
+    timeRange: 1000,
+    orientation: timeline_orientation
+  });
+  const animation = new WorkletAnimation('test_animator', effect, timeline);
+  animation.play();
+}
+
+setup(setupAndRegisterTests, {explicit_done: true});
+
+function setupAndRegisterTests() {
+  const worklet_code = document.getElementById('worklet_code').textContent;
+  runInAnimationWorklet(worklet_code).then(() => {
+    // Note that block horizontal-tb is tested implicitly in the basic
+    // ScrollTimeline tests (as it is the default).
+    async_test(
+        block_vertical_lr,
+        'A block ScrollTimeline should produce the correct current time for vertical-lr');
+    async_test(
+        block_vertical_rl,
+        'A block ScrollTimeline should produce the correct current time for vertical-rl');
+    // Again, inline for horizontal-tb and direction: ltr is the default
+    // inline mode and so is tested elsewhere.
+    async_test(
+        inline_horizontal_tb_rtl,
+        'An inline ScrollTimeline should produce the correct current time for horizontal-tb and direction: rtl');
+    async_test(
+        inline_vertical_writing_mode_ltr,
+        'An inline ScrollTimeline should produce the correct current time for vertical writing mode');
+    async_test(
+        inline_vertical_writing_mode_rtl,
+        'An inline ScrollTimeline should produce the correct current time for vertical writing mode and direction: rtl');
+    done();
+  });
+}
+
+function block_vertical_lr(t) {
+  const elements = createTestDOM(true, 'vertical-lr', 'ltr');
+  createAndPlayTestAnimation(elements, 'block');
+
+  // Move the scroller to the 25% point.
+  const maxScroll =
+      elements.scroller.scrollWidth - elements.scroller.clientWidth;
+  elements.scroller.scrollLeft = 0.25 * maxScroll;
+
+  waitForAnimationFrames(3, t.step_func_done(() => {
+    assert_equals(
+        getComputedStyle(elements.box).transform, 'matrix(1, 0, 0, 1, 0, 50)');
+  }));
+}
+
+function block_vertical_rl(t) {
+  const elements = createTestDOM(true, 'vertical-rl', 'ltr');
+  createAndPlayTestAnimation(elements, 'block');
+
+  // Move the scroller to the 75% point. Since it is vertical-rl, this is
+  // equivalent to the 25% point for the ScrollTimeline.
+  const maxScroll =
+      elements.scroller.scrollWidth - elements.scroller.clientWidth;
+  elements.scroller.scrollLeft = 0.75 * maxScroll;
+
+  waitForAnimationFrames(3, t.step_func_done(() => {
+    assert_equals(
+        getComputedStyle(elements.box).transform, 'matrix(1, 0, 0, 1, 0, 50)');
+  }));
+}
+
+function inline_horizontal_tb_rtl(t) {
+  const elements = createTestDOM(true, 'horizontal-tb', 'rtl');
+  createAndPlayTestAnimation(elements, 'inline');
+
+  // Move the scroller to the 75% point. Since it is direction: rtl, this is
+  // equivalent to the 25% point for the ScrollTimeline.
+  const maxScroll =
+      elements.scroller.scrollWidth - elements.scroller.clientWidth;
+  elements.scroller.scrollLeft = 0.75 * maxScroll;
+
+  waitForAnimationFrames(3, t.step_func_done(() => {
+    assert_equals(
+        getComputedStyle(elements.box).transform, 'matrix(1, 0, 0, 1, 0, 50)');
+  }));
+}
+
+function inline_vertical_writing_mode_ltr(t) {
+  const elements = createTestDOM(false, 'vertical-lr', 'ltr');
+  createAndPlayTestAnimation(elements, 'inline');
+
+  // Move the scroller to the 25% point.
+  const maxScroll =
+      elements.scroller.scrollHeight - elements.scroller.clientHeight;
+  elements.scroller.scrollTop = 0.25 * maxScroll;
+
+  waitForAnimationFrames(3, t.step_func_done(() => {
+    assert_equals(
+        getComputedStyle(elements.box).transform, 'matrix(1, 0, 0, 1, 0, 50)');
+  }));
+}
+
+function inline_vertical_writing_mode_rtl(t) {
+  const elements = createTestDOM(false, 'vertical-lr', 'rtl');
+  createAndPlayTestAnimation(elements, 'inline');
+
+  // Move the scroller to the 75% point. Since this is a vertical writing mode
+  // and direction: rtl, this is 25% of the ScrollTimeline currentTime.
+  const maxScroll =
+      elements.scroller.scrollHeight - elements.scroller.clientHeight;
+  elements.scroller.scrollTop = 0.75 * maxScroll;
+
+  waitForAnimationFrames(3, t.step_func_done(() => {
+    assert_equals(
+        getComputedStyle(elements.box).transform, 'matrix(1, 0, 0, 1, 0, 50)');
+  }));
+}
+</script>

third_party/blink/renderer/modules/animationworklet/worklet_animation.cc

@@ -136,26 +136,56 @@ base::Optional<CompositorElementId> GetCompositorScrollElementId(
 // web concepts of 'block' and 'inline' direction into absolute vertical or
 // horizontal directions.
 //
+// This implements a subset of the conversions documented in
+// https://drafts.csswg.org/css-writing-modes-3/#logical-to-physical
+//
 // TODO(smcgruer): If the writing mode of a scroller changes, we have to update
 // any related cc::ScrollTimeline somehow.
 CompositorScrollTimeline::ScrollDirection ConvertOrientation(
     ScrollTimeline::ScrollDirection orientation,
-    bool is_horizontal_writing_mode) {
-  switch (orientation) {
-    case ScrollTimeline::Block:
-      return is_horizontal_writing_mode ? CompositorScrollTimeline::Vertical
-                                        : CompositorScrollTimeline::Horizontal;
-    case ScrollTimeline::Inline:
-      return is_horizontal_writing_mode ? CompositorScrollTimeline::Horizontal
-                                        : CompositorScrollTimeline::Vertical;
-    case ScrollTimeline::Horizontal:
-      return CompositorScrollTimeline::Horizontal;
-    case ScrollTimeline::Vertical:
-      return CompositorScrollTimeline::Vertical;
-    default:
-      NOTREACHED();
-      return CompositorScrollTimeline::Vertical;
+    const ComputedStyle* style) {
+  // Easy cases; physical is always physical.
+  if (orientation == ScrollTimeline::Horizontal)
+    return CompositorScrollTimeline::ScrollRight;
+  if (orientation == ScrollTimeline::Vertical)
+    return CompositorScrollTimeline::ScrollDown;
+
+  // Harder cases; first work out which axis is which, and then for each check
+  // which edge we start at.
+
+  // writing-mode: horizontal-tb
+  bool is_horizontal_writing_mode =
+      style ? style->IsHorizontalWritingMode() : true;
+  // writing-mode: vertical-lr
+  bool is_flipped_lines_writing_mode =
+      style ? style->IsFlippedLinesWritingMode() : false;
+  // direction: ltr;
+  bool is_ltr_direction = style ? style->IsLeftToRightDirection() : true;
+
+  if (orientation == ScrollTimeline::Block) {
+    if (is_horizontal_writing_mode) {
+      // For horizontal writing mode, block is vertical. The starting edge is
+      // always the top.
+      return CompositorScrollTimeline::ScrollDown;
+    }
+    // For vertical writing mode, the block axis is horizontal. The starting
+    // edge depends on if we are lr or rl.
+    return is_flipped_lines_writing_mode ? CompositorScrollTimeline::ScrollRight
+                                         : CompositorScrollTimeline::ScrollLeft;
+  }
+
+  DCHECK_EQ(orientation, ScrollTimeline::Inline);
+  if (is_horizontal_writing_mode) {
+    // For horizontal writing mode, inline is horizontal. The starting edge
+    // depends on the directionality.
+    return is_ltr_direction ? CompositorScrollTimeline::ScrollRight
+                            : CompositorScrollTimeline::ScrollLeft;
   }
+  // For vertical writing mode, inline is vertical. The starting edge still
+  // depends on the directionality; whether it is vertical-lr or vertical-rl
+  // does not matter.
+  return is_ltr_direction ? CompositorScrollTimeline::ScrollDown
+                          : CompositorScrollTimeline::ScrollUp;
 }
 
 // Converts a blink::ScrollTimeline into a cc::ScrollTimeline.
@@ -181,9 +211,8 @@ std::unique_ptr<CompositorScrollTimeline> ToCompositorScrollTimeline(
   // correct orientation and start/end offset information.
   LayoutBox* box = scroll_source->GetLayoutBox();
 
-  CompositorScrollTimeline::ScrollDirection orientation =
-      ConvertOrientation(scroll_timeline->GetOrientation(),
-                         box ? box->IsHorizontalWritingMode() : true);
+  CompositorScrollTimeline::ScrollDirection orientation = ConvertOrientation(
+      scroll_timeline->GetOrientation(), box ? box->Style() : nullptr);
 
   base::Optional<double> start_scroll_offset;
   base::Optional<double> end_scroll_offset;