MD - add ripples to DL shelf items

ui/views/animation/flood_fill_ink_drop_animation.cc

Index: ui/views/animation/flood_fill_ink_drop_animation.cc
diff --git a/ui/views/animation/flood_fill_ink_drop_animation.cc b/ui/views/animation/flood_fill_ink_drop_animation.cc
index 77e091e4e495e6e81961bff87257feeec3d10679..2de0c90eb4d4ebe27d0f0e43e5b6fad2543eadd0 100644
--- a/ui/views/animation/flood_fill_ink_drop_animation.cc
+++ b/ui/views/animation/flood_fill_ink_drop_animation.cc
@@ -18,7 +18,7 @@ namespace {
 
 // The minimum radius to use when scaling the painted layers. Smaller values
 // were causing visual anomalies.
-const float kMinRadius = 0.01f;
+const float kMinRadius = 1.f;

[Evan Stade, 2016/03/31 03:41:27]

this was the source of the problematic sizing (it was starting at 1x and
animating to Nx instead of starting at 0x and animating to Nx). It worked fine
if I changed this to 0.1f, but realistically there's no difference between
displaying a circle of size 1px vs 0.1px for a tiny fraction of a second. I
wanted to bump it to a higher number to avoid running into this problem again.

[bruthig, 2016/03/31 19:39:03]

Acknowledged.

 
 // All the sub animations that are used to animate each of the InkDropStates.
 // These are used to get time durations with
@@ -100,25 +100,6 @@ base::TimeDelta GetAnimationDuration(InkDropSubAnimations state) {
       kAnimationDurationInMs[state]);
 }
 
