New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 612573 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 1
Type: Feature

Blocking:
issue 602756
issue 612544
issue 612567
issue 612579
issue 631516



Sign in to add a comment

Add the ability for the material design flood fill animation to clip to arbitrary bounds

Project Member Reported by tdander...@chromium.org, May 17 2016

Issue description

Add the ability for the material design flood fill animation to clip to arbitrary bounds, e.g., for buttons with circular or rounded rectangle bounds.

 
Cc: -bruthig@chromium.org
Owner: bruthig@chromium.org
Status: Assigned (was: Available)
Ben, I'm handing this to you for the time being. If you won't have time for this can you instead share some thoughts on the design?
Blocking: 612579
Blocking: 612544
My recollection is that this is P1 for CrOS only, and not critical for the other platforms right now.  Ben, can you update this with the state of the world and how this affects our various platforms?
The list of surfaces that require this are as follows:

- Bookmarks buttons (Windows, CrOS, Linux)
- AppListButton (CrOS)
- Shelf overflow button (CrOS)
- Status area buttons, e.g. VKB, notifications, etc (CrOS)

It is currently unclear if the potential solution for this will work on Windows (There is a 4+ year old TODO that says it's not, but this is clearly really old https://cs.chromium.org/chromium/src/ui/views/bubble/tray_bubble_view.cc?rcl=0&l=177).

Since Ash MD will NOT be turned on in M53 the only question is whether the Bookmarks button is a P-1 for M53 or whether we can re-target this for M-54.
Labels: -M-53 M-54
Peter has marked  issue 602756  (for bookmark bar buttons) as a P3, so let's re-target this for M-54.
Cc: -moh...@chromium.org bruthig@chromium.org
Owner: moh...@chromium.org
Handing off to Mohsen.

For reference check out usage of ui::Layer::SetMaskLayer(). (See [1] below)

Some non-trivial things that we will need to consider:
- The mask should apply to the hover and the ripple
- The mask bounds should get updated as the hosting View's bounds are changed.  Not sure about the implications of animated bound changes. e.g. appearing/disappearing TrayItems animate the bounds changes of the system tray.
- Are there any performance considerations (e.g. mask Layers are not tiled)



[1] https://cs.chromium.org/chromium/src/ui/compositor/layer.h?l=230&gs=cpp%253Aui%253A%253Aclass-Layer%253A%253ASetMaskLayer(ui%253A%253ALayer%2B*)%2540chromium%252F..%252F..%252Fui%252Fcompositor%252Flayer.h%257Cdecl&gsn=SetMaskLayer&ct=xref_usages

Comment 8 by moh...@chromium.org, Aug 25 2016

Status: Started (was: Assigned)
Labels: -M-54 M-55
Components: UI>Shell>Shelf
Labels: -M-55 M-56
Blocking: 631516
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/932af7361a3588b36a1f15aa76b6199a4a66f646

commit 932af7361a3588b36a1f15aa76b6199a4a66f646
Author: mohsen <mohsen@chromium.org>
Date: Mon Nov 14 23:36:17 2016

Add ink drop masking to TrayBackgroundView

A virtual CreateInkDropMask() is added to InkDropHostView that
subclasses can override and provide their choice of mask for the ink
drop. TrayBackgroundView has implemented this to create a rounded
rectangle mask.

A few masks types are declared in ui/views/animation/ink_drop_mask.h
file.

BUG= 612573 
TEST=manually run TouchView Ash with --ash-md=experimental and try
     ripples on overview mode button.

Review-Url: https://codereview.chromium.org/2499523003
Cr-Commit-Position: refs/heads/master@{#431972}

[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ash/common/system/tray/tray_background_view.cc
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ash/common/system/tray/tray_background_view.h
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/BUILD.gn
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop.h
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_host_view.cc
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_host_view.h
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_impl.cc
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_impl.h
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_impl_unittest.cc
[add] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_mask.cc
[add] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_mask.h
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_stub.cc
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_stub.h
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/ink_drop_unittest.cc
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/test/test_ink_drop.cc
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/animation/test/test_ink_drop.h
[modify] https://crrev.com/932af7361a3588b36a1f15aa76b6199a4a66f646/ui/views/controls/button/custom_button_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/431883e99a2fcdc5e6e54868ac98d3474aaaf198

commit 431883e99a2fcdc5e6e54868ac98d3474aaaf198
Author: mohsen <mohsen@chromium.org>
Date: Thu Nov 17 17:00:52 2016

Take size in ink drop masks instead of bounds

Currently, constructors for ink drop masks, take layer bounds which is
expected to have (0, 0) as their origin. It's better for them to take
size instead.

BUG= 612573 
TEST=none

Review-Url: https://codereview.chromium.org/2508063004
Cr-Commit-Position: refs/heads/master@{#432898}

[modify] https://crrev.com/431883e99a2fcdc5e6e54868ac98d3474aaaf198/ash/common/shelf/app_list_button.cc
[modify] https://crrev.com/431883e99a2fcdc5e6e54868ac98d3474aaaf198/ash/common/system/tray/tray_background_view.cc
[modify] https://crrev.com/431883e99a2fcdc5e6e54868ac98d3474aaaf198/ui/views/animation/ink_drop_mask.cc
[modify] https://crrev.com/431883e99a2fcdc5e6e54868ac98d3474aaaf198/ui/views/animation/ink_drop_mask.h

Project Member

Comment 16 by bugdroid1@chromium.org, Nov 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/805a5e5f29072e481bb9cd245751d39da874a10b

commit 805a5e5f29072e481bb9cd245751d39da874a10b
Author: estade <estade@chromium.org>
Date: Thu Nov 17 17:23:57 2016

Revert of Take size in ink drop masks instead of bounds (patchset #1 id:1 of https://codereview.chromium.org/2508063004/ )

Reason for revert:
compile failure on Windows - https://build.chromium.org/p/chromium.win/builders/Win%20Builder/builds/32175/steps/compile/logs/stdio

'views::CircleInkDropMask::CircleInkDropMask(const views::CircleInkDropMask &)': cannot convert argument 1 from 'const gfx::Rect' to 'const gfx::Size &'

Original issue's description:
> Take size in ink drop masks instead of bounds
>
> Currently, constructors for ink drop masks, take layer bounds which is
> expected to have (0, 0) as their origin. It's better for them to take
> size instead.
>
> BUG= 612573 
> TEST=none
>
> Committed: https://crrev.com/431883e99a2fcdc5e6e54868ac98d3474aaaf198
> Cr-Commit-Position: refs/heads/master@{#432898}

TBR=bruthig@chromium.org,jamescook@chromium.org,mohsen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 612573 

Review-Url: https://codereview.chromium.org/2510633004
Cr-Commit-Position: refs/heads/master@{#432899}

[modify] https://crrev.com/805a5e5f29072e481bb9cd245751d39da874a10b/ash/common/shelf/app_list_button.cc
[modify] https://crrev.com/805a5e5f29072e481bb9cd245751d39da874a10b/ash/common/system/tray/tray_background_view.cc
[modify] https://crrev.com/805a5e5f29072e481bb9cd245751d39da874a10b/ui/views/animation/ink_drop_mask.cc
[modify] https://crrev.com/805a5e5f29072e481bb9cd245751d39da874a10b/ui/views/animation/ink_drop_mask.h

Project Member

Comment 17 by bugdroid1@chromium.org, Nov 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c8b9453199a880668feab3a3f1bf39ca603a447

commit 7c8b9453199a880668feab3a3f1bf39ca603a447
Author: mohsen <mohsen@chromium.org>
Date: Thu Nov 17 21:42:16 2016

Reland: Take size in ink drop masks instead of bounds

Currently, constructors for ink drop masks, take layer bounds which is
expected to have (0, 0) as their origin. It's better for them to take
size instead.

BUG= 612573 
TEST=none

Committed: https://crrev.com/431883e99a2fcdc5e6e54868ac98d3474aaaf198
Review-Url: https://codereview.chromium.org/2508063004
Cr-Original-Commit-Position: refs/heads/master@{#432898}
Cr-Commit-Position: refs/heads/master@{#432971}

[modify] https://crrev.com/7c8b9453199a880668feab3a3f1bf39ca603a447/ash/common/shelf/app_list_button.cc
[modify] https://crrev.com/7c8b9453199a880668feab3a3f1bf39ca603a447/ash/common/system/tray/tray_background_view.cc
[modify] https://crrev.com/7c8b9453199a880668feab3a3f1bf39ca603a447/ash/common/system/tray/tray_popup_utils.cc
[modify] https://crrev.com/7c8b9453199a880668feab3a3f1bf39ca603a447/ui/views/animation/ink_drop_mask.cc
[modify] https://crrev.com/7c8b9453199a880668feab3a3f1bf39ca603a447/ui/views/animation/ink_drop_mask.h

Status: Verified (was: Fixed)

Sign in to add a comment