These are the non-functional code simplification parts of bug 619992 unbitrotted.

Comment on attachment 546179 [details] [diff] [review] patch >diff --git a/content/svg/content/src/nsSVGFilters.cpp b/content/svg/content/src/nsSVGFilters.cpp >--- a/content/svg/content/src/nsSVGFilters.cpp >+++ b/content/svg/content/src/nsSVGFilters.cpp >@@ -144,26 +144,24 @@ nsSVGFE::SetupScalingFilter(nsSVGFilterI > result.mRescaling = aKernelUnitLength->IsExplicitlySet(); > if (!result.mRescaling) { > result.mSource = aSource->mImage; > result.mTarget = aTarget->mImage; > result.mDataRect = aDataRect; > return result; > } > >- float kernelX, kernelY; >- nsSVGLength2 val; >- val.Init(nsSVGUtils::X, 0xff, >- aKernelUnitLength->GetAnimValue(nsSVGNumberPair::eFirst), >- nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- kernelX = aInstance->GetPrimitiveLength(&val); >- val.Init(nsSVGUtils::Y, 0xff, >- aKernelUnitLength->GetAnimValue(nsSVGNumberPair::eSecond), >- nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- kernelY = aInstance->GetPrimitiveLength(&val); >+ float kernelX = >+ aInstance->GetPrimitiveNumber( >+ nsSVGUtils::X, >+ aKernelUnitLength, nsSVGNumberPair::eFirst); >+ float kernelY = >+ aInstance->GetPrimitiveNumber( >+ nsSVGUtils::Y, >+ aKernelUnitLength, nsSVGNumberPair::eSecond); > if (kernelX <= 0 || kernelY <= 0) > return result; > > PRBool overflow = PR_FALSE; > gfxIntSize scaledSize = > nsSVGUtils::ConvertToSurfaceSize(gfxSize(aTarget->mImage->Width() / kernelX, > aTarget->mImage->Height() / kernelY), > &overflow); >@@ -646,25 +644,22 @@ GetBlurBoxSize(double aStdDev) > return max; > return PRUint32(floor(size + 0.5)); > } > > nsresult > nsSVGFEGaussianBlurElement::GetDXY(PRUint32 *aDX, PRUint32 *aDY, > const nsSVGFilterInstance& aInstance) > { >- float stdX = mNumberPairAttributes[STD_DEV].GetAnimValue(nsSVGNumberPair::eFirst); >- float stdY = mNumberPairAttributes[STD_DEV].GetAnimValue(nsSVGNumberPair::eSecond); >- >- nsSVGLength2 val; >- val.Init(nsSVGUtils::X, 0xff, stdX, nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- stdX = aInstance.GetPrimitiveLength(&val); >- >- val.Init(nsSVGUtils::Y, 0xff, stdY, nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- stdY = aInstance.GetPrimitiveLength(&val); >+ float stdX = aInstance.GetPrimitiveNumber( >+ nsSVGUtils::X, >+ &mNumberPairAttributes[STD_DEV], nsSVGNumberPair::eFirst); >+ float stdY = aInstance.GetPrimitiveNumber( >+ nsSVGUtils::Y, >+ &mNumberPairAttributes[STD_DEV], nsSVGNumberPair::eSecond); > > if (stdX < 0 || stdY < 0) > return NS_ERROR_FAILURE; > > // If the box size is greater than twice the temporary surface size > // in an axis, then each pixel will be set to the average of all the > // other pixel values. > *aDX = GetBlurBoxSize(stdX); >@@ -2561,28 +2556,20 @@ NS_IMETHODIMP nsSVGFEOffsetElement::GetD > NS_IMETHODIMP nsSVGFEOffsetElement::GetDy(nsIDOMSVGAnimatedNumber * *aDy) > { > return mNumberAttributes[DY].ToDOMAnimatedNumber(aDy, this); > } > > nsIntPoint > nsSVGFEOffsetElement::GetOffset(const nsSVGFilterInstance& aInstance) > { >- nsIntPoint offset; >- float fltX, fltY; >- nsSVGLength2 val; >- >- GetAnimatedNumberValues(&fltX, &fltY, nsnull); >- val.Init(nsSVGUtils::X, 0xff, fltX, nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- offset.x = PRInt32(aInstance.GetPrimitiveLength(&val)); >- >- val.Init(nsSVGUtils::Y, 0xff, fltY, nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- offset.y = PRInt32(aInstance.GetPrimitiveLength(&val)); >- >- return offset; >+ return nsIntPoint(PRInt32(aInstance.GetPrimitiveNumber( >+ nsSVGUtils::X, &mNumberAttributes[DX])), >+ PRInt32(aInstance.GetPrimitiveNumber( >+ nsSVGUtils::Y, &mNumberAttributes[DY]))); > } > > nsresult > nsSVGFEOffsetElement::Filter(nsSVGFilterInstance *instance, > const nsTArray<const Image*>& aSources, > const Image* aTarget, > const nsIntRect& rect) > { >@@ -3693,29 +3680,30 @@ nsSVGFEMorphologyElement::ComputeChangeB > { > return InflateRect(aSourceChangeBoxes[0], aInstance); > } > > #define MORPHOLOGY_EPSILON 0.0001 > > void > nsSVGFEMorphologyElement::GetRXY(PRInt32 *aRX, PRInt32 *aRY, >- const nsSVGFilterInstance& aInstance) >-{ >- float rx = mNumberPairAttributes[RADIUS].GetAnimValue(nsSVGNumberPair::eFirst); >- float ry = mNumberPairAttributes[RADIUS].GetAnimValue(nsSVGNumberPair::eSecond); >- nsSVGLength2 val; >- val.Init(nsSVGUtils::X, 0xff, rx, nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >+ const nsSVGFilterInstance& aInstance) >+{ > // Subtract an epsilon here because we don't want a value that's just > // slightly larger than an integer to round up to the next integer; it's > // probably meant to be the integer it's close to, modulo machine precision > // issues. >- *aRX = NSToIntCeil(aInstance.GetPrimitiveLength(&val) - MORPHOLOGY_EPSILON); >- val.Init(nsSVGUtils::Y, 0xff, ry, nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- *aRY = NSToIntCeil(aInstance.GetPrimitiveLength(&val) - MORPHOLOGY_EPSILON); >+ *aRX = NSToIntCeil(aInstance.GetPrimitiveNumber( >+ nsSVGUtils::X, >+ &mNumberPairAttributes[RADIUS], nsSVGNumberPair::eFirst) - >+ MORPHOLOGY_EPSILON); >+ *aRY = NSToIntCeil(aInstance.GetPrimitiveNumber( >+ nsSVGUtils::Y, >+ &mNumberPairAttributes[RADIUS], nsSVGNumberPair::eSecond) - >+ MORPHOLOGY_EPSILON); > } > > nsresult > nsSVGFEMorphologyElement::Filter(nsSVGFilterInstance *instance, > const nsTArray<const Image*>& aSources, > const Image* aTarget, > const nsIntRect& rect) > { >@@ -5852,34 +5840,31 @@ NS_IMETHODIMP nsSVGFEDisplacementMapElem > } > > nsresult > nsSVGFEDisplacementMapElement::Filter(nsSVGFilterInstance *instance, > const nsTArray<const Image*>& aSources, > const Image* aTarget, > const nsIntRect& rect) > { >- float scale = mNumberAttributes[SCALE].GetAnimValue(); >+ float scale = instance->GetPrimitiveNumber( >+ nsSVGUtils::XY, &mNumberAttributes[SCALE]); > if (scale == 0.0f) { > CopyRect(aTarget, aSources[0], rect); > return NS_OK; > } > > PRInt32 width = instance->GetSurfaceWidth(); > PRInt32 height = instance->GetSurfaceHeight(); > > PRUint8* sourceData = aSources[0]->mImage->Data(); > PRUint8* displacementData = aSources[1]->mImage->Data(); > PRUint8* targetData = aTarget->mImage->Data(); > PRUint32 stride = aTarget->mImage->Stride(); > >- nsSVGLength2 val; >- val.Init(nsSVGUtils::XY, 0xff, scale, nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >- scale = instance->GetPrimitiveLength(&val); >- > static const PRUint16 channelMap[5] = { > 0, > GFX_ARGB32_OFFSET_R, > GFX_ARGB32_OFFSET_G, > GFX_ARGB32_OFFSET_B, > GFX_ARGB32_OFFSET_A }; > PRUint16 xChannel = channelMap[mEnumAttributes[CHANNEL_X].GetAnimValue()]; > PRUint16 yChannel = channelMap[mEnumAttributes[CHANNEL_Y].GetAnimValue()]; >diff --git a/layout/svg/base/src/nsSVGFilterInstance.cpp b/layout/svg/base/src/nsSVGFilterInstance.cpp >--- a/layout/svg/base/src/nsSVGFilterInstance.cpp >+++ b/layout/svg/base/src/nsSVGFilterInstance.cpp >@@ -44,26 +44,30 @@ > #include "gfxUtils.h" > > static double Square(double aX) > { > return aX*aX; > } > > float >-nsSVGFilterInstance::GetPrimitiveLength(nsSVGLength2 *aLength) const >+nsSVGFilterInstance::GetPrimitiveNumber(PRUint8 aCtxType, float aValue) const > { >+ nsSVGLength2 val; >+ val.Init(aCtxType, 0xff, aValue, >+ nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER); >+ > float value; > if (mPrimitiveUnits == nsIDOMSVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) { >- value = nsSVGUtils::ObjectSpace(mTargetBBox, aLength); >+ value = nsSVGUtils::ObjectSpace(mTargetBBox, &val); > } else { >- value = nsSVGUtils::UserSpace(mTargetFrame, aLength); >+ value = nsSVGUtils::UserSpace(mTargetFrame, &val); > } > >- switch (aLength->GetCtxType()) { >+ switch (aCtxType) { > case nsSVGUtils::X: > return value * mFilterSpaceSize.width / mFilterRect.Width(); > case nsSVGUtils::Y: > return value * mFilterSpaceSize.height / mFilterRect.Height(); > case nsSVGUtils::XY: > default: > return value * > sqrt(Square(mFilterSpaceSize.width) + Square(mFilterSpaceSize.height)) / >@@ -121,17 +125,17 @@ nsSVGFilterInstance::GetUserSpaceToFilte > void > nsSVGFilterInstance::ComputeFilterPrimitiveSubregion(PrimitiveInfo* aPrimitive) > { > nsSVGFE* fE = aPrimitive->mFE; > > gfxRect defaultFilterSubregion(0,0,0,0); > if (fE->SubregionIsUnionOfRegions()) { > for (PRUint32 i = 0; i < aPrimitive->mInputs.Length(); ++i) { >- defaultFilterSubregion = >+ defaultFilterSubregion = > defaultFilterSubregion.Union( > aPrimitive->mInputs[i]->mImage.mFilterPrimitiveSubregion); > } > } else { > defaultFilterSubregion = > gfxRect(0, 0, mFilterSpaceSize.width, mFilterSpaceSize.height); > } > >diff --git a/layout/svg/base/src/nsSVGFilterInstance.h b/layout/svg/base/src/nsSVGFilterInstance.h >--- a/layout/svg/base/src/nsSVGFilterInstance.h >+++ b/layout/svg/base/src/nsSVGFilterInstance.h >@@ -38,38 +38,36 @@ > #define __NS_SVGFILTERINSTANCE_H__ > > #include "nsIDOMSVGLength.h" > #include "nsIDOMSVGFilters.h" > #include "nsRect.h" > #include "nsIContent.h" > #include "nsAutoPtr.h" > #include "nsSVGFilters.h" >-#include "nsISVGChildFrame.h" >-#include "nsSVGString.h" >+#include "nsSVGNumber2.h" >+#include "nsSVGNumberPair.h" > > #include "gfxImageSurface.h" > >-class nsSVGLength2; > class nsSVGElement; > class nsSVGFilterElement; > class nsSVGFilterPaintCallback; > struct gfxRect; > > /** > * This class performs all filter processing. > * > * We build a graph of the filter image data flow, essentially > * converting the filter graph to SSA. This lets us easily propagate > * analysis data (such as bounding-boxes) over the filter primitive graph. > */ > class NS_STACK_CLASS nsSVGFilterInstance > { > public: >- float GetPrimitiveLength(nsSVGLength2 *aLength) const; > void ConvertLocation(float aValues[3]) const; > > nsSVGFilterInstance(nsIFrame *aTargetFrame, > nsSVGFilterPaintCallback *aPaintCallback, > nsSVGFilterElement *aFilterElement, > const gfxRect &aTargetBBox, > const gfxRect& aFilterRect, > const nsIntSize& aFilterSpaceSize, >@@ -103,16 +101,25 @@ public: > PRInt32 GetSurfaceWidth() const { return mSurfaceRect.width; } > PRInt32 GetSurfaceHeight() const { return mSurfaceRect.height; } > > nsresult Render(gfxASurface** aOutput); > nsresult ComputeOutputDirtyRect(nsIntRect* aDirty); > nsresult ComputeSourceNeededRect(nsIntRect* aDirty); > nsresult ComputeOutputBBox(nsIntRect* aBBox); > >+ float GetPrimitiveNumber(PRUint8 aCtxType, const nsSVGNumber2 *aNumber) const >+ { >+ return GetPrimitiveNumber(aCtxType, aNumber->GetAnimValue()); >+ } >+ float GetPrimitiveNumber(PRUint8 aCtxType, const nsSVGNumberPair *aNumberPair, >+ nsSVGNumberPair::PairIndex aIndex) const >+ { >+ return GetPrimitiveNumber(aCtxType, aNumberPair->GetAnimValue(aIndex)); >+ } > gfxMatrix GetUserSpaceToFilterSpaceTransform() const; > gfxMatrix GetFilterSpaceToDeviceSpaceTransform() const { > return mFilterSpaceToDeviceSpaceTransform; > } > > private: > typedef nsSVGFE::Image Image; > typedef nsSVGFE::ColorModel ColorModel; >@@ -169,16 +176,22 @@ private: > // when using cairo to draw into the surface). The surface is cleared > // to transparent black. > already_AddRefed<gfxImageSurface> CreateImage(); > > void ComputeFilterPrimitiveSubregion(PrimitiveInfo* aInfo); > void EnsureColorModel(PrimitiveInfo* aPrimitive, > ColorModel aColorModel); > >+ /** >+ * Scales a numeric filter primitive length in the X, Y or "XY" directions >+ * into a length in filter space (no offset is applied). >+ */ >+ float GetPrimitiveNumber(PRUint8 aCtxType, float aValue) const; >+ > gfxRect UserSpaceToFilterSpace(const gfxRect& aUserSpace) const; > void ClipToFilterSpace(nsIntRect* aRect) const > { > nsIntRect filterSpace(nsIntPoint(0, 0), mFilterSpaceSize); > aRect->IntersectRect(*aRect, filterSpace); > } > void ClipToGfxRect(nsIntRect* aRect, const gfxRect& aGfx) const; >

Comment on attachment 546179 [details] [diff] [review] patch Looks good! Just a few formatting nits: >@@ -144,26 +144,24 @@ nsSVGFE::SetupScalingFilter(nsSVGFilterI >+ float kernelX = >+ aInstance->GetPrimitiveNumber( >+ nsSVGUtils::X, >+ aKernelUnitLength, nsSVGNumberPair::eFirst); >+ float kernelY = >+ aInstance->GetPrimitiveNumber( >+ nsSVGUtils::Y, >+ aKernelUnitLength, nsSVGNumberPair::eSecond); Newline / indentation seems a bit odd here. It'd make more sense if we were in danger of breaking 80 chars, but we're not really. :) This is easier to read (& fits better with Mozilla coding style): float kernelX = aInstance->GetPrimitiveNumber(nsSVGUtils::X, aKernelUnitLength, nsSVGNumberPair::eFirst); float kernelY = aInstance->GetPrimitiveNumber(nsSVGUtils::Y, aKernelUnitLength, nsSVGNumberPair::eSecond); > nsresult > nsSVGFEGaussianBlurElement::GetDXY(PRUint32 *aDX, PRUint32 *aDY, > const nsSVGFilterInstance& aInstance) > { >+ float stdX = aInstance.GetPrimitiveNumber( >+ nsSVGUtils::X, >+ &mNumberPairAttributes[STD_DEV], nsSVGNumberPair::eFirst); >+ float stdY = aInstance.GetPrimitiveNumber( >+ nsSVGUtils::Y, >+ &mNumberPairAttributes[STD_DEV], nsSVGNumberPair::eSecond); Same here -- we can format these lines similar to my suggested text above, without breaking 80 chars. > nsSVGFEMorphologyElement::GetRXY(PRInt32 *aRX, PRInt32 *aRY, [...] >+ *aRX = NSToIntCeil(aInstance.GetPrimitiveNumber( >+ nsSVGUtils::X, >+ &mNumberPairAttributes[RADIUS], nsSVGNumberPair::eFirst) - >+ MORPHOLOGY_EPSILON); >+ *aRY = NSToIntCeil(aInstance.GetPrimitiveNumber( >+ nsSVGUtils::Y, >+ &mNumberPairAttributes[RADIUS], nsSVGNumberPair::eSecond) - >+ MORPHOLOGY_EPSILON); I think MORPHOLOGY EPSILON is indented 4 spaces too far here. (The current spacing implies that it's nested deeper than the 2 lines above it, when really it's nested less-deeply.) > nsSVGFEDisplacementMapElement::Filter(nsSVGFilterInstance *instance, > const nsTArray<const Image*>& aSources, > const Image* aTarget, > const nsIntRect& rect) > { >- float scale = mNumberAttributes[SCALE].GetAnimValue(); >+ float scale = instance->GetPrimitiveNumber( >+ nsSVGUtils::XY, &mNumberAttributes[SCALE]); As above, this would be better as: float scale = instance->GetPrimitiveNumber(nsSVGUtils::XY, &mNumberAttributes[SCALE]);