-// Calculates the largest distance from |point| to the corners of a rectangle
-// with origin (0, 0) and the given |size|.
-float CalculateLargestDistanceToCorners(const gfx::Size& size,

[Evan Stade, 2016/03/31 03:41:27]

This calculation is a bit overkill for any centerpoint-centered ripple as all
corners are equidistant from the centerpoint. It also didn't make sense for
click-centered ripples, because it would mean the max size of the ripple (hence
the speed with which the ripple expanded) would change based on the click
location. I think the speed should always be the same, hence the simplification
(the calculation is now inlined in the one place it's called). This would
theoretically affect bookmark buttons slightly, but it actually looks pretty
similar to me at full speed (perhaps due to tweening).

[bruthig, 2016/03/31 19:39:03]

Yes this is definitely overkill for ripples that are always View centered, it
was added purely for the event centered ripples.
You are correct and varkha@ and I debated about this same point.  Should it be
constant speed or constant time to fill the View surface.  We chose to go with
constant time for two main reasons.  

1. On a QUICK_ACTION we wanted the ripple to be fading out by the time it was
getting close to the boundaries.  If you fix the radius then you will see the
ripple fading out when you click near a corner, but if you click near the center
then the ripple hits the edges hard at full opacity before fading out. 

2. In regards to the BookmarkButtons we had to balance different sizes of
buttons.  We felt like using a constant speed caused the smallest of the
BookmarkButtons to fill too quickly with little sense of shape to the expanding
ripple.

We may want to consider using more dynamic ripple durations and have them be a
function of the View surface.  The download shelf buttons are much much larger
in comparison to the other ripple surfaces and I think the ripple might benefit
from using different timings.

Perhaps we should show both versions to the designers to see if they have any
strong opinions.

WDYT?

I will add varkha@ as he may also have input.

[Evan Stade, 2016/03/31 21:07:38]

With this change, the speed is not constant WRT the size of the view, it is
constant WRT the location of the click. It is still slower for smaller views and
faster for larger views.

I feel pretty confident it looks bad for the ripple speed to change as you mouse
around the view and click repeatedly in different places.

[bruthig, 2016/03/31 22:46:19]

I need to digest this a bit more.
Fair point, I will have to play with it a bit more.

Ultimately though, if we made the durations dynamically calculated then we would
be able to A. keep a constant speed and B. have the ripple fading out as it hits
the bounds of the View.  These are really my only two goals.

-                                        const gfx::Point& point) {
-  const float top_left_distance = gfx::Vector2dF(point.x(), point.y()).Length();
-  const float top_right_distance =
-      gfx::Vector2dF(size.width() - point.x(), point.y()).Length();
-  const float bottom_left_distance =
-      gfx::Vector2dF(point.x(), size.height() - point.y()).Length();
-  const float bottom_right_distance =
-      gfx::Vector2dF(size.width() - point.x(), size.height() - point.y())
-          .Length();
-
-  float largest_distance = std::max(top_left_distance, top_right_distance);
-  largest_distance = std::max(largest_distance, bottom_left_distance);
-  largest_distance = std::max(largest_distance, bottom_right_distance);
-  return largest_distance;
-}
-
 }  // namespace
 
 namespace views {
@@ -180,15 +161,10 @@ void FloodFillInkDropAnimation::AnimateStateChange(
     case InkDropState::HIDDEN:
       if (!IsVisible()) {
         SetStateToHidden();
-        break;
       } else {
         AnimateToOpacity(kHiddenOpacity, GetAnimationDuration(HIDDEN_FADE_OUT),
                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
                          gfx::Tween::EASE_IN_OUT, animation_observer);
-        const gfx::Transform transform = CalculateTransform(kMinRadius);
-        AnimateToTransform(transform, GetAnimationDuration(HIDDEN_TRANSFORM),

[Evan Stade, 2016/03/31 03:41:27]

I got rid of this because it looked weird on the shelf items and didn't match
polymer buttons[1], which fade while continuing to expand.

[1]
https://elements.polymer-project.org/elements/paper-button?view=demo:demo/ind...

[bruthig, 2016/03/31 19:39:03]

This was a very intentional choice and I feel like it is a rough edge in the
Polymer spec.  It's main purpose is to make the ripple visually distinct for
when a user aborts a pending action compared to when a QUICK_ACTION is
triggered.  Without it these two actions are indistinguishable.
(Oddly enough I'm having a tough time triggering it, I may have to investigate)

What looked weird about it? And was the button actually being triggered or
aborted?

[Evan Stade, 2016/03/31 21:07:38]

IIUC, you're saying that there's no visual distinction between click and click,
drag, release, which is a common way to cancel the action when the view is a
button. Yet traditional button press effects don't distinguish these two cases
either. They just switch between pressed and not-pressed.
To my knowledge, the analogy to real life is a drop of water falling into a
larger, placid body of water. The ripple expands outwards then fades away. It
never contracts or rewinds. The polymer buttons match this and look natural to
me (your opinion may differ). I also think it would be nicer if we supported
multiple simultaneous ripples.
triggered.

[bruthig, 2016/03/31 22:46:19]

Correct.  You can try this out on the "Home" button as well.  I was trying to
mimic that implosion effect for the FloodFill style here.
I still have the proof of concept for multiple ripples and definitely think it's
something we should entertain but unfortunately there have been more pressing
issues.
If the button action is triggered then the QUICK_ACTION animation should be
finishing off at a hidden opacity and there should be no visual effect from this
code.  It should essentially be a no-op.  The only time I expect there to be an
effect is in the 'cancel' situations.  This could be caused by the extra HIDDEN
trigger in DownloadItemViewMd::HandleClickEvent().

[Evan Stade, 2016/04/01 19:03:13]

ah ok. So if I change DownloadItemViewMd's sequence for click events from:
ACTION_PENDING, HIDDEN
to
ACTION_PENDING, QUICK_ACTION

the effect looks good

-                           ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
-                           gfx::Tween::EASE_IN_OUT, animation_observer);
       }
       break;
     case InkDropState::ACTION_PENDING: {
@@ -204,9 +180,7 @@ void FloodFillInkDropAnimation::AnimateStateChange(
                        ui::LayerAnimator::ENQUEUE_NEW_ANIMATION,
                        gfx::Tween::EASE_IN, animation_observer);
 
-      const gfx::Transform transform = CalculateTransform(
-          CalculateLargestDistanceToCorners(size_, center_point_));
-      AnimateToTransform(transform,
+      AnimateToTransform(GetActivatedTargetTransform(),
                          GetAnimationDuration(ACTION_PENDING_TRANSFORM),
                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
                          gfx::Tween::FAST_OUT_SLOW_IN, animation_observer);
@@ -231,9 +205,8 @@ void FloodFillInkDropAnimation::AnimateStateChange(
                        GetAnimationDuration(SLOW_ACTION_PENDING),
                        ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
                        gfx::Tween::EASE_IN, animation_observer);
-      const gfx::Transform transform = CalculateTransform(
-          CalculateLargestDistanceToCorners(size_, center_point_));
-      AnimateToTransform(transform, GetAnimationDuration(SLOW_ACTION_PENDING),
+      AnimateToTransform(GetActivatedTargetTransform(),
+                         GetAnimationDuration(SLOW_ACTION_PENDING),
                          ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET,
                          gfx::Tween::EASE_IN_OUT, animation_observer);
       break;
@@ -265,8 +238,7 @@ void FloodFillInkDropAnimation::AnimateStateChange(
 }
 
 void FloodFillInkDropAnimation::SetStateToHidden() {
-  gfx::Transform transform = CalculateTransform(kMinRadius);
-  painted_layer_.SetTransform(transform);
+  painted_layer_.SetTransform(CalculateTransform(kMinRadius));
   root_layer_.SetOpacity(InkDropAnimation::kHiddenOpacity);
   root_layer_.SetVisible(false);
 }
@@ -338,7 +310,7 @@ gfx::Transform FloodFillInkDropAnimation::CalculateTransform(
 
 gfx::Transform FloodFillInkDropAnimation::GetActivatedTargetTransform() const {
   return CalculateTransform(
-      CalculateLargestDistanceToCorners(size_, center_point_));
+      gfx::Vector2dF(size_.width(), size_.height()).Length());
 }
 
 }  // namespace views