New issue
Advanced search Search tips

Issue 599933 link

Starred by 1 user
Status: Fixed
Owner:
senorblanco@chromium.org
Closed: Apr 2016
Cc:
robertphillips@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

For CSS reference filters, Blink should create Skia image filters with fully-specified crop rects

Project Member Reported by senorblanco@chromium.org, Apr 1 2016 
For CSS reference filters, Blink should create Skia image filters with fully-specified crop rects.

Currently, for filters which affect transparent black (e.g., lighting, color matrix with non-zero alpha translation), Skia fills the entire clip rect. However, Blink does not set a clip rect for reference filters (since
it's difficult to determine the correct value from a chain of reference and non-reference filters).

An easier solution is for Blink to set crop rects on all filters derived
from SVG nodes, since Blink already has this information. This will also enforce the default 10% margin required by SVG semantics.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 1 2016 

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

commit 259aaa35d268d314790739ecfd1111b06d07f429
Author: Stephen White <senorblanco@chromium.org>
Date: Fri Apr 01 18:58:03 2016

Suppress some layout test failures for upcoming Skia change.

BUG= 599933 
TBR=fmalita@chromium.org

Review URL: https://codereview.chromium.org/1854873002 .

Cr-Commit-Position: refs/heads/master@{#384651}

[modify] https://crrev.com/259aaa35d268d314790739ecfd1111b06d07f429/third_party/WebKit/LayoutTests/TestExpectations
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 1 2016 

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/6db0a7bdceb6be85721bfb0db8dea7fd27db5970

commit 6db0a7bdceb6be85721bfb0db8dea7fd27db5970
Author: senorblanco <senorblanco@chromium.org>
Date: Fri Apr 01 23:41:10 2016

Image filters: optimize crop rect application.

If a filter does not affect transparent black, there's no
reason to allow the crop rect to expand beyond the optimal
size determined by onFilterNodeBounds(). So don't enlarge
the bounds unless the filter affects transparent black.

In order to determine which filters affect transparent
black, I've pulled the inverse of the canComputeFastBounds()
logic into a non-recursive, affectsTransparentBlack()
virtual, and left canComputeFastBounds() as a non-virtual,
recursive function that calls it.

BUG= 599933 
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1848953002
TBR=reed@google.com

Review URL: https://codereview.chromium.org/1848953002

[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/include/core/SkImageFilter.h
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/include/effects/SkColorFilterImageFilter.h
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/include/effects/SkLightingImageFilter.h
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/include/effects/SkMatrixConvolutionImageFilter.h
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/include/effects/SkPaintImageFilter.h
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/src/core/SkImageFilter.cpp
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/src/effects/SkColorFilterImageFilter.cpp
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/src/effects/SkMatrixConvolutionImageFilter.cpp
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/src/effects/SkMergeImageFilter.cpp
[modify] https://crrev.com/6db0a7bdceb6be85721bfb0db8dea7fd27db5970/src/effects/SkPaintImageFilter.cpp
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 4 2016 

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

commit e67d10dc54276fa9af7d4bf12b3c8735449a4c07
Author: Stephen White <senorblanco@chromium.org>
Date: Mon Apr 04 17:00:08 2016

Blink: mark some css3/filters layout tests for rebaseline.

BUG= 599933 
TBR=fmalita@chromium.org

Review URL: https://codereview.chromium.org/1856833002 .

Cr-Commit-Position: refs/heads/master@{#384931}

[modify] https://crrev.com/e67d10dc54276fa9af7d4bf12b3c8735449a4c07/third_party/WebKit/LayoutTests/TestExpectations
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 4 2016 

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

commit a6b82d5ac158b95e5247c873eb41394acfd4c011
Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org>
Date: Mon Apr 04 18:58:19 2016

Auto-rebaseline for r384931

https://chromium.googlesource.com/chromium/src/+/e67d10dc5

BUG= 599933 
TBR=senorblanco@chromium.org

Review URL: https://codereview.chromium.org/1855963003 .

Cr-Commit-Position: refs/heads/master@{#384967}

[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-colorspace-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-ordering-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-zoom-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-colorspace-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-ordering-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-zoom-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-colorspace-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-ordering-hw-expected.png
[modify] https://crrev.com/a6b82d5ac158b95e5247c873eb41394acfd4c011/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-zoom-hw-expected.png

Comment 5 by senorblanco@chromium.org, Apr 4 2016

Cc: robertphillips@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 4 2016 

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

commit 0abb8cd84553174c24442b173f63110198145e81
Author: senorblanco <senorblanco@chromium.org>
Date: Mon Apr 04 22:32:56 2016

Use Blink's computed filterPrimitiveSubregion() as CropRect.

For filters which affect transparent black, we were falling back to Skia's clip rect to know which pixels to fill. This works for SVG, where we set the clip rect to the filter region. However, this is not possible for CSS filters on HTML content, where there may be multiple reference filters with different filter regions, so we don't actually set a clip rect at all.

The easiest fix is to set the crop rect unconditionally for all filters with a non-empty filter primitive subregion (aka all reference filters), which is what we do here. This also gets us closer to being able to removing the individual crop edges from Skia. This has minor pixel diffs in the affected tests.

BUG= 599933 

Review URL: https://codereview.chromium.org/1840303006

Cr-Commit-Position: refs/heads/master@{#385031}

[modify] https://crrev.com/0abb8cd84553174c24442b173f63110198145e81/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/0abb8cd84553174c24442b173f63110198145e81/third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp
[modify] https://crrev.com/0abb8cd84553174c24442b173f63110198145e81/third_party/WebKit/Source/platform/graphics/filters/FilterEffect.h
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 5 2016 

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

commit fdd9d2dd4cf25ded17857fad6960af3faf6c24c4
Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org>
Date: Tue Apr 05 00:48:22 2016

Auto-rebaseline for r385031

https://chromium.googlesource.com/chromium/src/+/0abb8cd84

BUG= 599933 
TBR=senorblanco@chromium.org

Review URL: https://codereview.chromium.org/1861443003 .

Cr-Commit-Position: refs/heads/master@{#385066}

[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-colorspace-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-ordering-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/linux/css3/filters/effect-reference-zoom-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-colorspace-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-ordering-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-zoom-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/mac/css3/filters/effect-reference-zoom-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-colorspace-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-ordering-hw-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-zoom-expected.png
[modify] https://crrev.com/fdd9d2dd4cf25ded17857fad6960af3faf6c24c4/third_party/WebKit/LayoutTests/platform/win/css3/filters/effect-reference-zoom-hw-expected.png
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 12 2016 

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

commit 8e52c44a881302e67c4d7d924bcbe69f603e1aa5
Author: senorblanco <senorblanco@chromium.org>
Date: Tue Apr 12 16:35:13 2016

Remove all use of crop edges from SkImageFilter::CropRect.

Since we're now always passing full crop rects to Skia (or nothing),
we don't need to use individual edge flags.  This required teaching
canvas filters to call determineFilterPrimitiveSubregion() in order to
ensure that the filter subregions are valid.

BUG= 599933 

Review URL: https://codereview.chromium.org/1862733002

Cr-Commit-Position: refs/heads/master@{#386709}

[modify] https://crrev.com/8e52c44a881302e67c4d7d924bcbe69f603e1aa5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/8e52c44a881302e67c4d7d924bcbe69f603e1aa5/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DState.cpp
[modify] https://crrev.com/8e52c44a881302e67c4d7d924bcbe69f603e1aa5/third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp

Comment 10 by senorblanco@chromium.org, Apr 28 2016 

This is essentially fixed. As of c#8 above, Chrome now always sets a crop rect for SVG filters, with either all edges set or none. We could change the second param of the CropRect constructor in Skia to take a bool, default true, and the Chrome code should work unchanged (modulo backwards compatibility support for older SKPs in Skia).

Comment 11 by senorblanco@chromium.org, Apr 28 2016

Status: Fixed (was: Assigned)

Comment 12 by suzyh@chromium.org, Apr 5 2017

Components: -Blink>CSS>Filters Blink>Compositing>Filters
►

Sign in to add a comment