Consider 'order' when updating feConvolveMatrix 'target*'

third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp

 /*
  * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 100 matching lines @@
     visitor->trace(m_kernelUnitLength);
     visitor->trace(m_order);
     visitor->trace(m_preserveAlpha);
     visitor->trace(m_targetX);
     visitor->trace(m_targetY);
     SVGFilterPrimitiveStandardAttributes::trace(visitor);
 }
 
 DEFINE_NODE_FACTORY(SVGFEConvolveMatrixElement)
 
+IntSize SVGFEConvolveMatrixElement::matrixOrder() const
+{
+    if (!m_order->isSpecified())
+        return IntSize(3, 3);
+    return IntSize(orderX()->currentValue()->value(), orderY()->currentValue()->value());
+}
+
+IntPoint SVGFEConvolveMatrixElement::targetPoint() const
+{
+    IntSize order = matrixOrder();
+    IntPoint target(m_targetX->currentValue()->value(), m_targetY->currentValue()->value());
+    // The spec says the default value is: targetX = floor ( orderX / 2 ))
+    if (!m_targetX->isSpecified())
+        target.setX(order.width() / 2);

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

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]

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

+    // The spec says the default value is: targetY = floor ( orderY / 2 ))
+    if (!m_targetY->isSpecified())
+        target.setY(order.height() / 2);
+    return target;
+}
+
 bool SVGFEConvolveMatrixElement::setFilterEffectAttribute(FilterEffect* effect, const QualifiedName& attrName)
 {
     FEConvolveMatrix* convolveMatrix = static_cast<FEConvolveMatrix*>(effect);
     if (attrName == SVGNames::edgeModeAttr)
         return convolveMatrix->setEdgeMode(m_edgeMode->currentValue()->enumValue());
     if (attrName == SVGNames::divisorAttr)
         return convolveMatrix->setDivisor(m_divisor->currentValue()->value());
     if (attrName == SVGNames::biasAttr)
         return convolveMatrix->setBias(m_bias->currentValue()->value());
-    if (attrName == SVGNames::targetXAttr)
-        return convolveMatrix->setTargetOffset(IntPoint(m_targetX->currentValue()->value(), m_targetY->currentValue()->value()));
-    if (attrName == SVGNames::targetYAttr)
-        return convolveMatrix->setTargetOffset(IntPoint(m_targetX->currentValue()->value(), m_targetY->currentValue()->value()));
+    if (attrName == SVGNames::targetXAttr || attrName == SVGNames::targetYAttr)
+        return convolveMatrix->setTargetOffset(targetPoint());
     if (attrName == SVGNames::preserveAlphaAttr)
         return convolveMatrix->setPreserveAlpha(m_preserveAlpha->currentValue()->value());
 
     ASSERT_NOT_REACHED();
     return false;
 }
 
 void SVGFEConvolveMatrixElement::svgAttributeChanged(const QualifiedName& attrName)
 {
     if (attrName == SVGNames::edgeModeAttr
@@ skipping 16 matching lines @@
     }
 
     SVGFilterPrimitiveStandardAttributes::svgAttributeChanged(attrName);
 }
 
 FilterEffect* SVGFEConvolveMatrixElement::build(SVGFilterBuilder* filterBuilder, Filter* filter)
 {
     FilterEffect* input1 = filterBuilder->getEffectById(AtomicString(m_in1->currentValue()->value()));
     ASSERT(input1);
 
-    int orderXValue = orderX()->currentValue()->value();
-    int orderYValue = orderY()->currentValue()->value();
-    if (!m_order->isSpecified()) {
-        orderXValue = 3;
-        orderYValue = 3;
-    }
-
-    int targetXValue = m_targetX->currentValue()->value();
-    // The spec says the default value is: targetX = floor ( orderX / 2 ))
-    if (!m_targetX->isSpecified())
-        targetXValue = static_cast<int>(floorf(orderXValue / 2));
-
-    int targetYValue = m_targetY->currentValue()->value();
-    // The spec says the default value is: targetY = floor ( orderY / 2 ))
-    if (!m_targetY->isSpecified())
-        targetYValue = static_cast<int>(floorf(orderYValue / 2));
-
     float divisorValue = m_divisor->currentValue()->value();
     if (!m_divisor->isSpecified()) {
         SVGNumberList* kernelMatrix = m_kernelMatrix->currentValue();
         size_t kernelMatrixSize = kernelMatrix->length();
         for (size_t i = 0; i < kernelMatrixSize; ++i)
             divisorValue += kernelMatrix->at(i)->value();
         if (!divisorValue)
             divisorValue = 1;
     }
 
     FilterEffect* effect = FEConvolveMatrix::create(filter,
-        IntSize(orderXValue, orderYValue), divisorValue,
-        m_bias->currentValue()->value(), IntPoint(targetXValue, targetYValue), m_edgeMode->currentValue()->enumValue(),
+        matrixOrder(), divisorValue,
+        m_bias->currentValue()->value(), targetPoint(), m_edgeMode->currentValue()->enumValue(),
         m_preserveAlpha->currentValue()->value(), m_kernelMatrix->currentValue()->toFloatVector());
     effect->inputEffects().append(input1);
     return effect;
 }
 
 } // namespace blink