New issue
Advanced search Search tips

Issue 652323 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 646176



Sign in to add a comment

Should we apply offset-path translation?

Project Member Reported by wangxianzhu@chromium.org, Oct 3 2016

Issue description

For test css3/motion-path/path-establishes-stacking-context.html:

<div id="div1" style="offset-path: path('M400,150'); width: 800px; height: 300px">
 ...
</div>

GeometryMapper and slow path generates different offsets from the LayoutView for div1.

The old path (mapToVisualRectInAncestorSpace) doesn't apply transform, and returns rect (8, 8, 800, 300).

PaintPropertyTreeBuilder creates a transform node (translate(400,150), result from ComputedStyle::applyTransform()) for div1, and then based on the property tree GeometryMapper returns rect (408, 158, 800, 300).

Our painting code seems to behave consistently with the old path, not applying transform to div1.

Which one is correct?
 
The center of the element should be placed at 400,150. This is equivalent to placing the top left corner at 0,0 (where it would be anyway without offset-path). 

 
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Available)
Thanks for the answer.

It seems that we are doing something wrong about the origin in PaintPropertyTreeBuilder. 

Comment 3 by suzyh@chromium.org, Oct 4 2016

Components: -Blink>Animation
Sounds like this is not an issue in the animations area, so removing Blink>Animation. Leaving Eric cc'd in case you need anything further from us.
Cc: pdr@chromium.org
The problem seems in ComputedStyle::applyTransform() when it is passed (..., ExcludeTransformOrigin, IncludeMotionPath, ...). ExcludeTransformOrigin will cause no origin (leaving default (0,0)) is calculated before motion path offset is applied, causing the motion offset is applied as a translation even if the offset is zero relative to the origin. ExcludeTransformOrigin should just avoid the origin from being applied as a translate in the transform,  not to treat the origin as (0,0).
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 4 2016

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

commit 59e73da2ef6b12fe29ffb65e2f7165f57d495118
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Tue Oct 04 22:31:14 2016

Fix wrong transform for motion-path

We call ComputedStyle::applyTransform(..., ExcludeTransformOrigin,
IncludeMotionPath, ...) in PaintPropertyTreeBuilder updateTransform().
Previously in the case applyTransform() passed wrong (0,0) origin
when calling applyMotionPathTransform().

Always pass the correct origin to applyMotionPathTransform().

BUG= 652323 
TEST=css3/motion-path/path-establishes-stacking-context.html with --enable-blink-features=slimmingPaintInvalidation

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

[modify] https://crrev.com/59e73da2ef6b12fe29ffb65e2f7165f57d495118/third_party/WebKit/Source/core/style/ComputedStyle.cpp

Status: Fixed (was: Assigned)

Sign in to add a comment