Chromium Code Reviews Chromium Code Reviews
Issue
882443006:
Revert of Fix pointer-events:all when stroke="none" (Closed)
DescriptionRevert of Fix pointer-events:all when stroke="none" (patchset #6 id:100001 of https://codereview.chromium.org/810343003/)
Reason for revert:
Issue 452284: 10.1%-69.4% regression in blink_perf.svg at 312837:312949
Original issue's description:
> Fix pointer-events:all when stroke="none"
>
> pointer-events:all should make event processing unaffected by the values
> of the 'stroke' and 'fill' properties. However, when the 'stroke' of a
> shape was set to "none", hit detection would fail for points that would
> be in the shape's stroke area if the 'stroke' were not "none".
>
> Originally, two CLs were committed implementing a fix for the issue, but
> they were reverted due to a 200%-225% regression of a performance test:
> PerformanceTests/SVG/SvgHitTesting.html
> The performance regression was due to the change in strokeContains() that
> skipped the check whether the stroke bounding box contained the point if
> !requiresStroke.
>
> The modified fix implemented by this commit has three parts:
> 1. m_innerStrokeRect/m_outerStrokeRect are deleted from RenderSVGRect.
> This is because shapeDependentStrokeContains(), which was the only
> user of these FloatRect objects, should ignore whether the shape
> actually has a stroke, but m_innerStrokeRect and m_outerStrokeRect
> were equal to the m_fillBoundingBox if the <rect> had no stroke.
> 2. The notion of the hit test stroke bounding box is introduced and a
> new RenderSVGShape member is added: m_hitTestStrokeBoundingBox. The
> hit test stroke bounding box of a shape is considered to be the
> bounding box of the stroke, regardless of whether the shape has a
> stroke. The stroke bounding box is usually m_hitTestStrokeBoundingBox
> if the shape has a stroke; otherwise, it's m_fillBoundingBox. In the
> case of RenderSVGPath, though, the stroke bounding box includes the
> markerRect.
> 3. RenderSVGShape::strokeContains() was changed so that if !requiresStroke,
> then we check whether the point is within the hit test stroke bounding
> box. If requiresStroke, then we check whether the point is within the
> stroke bounding box.
>
> BUG=350338, 445410
>
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188888
TBR=pdr@chromium.org,fs@opera.com,ed@opera.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=350338, 445410, 452284
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189035
Patch Set 1 #
Created: 5 years, 11 months ago
(Patch set is too large to download)
Messages
Total messages: 8 (2 generated)
|
