Consider 'order' when updating feConvolveMatrix 'target*'

Consider 'order' when updating feConvolveMatrix 'target*'

SVGFEConvolveMatrixElement's 'targetX' or 'targetY' attribute depend on
'order' for their initial value. When updating the target value of
an instantiated FEConvolveMatrix, order would however not be considered,
instead using the initial value of the attribute (zero.)

Refactor the code a bit to make it easy to consider the correct initial
value even when updating an existing FEConvolveMatrix, introducing
new methods matrixOrder() and targetPoint(). Clean up and simplify
as appropriate.

This fixes the following tests:

 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetX-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetY-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetX-prop.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetY-prop.html

with https://codereview.chromium.org/2104943005 applied.

BUG=231560
Committed: https://crrev.com/2afeaf3f17234c598144446360c869d7cb442867
Cr-Commit-Position: refs/heads/master@{#403814}

[fs, 2016-07-04 16:40:50]

Description was changed from

==========
Consider 'order' when updating feConvolveMatrix 'target*'

SVGFEConvolveMatrixElement's 'targetX' or 'targetY' attribute depend on
'order' for their initial value. When updating the target value of
an instantiated FEConvolveMatrix, order would however not be considered,
instead using the initial value of the attribute (zero.)

Refactor the code a bit to make it easy to consider the correct initial
value even when updating an existing FEConvolveMatrix, introducing
new methods matrixOrder() and targetPoint(). Clean up and simplify
as appropriate.

This fixes the following tests:

 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetX-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetY-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetX-prop.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetY-prop.html

With https://codereview.chromium.org/2104943005 applied.

BUG=231560
==========

to

==========
Consider 'order' when updating feConvolveMatrix 'target*'

SVGFEConvolveMatrixElement's 'targetX' or 'targetY' attribute depend on
'order' for their initial value. When updating the target value of
an instantiated FEConvolveMatrix, order would however not be considered,
instead using the initial value of the attribute (zero.)

Refactor the code a bit to make it easy to consider the correct initial
value even when updating an existing FEConvolveMatrix, introducing
new methods matrixOrder() and targetPoint(). Clean up and simplify
as appropriate.

This fixes the following tests:

 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetX-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetY-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetX-prop.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetY-prop.html

with https://codereview.chromium.org/2104943005 applied.

BUG=231560
==========

[fs, 2016-07-04 17:36:24]

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

[f(malita), 2016-07-05 15:20:12]

lgtm

https://codereview.chromium.org/2124583002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp (right):

https://codereview.chromium.org/2124583002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp:134:
target.setX(order.width() / 2);
I was worried here about negative half-value treatment (both int coercion and
trunc vs. floor discrepancies).

But, reading the spec, 'order' cannot be negative.  Just to double-check: is the
parser enforcing order.x/y > 0 or do we need to clamp in matrixOrder?

[fs, 2016-07-05 15:29:43]

https://codereview.chromium.org/2124583002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp (right):

https://codereview.chromium.org/2124583002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp:134:
target.setX(order.width() / 2);
On 2016/07/05 at 15:20:12, f(malita) wrote:
> I was worried here about negative half-value treatment (both int coercion and
trunc vs. floor discrepancies).
> 
> But, reading the spec, 'order' cannot be negative.  Just to double-check: is
the parser enforcing order.x/y > 0 or do we need to clamp in matrixOrder?

We verify all the parameters down in FEConvolveMatrix::parametersValid (and
produce transparent black if anything is "out of spec").

[fs, 2016-07-05 17:30:03]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-07-05 17:30:16]

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-07-05 19:29:26]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-05 19:31:28]

Description was changed from

==========
Consider 'order' when updating feConvolveMatrix 'target*'

SVGFEConvolveMatrixElement's 'targetX' or 'targetY' attribute depend on
'order' for their initial value. When updating the target value of
an instantiated FEConvolveMatrix, order would however not be considered,
instead using the initial value of the attribute (zero.)

Refactor the code a bit to make it easy to consider the correct initial
value even when updating an existing FEConvolveMatrix, introducing
new methods matrixOrder() and targetPoint(). Clean up and simplify
as appropriate.

This fixes the following tests:

 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetX-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetY-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetX-prop.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetY-prop.html

with https://codereview.chromium.org/2104943005 applied.

BUG=231560
==========

to

==========
Consider 'order' when updating feConvolveMatrix 'target*'

SVGFEConvolveMatrixElement's 'targetX' or 'targetY' attribute depend on
'order' for their initial value. When updating the target value of
an instantiated FEConvolveMatrix, order would however not be considered,
instead using the initial value of the attribute (zero.)

Refactor the code a bit to make it easy to consider the correct initial
value even when updating an existing FEConvolveMatrix, introducing
new methods matrixOrder() and targetPoint(). Clean up and simplify
as appropriate.

This fixes the following tests:

 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetX-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-targetY-attr.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetX-prop.html
 svg/dynamic-updates/SVGFEConvolveMatrixElement-svgdom-targetY-prop.html

with https://codereview.chromium.org/2104943005 applied.

BUG=231560

Committed: https://crrev.com/2afeaf3f17234c598144446360c869d7cb442867
Cr-Commit-Position: refs/heads/master@{#403814}
==========

[commit-bot: I haz the power, 2016-07-05 19:31:29]

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