Replace FilterEffect::maxEffectRect() with absoluteBounds()

Replace FilterEffect::maxEffectRect() with absoluteBounds()

This removes maxEffectRect(), determineMaximumEffectRect() and the
associated state (m_maxEffectRect) from FilterEffect. The replacement is
the new method absoluteBounds() that computes bounds for the node in
question using the filter primitive (sub)region and the filter region.
This should match what determineMaximumEffectRect() was computing for
use from SVGFilterPainter.

BUG=642035
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/61354a46c5d80d8d8450ef06599d7b41f617e35b
Cr-Commit-Position: refs/heads/master@{#417530}

[fs, 2016-09-08 16:01:42]

Description was changed from

==========
Replace FilterEffect::maxEffectRect() with absoluteBounds()

This removes maxEffectRect(), determineMaximumEffectRect() and the
associated state (m_maxEffectRect) from FilterEffect. The replacement is
the new method absoluteBounds() that computes bounds for the node in
question using the filter primitive (sub)region and the filter region.
This should match what determineMaximumEffectRect() was computing for
use from SVGFilterPainter.

BUG=642035
==========

to

==========
Replace FilterEffect::maxEffectRect() with absoluteBounds()

This removes maxEffectRect(), determineMaximumEffectRect() and the
associated state (m_maxEffectRect) from FilterEffect. The replacement is
the new method absoluteBounds() that computes bounds for the node in
question using the filter primitive (sub)region and the filter region.
This should match what determineMaximumEffectRect() was computing for
use from SVGFilterPainter.

BUG=642035
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[fs, 2016-09-08 16:01:49]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 16:02:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 17:50:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-08 17:50:38]

Dry run: This issue passed the CQ dry run.

[fs, 2016-09-08 18:17:11]

fs@opera.com changed reviewers:
+ pdr@chromium.org

[pdr., 2016-09-08 20:46:12]

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/graphics/filters/SVGFEImage.cpp (right):

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/graphics/filters/SVGFEImage.cpp:100: if
(clipsToBounds())
Do you know what these clipsToBounds calls are doing? Removing them has no
effect.

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp
(right):

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp:53:
FloatRect computedBounds = getFilter()->filterRegion();
Should we clip/intersect with the filter region here or in
determineAbsolutePaintRect?

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp:63: if
(clipsToBounds())
The comment above m_clipsToBounds makes me think this should be after the union
of all input paint rects, below.

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/filters/FilterEffect.h (right):

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/filters/FilterEffect.h:69: FloatRect
absoluteBounds() const;
Can you add a comment here? Maybe something about being the filter primitive
subregion clipped and in the space of the filtered element.

[fs, 2016-09-08 21:07:20]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 21:07:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2016-09-08 21:08:12]

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/graphics/filters/SVGFEImage.cpp (right):

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/graphics/filters/SVGFEImage.cpp:100: if
(clipsToBounds())
On 2016/09/08 at 20:46:12, pdr. wrote:
> Do you know what these clipsToBounds calls are doing? Removing them has no
effect.

The reason that they have no effect is because they are always true in this
code-path (and all the others where it's used.) In a near future that will no
longer be true though, but each removal has its time =)

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp
(right):

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp:53:
FloatRect computedBounds = getFilter()->filterRegion();
On 2016/09/08 at 20:46:12, pdr. wrote:
> Should we clip/intersect with the filter region here or in
determineAbsolutePaintRect?

For the same reason stated in SVGFEImage, it doesn't really matter at this point
in time. Eventually this method (and its users) will need to morph though.

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp:63: if
(clipsToBounds())
On 2016/09/08 at 20:46:12, pdr. wrote:
> The comment above m_clipsToBounds makes me think this should be after the
union of all input paint rects, below.

Yes, that probably makes more sense. The way things are now we essentially limit
both input rect and output rect though (see the if-else at the end.)

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/filters/FilterEffect.h (right):

https://codereview.chromium.org/2319293004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/filters/FilterEffect.h:69: FloatRect
absoluteBounds() const;
On 2016/09/08 at 20:46:12, pdr. wrote:
> Can you add a comment here? Maybe something about being the filter primitive
subregion clipped and in the space of the filtered element.

Done.

[pdr., 2016-09-08 21:09:37]

Thanks, LGTM!

[commit-bot: I haz the power, 2016-09-08 23:26:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-08 23:26:17]

Dry run: This issue passed the CQ dry run.

[fs, 2016-09-09 07:47:18]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-09-09 07:47:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 07:51:04]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-09 07:54:42]

Description was changed from

==========
Replace FilterEffect::maxEffectRect() with absoluteBounds()

This removes maxEffectRect(), determineMaximumEffectRect() and the
associated state (m_maxEffectRect) from FilterEffect. The replacement is
the new method absoluteBounds() that computes bounds for the node in
question using the filter primitive (sub)region and the filter region.
This should match what determineMaximumEffectRect() was computing for
use from SVGFilterPainter.

BUG=642035
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Replace FilterEffect::maxEffectRect() with absoluteBounds()

This removes maxEffectRect(), determineMaximumEffectRect() and the
associated state (m_maxEffectRect) from FilterEffect. The replacement is
the new method absoluteBounds() that computes bounds for the node in
question using the filter primitive (sub)region and the filter region.
This should match what determineMaximumEffectRect() was computing for
use from SVGFilterPainter.

BUG=642035
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/61354a46c5d80d8d8450ef06599d7b41f617e35b
Cr-Commit-Position: refs/heads/master@{#417530}
==========

[commit-bot: I haz the power, 2016-09-09 07:54:43]

Patchset 2 (id:??) landed as
https://crrev.com/61354a46c5d80d8d8450ef06599d7b41f617e35b
Cr-Commit-Position: refs/heads/master@{#417530}