From 39b28dbb283f7dd0ddb93d6d7b0036a02932fee0 Mon Sep 17 00:00:00 2001 From: Doug Thayer Date: Wed, 19 Sep 2018 17:17:43 +0000 Subject: [PATCH] Bug 1417699 - Avoid copies of filter attributes when possible r=mstange This is a more conservative optimization for bug 1417699. There's no reason we need to be copying these everywhere, so let's just go ahead and implement moves. Differential Revision: https://phabricator.services.mozilla.com/D4899 --HG-- extra : moz-landing-system : lando --- dom/svg/SVGFEComponentTransferElement.cpp | 2 +- dom/svg/SVGFEConvolveMatrixElement.cpp | 2 +- gfx/ipc/GfxMessageUtils.h | 11 ++- gfx/src/FilterSupport.cpp | 88 +++++++++++++++++++++-- gfx/src/FilterSupport.h | 20 ++++-- layout/svg/nsCSSFilterInstance.cpp | 20 ++---- layout/svg/nsFilterInstance.cpp | 39 ++++++---- layout/svg/nsFilterInstance.h | 4 +- layout/svg/nsSVGFilterInstance.cpp | 2 +- 9 files changed, 143 insertions(+), 45 deletions(-) diff --git a/dom/svg/SVGFEComponentTransferElement.cpp b/dom/svg/SVGFEComponentTransferElement.cpp index f526ac25bbb3..e6d127999380 100644 --- a/dom/svg/SVGFEComponentTransferElement.cpp +++ b/dom/svg/SVGFEComponentTransferElement.cpp @@ -87,7 +87,7 @@ SVGFEComponentTransferElement::GetPrimitiveDescription(nsSVGFilterInstance* aIns AttributeMap functionAttributes; functionAttributes.Set(eComponentTransferFunctionType, (uint32_t)SVG_FECOMPONENTTRANSFER_TYPE_IDENTITY); - descr.Attributes().Set(attributeNames[i], functionAttributes); + descr.Attributes().Set(attributeNames[i], std::move(functionAttributes)); } } return descr; diff --git a/dom/svg/SVGFEConvolveMatrixElement.cpp b/dom/svg/SVGFEConvolveMatrixElement.cpp index 5f87b2d5c93f..a224948eecf8 100644 --- a/dom/svg/SVGFEConvolveMatrixElement.cpp +++ b/dom/svg/SVGFEConvolveMatrixElement.cpp @@ -172,7 +172,7 @@ SVGFEConvolveMatrixElement::GetPrimitiveDescription(nsSVGFilterInstance* aInstan const nsTArray& aInputsAreTainted, nsTArray>& aInputImages) { - const FilterPrimitiveDescription failureDescription(PrimitiveType::Empty); + FilterPrimitiveDescription failureDescription(PrimitiveType::Empty); const SVGNumberList &kernelMatrix = mNumberListAttributes[KERNELMATRIX].GetAnimValue(); diff --git a/gfx/ipc/GfxMessageUtils.h b/gfx/ipc/GfxMessageUtils.h index d43335f5d5d5..f3d271fd22ba 100644 --- a/gfx/ipc/GfxMessageUtils.h +++ b/gfx/ipc/GfxMessageUtils.h @@ -819,10 +819,19 @@ struct ParamTraits HANDLE_TYPE(mozilla::gfx::Matrix5x4, Matrix5x4) HANDLE_TYPE(mozilla::gfx::Point3D, Point3D) HANDLE_TYPE(mozilla::gfx::Color, Color) - HANDLE_TYPE(mozilla::gfx::AttributeMap, AttributeMap) #undef HANDLE_TYPE + case mozilla::gfx::AttributeType::eAttributeMap: + { + mozilla::gfx::AttributeMap value; + if (!ReadParam(aMsg, aIter, &value)) { + return false; + } + aResult->Set(name, std::move(value)); + break; + } + case mozilla::gfx::AttributeType::eFloats: { nsTArray value; diff --git a/gfx/src/FilterSupport.cpp b/gfx/src/FilterSupport.cpp index c72651cef021..dc81310a9b54 100644 --- a/gfx/src/FilterSupport.cpp +++ b/gfx/src/FilterSupport.cpp @@ -844,9 +844,11 @@ FilterNodeFromPrimitiveDescription(const FilterPrimitiveDescription& aDescriptio eComponentTransferFunctionB, eComponentTransferFunctionA }; + const AttributeMap* rgbFunctionAttributes = + atts.MaybeGetAttributeMap(eComponentTransferFunctionRGB); for (int32_t i = 0; i < 4; i++) { - AttributeMap functionAttributes = - atts.GetAttributeMap(componentFunctionNames[i]); + const AttributeMap& functionAttributes = i < 3 && rgbFunctionAttributes ? + *rgbFunctionAttributes : atts.GetAttributeMap(componentFunctionNames[i]); ConvertComponentTransferFunctionToFilter(functionAttributes, i, aDT, filters[0], filters[1], filters[2], filters[3]); } @@ -1072,7 +1074,7 @@ FilterNodeFromPrimitiveDescription(const FilterPrimitiveDescription& aDescriptio bool isSpecular = aDescription.Type() == PrimitiveType::SpecularLighting; - AttributeMap lightAttributes = atts.GetAttributeMap(eLightingLight); + const AttributeMap& lightAttributes = atts.GetAttributeMap(eLightingLight); if (lightAttributes.GetUint(eLightType) == eLightTypeNone) { return nullptr; @@ -1619,7 +1621,7 @@ FilterSupport::PostFilterExtentsForPrimitive(const FilterPrimitiveDescription& a case PrimitiveType::ComponentTransfer: { - AttributeMap functionAttributes = + const AttributeMap& functionAttributes = atts.GetAttributeMap(eComponentTransferFunctionA); if (ResultOfZeroUnderTransferFunction(functionAttributes) > 0.0f) { return aDescription.PrimitiveSubregion(); @@ -1875,6 +1877,34 @@ FilterPrimitiveDescription::operator=(const FilterPrimitiveDescription& aOther) return *this; } +FilterPrimitiveDescription::FilterPrimitiveDescription(FilterPrimitiveDescription&& aOther) + : mType(aOther.mType) + , mAttributes(std::move(aOther.mAttributes)) + , mInputPrimitives(std::move(aOther.mInputPrimitives)) + , mFilterPrimitiveSubregion(aOther.mFilterPrimitiveSubregion) + , mFilterSpaceBounds(aOther.mFilterSpaceBounds) + , mInputColorSpaces(std::move(aOther.mInputColorSpaces)) + , mOutputColorSpace(aOther.mOutputColorSpace) + , mIsTainted(aOther.mIsTainted) +{ +} + +FilterPrimitiveDescription& +FilterPrimitiveDescription::operator=(FilterPrimitiveDescription&& aOther) +{ + if (this != &aOther) { + mType = aOther.mType; + mAttributes = std::move(aOther.mAttributes); + mInputPrimitives = std::move(aOther.mInputPrimitives); + mFilterPrimitiveSubregion = aOther.mFilterPrimitiveSubregion; + mFilterSpaceBounds = aOther.mFilterSpaceBounds; + mInputColorSpaces = std::move(aOther.mInputColorSpaces); + mOutputColorSpace = aOther.mOutputColorSpace; + mIsTainted = aOther.mIsTainted; + } + return *this; +} + bool FilterPrimitiveDescription::operator==(const FilterPrimitiveDescription& aOther) const { @@ -1902,6 +1932,7 @@ FilterDescription::operator==(const FilterDescription& aOther) const // used by AttributeMap. struct FilterAttribute { FilterAttribute(const FilterAttribute& aOther); + ~FilterAttribute(); bool operator==(const FilterAttribute& aOther) const; @@ -1940,11 +1971,19 @@ struct FilterAttribute { MAKE_CONSTRUCTOR_AND_ACCESSOR_CLASS(Matrix5x4) MAKE_CONSTRUCTOR_AND_ACCESSOR_CLASS(Point3D) MAKE_CONSTRUCTOR_AND_ACCESSOR_CLASS(Color) - MAKE_CONSTRUCTOR_AND_ACCESSOR_CLASS(AttributeMap) #undef MAKE_CONSTRUCTOR_AND_ACCESSOR_BASIC #undef MAKE_CONSTRUCTOR_AND_ACCESSOR_CLASS + explicit FilterAttribute(AttributeMap&& aValue) + : mType(AttributeType::eAttributeMap), mAttributeMap(new AttributeMap(aValue)) + {} + + AttributeMap* AsAttributeMap() { + MOZ_ASSERT(mType == AttributeType::eAttributeMap); + return mAttributeMap; + } + FilterAttribute(const float* aValue, uint32_t aLength) : mType(AttributeType::eFloats) { @@ -2110,6 +2149,21 @@ AttributeMap::operator=(const AttributeMap& aOther) return *this; } +AttributeMap::AttributeMap(AttributeMap&& aOther) +{ + mMap.SwapElements(aOther.mMap); +} + +AttributeMap& +AttributeMap::operator=(AttributeMap&& aOther) +{ + if (this != &aOther) { + mMap.Clear(); + mMap.SwapElements(aOther.mMap); + } + return *this; +} + bool AttributeMap::operator==(const AttributeMap& aOther) const { @@ -2179,11 +2233,33 @@ MAKE_ATTRIBUTE_HANDLERS_CLASS(Matrix) MAKE_ATTRIBUTE_HANDLERS_CLASS(Matrix5x4) MAKE_ATTRIBUTE_HANDLERS_CLASS(Point3D) MAKE_ATTRIBUTE_HANDLERS_CLASS(Color) -MAKE_ATTRIBUTE_HANDLERS_CLASS(AttributeMap) #undef MAKE_ATTRIBUTE_HANDLERS_BASIC #undef MAKE_ATTRIBUTE_HANDLERS_CLASS +const AttributeMap sEmptyAttributeMap; + +const AttributeMap& +AttributeMap::GetAttributeMap(AttributeName aName) const { + Attribute* value = mMap.Get(aName); + return value ? *value->AsAttributeMap() : sEmptyAttributeMap; +} + +const AttributeMap* +AttributeMap::MaybeGetAttributeMap(AttributeName aName) const { + Attribute* value = mMap.Get(aName); + if (value) { + return value->AsAttributeMap(); + } else { + return nullptr; + } +} + +void +AttributeMap::Set(AttributeName aName, AttributeMap&& aValue) { + mMap.Put(aName, new Attribute(std::move(aValue))); +} + const nsTArray& AttributeMap::GetFloats(AttributeName aName) const { diff --git a/gfx/src/FilterSupport.h b/gfx/src/FilterSupport.h index 6524d929b887..d39e9a1fb5f6 100644 --- a/gfx/src/FilterSupport.h +++ b/gfx/src/FilterSupport.h @@ -96,6 +96,10 @@ enum AttributeName { eFloodColor, eTileSourceRect, eOpacityOpacity, + // In many cases R, G, and B will all use an identical + // attribute map - in this case we can reduce unnecessary + // copying by having one shared AttributeName + eComponentTransferFunctionRGB, eComponentTransferFunctionR, eComponentTransferFunctionG, eComponentTransferFunctionB, @@ -190,6 +194,8 @@ public: AttributeMap(); AttributeMap(const AttributeMap& aOther); AttributeMap& operator=(const AttributeMap& aOther); + AttributeMap(AttributeMap&& aOther); + AttributeMap& operator=(AttributeMap&& aOther); bool operator==(const AttributeMap& aOther) const; bool operator!=(const AttributeMap& aOther) const { @@ -207,7 +213,7 @@ public: void Set(AttributeName aName, const Matrix5x4& aValue); void Set(AttributeName aName, const Point3D& aValue); void Set(AttributeName aName, const Color& aValue); - void Set(AttributeName aName, const AttributeMap& aValue); + void Set(AttributeName aName, AttributeMap&& aValue); void Set(AttributeName aName, const float* aValues, int32_t aLength); bool GetBool(AttributeName aName) const; @@ -220,7 +226,8 @@ public: Matrix5x4 GetMatrix5x4(AttributeName aName) const; Point3D GetPoint3D(AttributeName aName) const; Color GetColor(AttributeName aName) const; - AttributeMap GetAttributeMap(AttributeName aName) const; + const AttributeMap& GetAttributeMap(AttributeName aName) const; + const AttributeMap* MaybeGetAttributeMap(AttributeName aName) const; const nsTArray& GetFloats(AttributeName aName) const; uint32_t Count() const; @@ -311,6 +318,8 @@ public: FilterPrimitiveDescription(); explicit FilterPrimitiveDescription(PrimitiveType aType); + FilterPrimitiveDescription(FilterPrimitiveDescription&& aOther); + FilterPrimitiveDescription& operator=(FilterPrimitiveDescription&& aOther); FilterPrimitiveDescription(const FilterPrimitiveDescription& aOther); FilterPrimitiveDescription& operator=(const FilterPrimitiveDescription& aOther); @@ -394,9 +403,10 @@ private: */ struct FilterDescription final { FilterDescription() {} - explicit FilterDescription(const nsTArray& aPrimitives) - : mPrimitives(aPrimitives) - {} + explicit FilterDescription(nsTArray&& aPrimitives) + { + mPrimitives.SwapElements(aPrimitives); + } bool operator==(const FilterDescription& aOther) const; bool operator!=(const FilterDescription& aOther) const diff --git a/layout/svg/nsCSSFilterInstance.cpp b/layout/svg/nsCSSFilterInstance.cpp index a54f2cddcd22..ec772113d180 100644 --- a/layout/svg/nsCSSFilterInstance.cpp +++ b/layout/svg/nsCSSFilterInstance.cpp @@ -122,7 +122,7 @@ nsCSSFilterInstance::BuildPrimitives(nsTArray& aPrim SetBounds(descr, aPrimitiveDescrs); // Add this primitive to the filter chain. - aPrimitiveDescrs.AppendElement(descr); + aPrimitiveDescrs.AppendElement(std::move(descr)); return NS_OK; } @@ -165,15 +165,13 @@ nsCSSFilterInstance::SetAttributesForBrightness(FilterPrimitiveDescription& aDes (uint32_t)SVG_FECOMPONENTTRANSFER_TYPE_LINEAR); brightnessAttrs.Set(eComponentTransferFunctionSlope, value); brightnessAttrs.Set(eComponentTransferFunctionIntercept, 0.0f); - aDescr.Attributes().Set(eComponentTransferFunctionR, brightnessAttrs); - aDescr.Attributes().Set(eComponentTransferFunctionG, brightnessAttrs); - aDescr.Attributes().Set(eComponentTransferFunctionB, brightnessAttrs); + aDescr.Attributes().Set(eComponentTransferFunctionRGB, std::move(brightnessAttrs)); // Set identity transfer function for A. AttributeMap identityAttrs; identityAttrs.Set(eComponentTransferFunctionType, (uint32_t)SVG_FECOMPONENTTRANSFER_TYPE_IDENTITY); - aDescr.Attributes().Set(eComponentTransferFunctionA, identityAttrs); + aDescr.Attributes().Set(eComponentTransferFunctionA, std::move(identityAttrs)); return NS_OK; } @@ -191,15 +189,13 @@ nsCSSFilterInstance::SetAttributesForContrast(FilterPrimitiveDescription& aDescr (uint32_t)SVG_FECOMPONENTTRANSFER_TYPE_LINEAR); contrastAttrs.Set(eComponentTransferFunctionSlope, value); contrastAttrs.Set(eComponentTransferFunctionIntercept, intercept); - aDescr.Attributes().Set(eComponentTransferFunctionR, contrastAttrs); - aDescr.Attributes().Set(eComponentTransferFunctionG, contrastAttrs); - aDescr.Attributes().Set(eComponentTransferFunctionB, contrastAttrs); + aDescr.Attributes().Set(eComponentTransferFunctionRGB, std::move(contrastAttrs)); // Set identity transfer function for A. AttributeMap identityAttrs; identityAttrs.Set(eComponentTransferFunctionType, (uint32_t)SVG_FECOMPONENTTRANSFER_TYPE_IDENTITY); - aDescr.Attributes().Set(eComponentTransferFunctionA, identityAttrs); + aDescr.Attributes().Set(eComponentTransferFunctionA, std::move(identityAttrs)); return NS_OK; } @@ -272,15 +268,13 @@ nsCSSFilterInstance::SetAttributesForInvert(FilterPrimitiveDescription& aDescr) invertAttrs.Set(eComponentTransferFunctionType, (uint32_t)SVG_FECOMPONENTTRANSFER_TYPE_TABLE); invertAttrs.Set(eComponentTransferFunctionTableValues, invertTableValues, 2); - aDescr.Attributes().Set(eComponentTransferFunctionR, invertAttrs); - aDescr.Attributes().Set(eComponentTransferFunctionG, invertAttrs); - aDescr.Attributes().Set(eComponentTransferFunctionB, invertAttrs); + aDescr.Attributes().Set(eComponentTransferFunctionRGB, std::move(invertAttrs)); // Set identity transfer function for A. AttributeMap identityAttrs; identityAttrs.Set(eComponentTransferFunctionType, (uint32_t)SVG_FECOMPONENTTRANSFER_TYPE_IDENTITY); - aDescr.Attributes().Set(eComponentTransferFunctionA, identityAttrs); + aDescr.Attributes().Set(eComponentTransferFunctionA, std::move(identityAttrs)); return NS_OK; } diff --git a/layout/svg/nsFilterInstance.cpp b/layout/svg/nsFilterInstance.cpp index f48566bcc479..0557cd432742 100644 --- a/layout/svg/nsFilterInstance.cpp +++ b/layout/svg/nsFilterInstance.cpp @@ -306,20 +306,20 @@ nsFilterInstance::BuildPrimitives(const nsTArray& aFilterChain, nsIFrame* aTargetFrame, bool aFilterInputIsTainted) { - NS_ASSERTION(!mPrimitiveDescriptions.Length(), - "expected to start building primitives from scratch"); + nsTArray primitiveDescriptions; for (uint32_t i = 0; i < aFilterChain.Length(); i++) { bool inputIsTainted = - mPrimitiveDescriptions.IsEmpty() ? aFilterInputIsTainted : - mPrimitiveDescriptions.LastElement().IsTainted(); - nsresult rv = BuildPrimitivesForFilter(aFilterChain[i], aTargetFrame, inputIsTainted); + primitiveDescriptions.IsEmpty() ? aFilterInputIsTainted : + primitiveDescriptions.LastElement().IsTainted(); + nsresult rv = BuildPrimitivesForFilter(aFilterChain[i], aTargetFrame, inputIsTainted, + primitiveDescriptions); if (NS_FAILED(rv)) { return rv; } } - mFilterDescription = FilterDescription(mPrimitiveDescriptions); + mFilterDescription = FilterDescription(std::move(primitiveDescriptions)); return NS_OK; } @@ -327,7 +327,8 @@ nsFilterInstance::BuildPrimitives(const nsTArray& aFilterChain, nsresult nsFilterInstance::BuildPrimitivesForFilter(const nsStyleFilter& aFilter, nsIFrame* aTargetFrame, - bool aInputIsTainted) + bool aInputIsTainted, + nsTArray& aPrimitiveDescriptions) { NS_ASSERTION(mUserSpaceToFilterSpaceScale.width > 0.0f && mFilterSpaceToUserSpaceScale.height > 0.0f, @@ -343,7 +344,7 @@ nsFilterInstance::BuildPrimitivesForFilter(const nsStyleFilter& aFilter, return NS_ERROR_FAILURE; } - return svgFilterInstance.BuildPrimitives(mPrimitiveDescriptions, mInputImages, + return svgFilterInstance.BuildPrimitives(aPrimitiveDescriptions, mInputImages, aInputIsTainted); } @@ -357,7 +358,7 @@ nsFilterInstance::BuildPrimitivesForFilter(const nsStyleFilter& aFilter, nsCSSFilterInstance cssFilterInstance(aFilter, shadowFallbackColor, mTargetBounds, mFrameSpaceInCSSPxToFilterSpaceTransform); - return cssFilterInstance.BuildPrimitives(mPrimitiveDescriptions, aInputIsTainted); + return cssFilterInstance.BuildPrimitives(aPrimitiveDescriptions, aInputIsTainted); } static void @@ -376,8 +377,9 @@ UpdateNeededBounds(const nsIntRegion& aRegion, nsIntRect& aBounds) void nsFilterInstance::ComputeNeededBoxes() { - if (mPrimitiveDescriptions.IsEmpty()) + if (mFilterDescription.mPrimitives.IsEmpty()) { return; + } nsIntRegion sourceGraphicNeededRegion; nsIntRegion fillPaintNeededRegion; @@ -497,8 +499,9 @@ void nsFilterInstance::Render(gfxContext* aCtx, imgDrawingParams& aImgParams, float aOpacity) { MOZ_ASSERT(mTargetFrame, "Need a frame for rendering"); + MOZ_ASSERT(mInitialized); - if (mPrimitiveDescriptions.IsEmpty()) { + if (mFilterDescription.mPrimitives.IsEmpty()) { // An filter without any primitive. Treat it as success and paint nothing. return; } @@ -528,7 +531,9 @@ nsFilterInstance::Render(gfxContext* aCtx, imgDrawingParams& aImgParams, float a nsRegion nsFilterInstance::ComputePostFilterDirtyRegion() { - if (mPreFilterDirtyRegion.IsEmpty() || mPrimitiveDescriptions.IsEmpty()) { + MOZ_ASSERT(mInitialized); + + if (mPreFilterDirtyRegion.IsEmpty() || mFilterDescription.mPrimitives.IsEmpty()) { return nsRegion(); } @@ -541,7 +546,9 @@ nsFilterInstance::ComputePostFilterDirtyRegion() nsRect nsFilterInstance::ComputePostFilterExtents() { - if (mPrimitiveDescriptions.IsEmpty()) { + MOZ_ASSERT(mInitialized); + + if (mFilterDescription.mPrimitives.IsEmpty()) { return nsRect(); } @@ -560,11 +567,13 @@ nsFilterInstance::ComputeSourceNeededRect() nsIntRect nsFilterInstance::OutputFilterSpaceBounds() const { - uint32_t numPrimitives = mPrimitiveDescriptions.Length(); + MOZ_ASSERT(mInitialized); + + uint32_t numPrimitives = mFilterDescription.mPrimitives.Length(); if (numPrimitives <= 0) return nsIntRect(); - return mPrimitiveDescriptions[numPrimitives - 1].PrimitiveSubregion(); + return mFilterDescription.mPrimitives[numPrimitives - 1].PrimitiveSubregion(); } nsIntRect diff --git a/layout/svg/nsFilterInstance.h b/layout/svg/nsFilterInstance.h index 3a26dd7988e0..e72ab24ca679 100644 --- a/layout/svg/nsFilterInstance.h +++ b/layout/svg/nsFilterInstance.h @@ -255,7 +255,8 @@ private: */ nsresult BuildPrimitivesForFilter(const nsStyleFilter& aFilter, nsIFrame* aTargetFrame, - bool aInputIsTainted); + bool aInputIsTainted, + nsTArray& aPrimitiveDescriptions); /** * Computes the filter space bounds of the areas that we actually *need* from @@ -372,7 +373,6 @@ private: gfxMatrix mPaintTransform; nsTArray> mInputImages; - nsTArray mPrimitiveDescriptions; FilterDescription mFilterDescription; bool mInitialized; }; diff --git a/layout/svg/nsSVGFilterInstance.cpp b/layout/svg/nsSVGFilterInstance.cpp index e41f1d94ebc0..1c284e0ed5ef 100644 --- a/layout/svg/nsSVGFilterInstance.cpp +++ b/layout/svg/nsSVGFilterInstance.cpp @@ -440,7 +440,7 @@ nsSVGFilterInstance::BuildPrimitives(nsTArray& aPrim descr.SetOutputColorSpace(filter->GetOutputColorSpace()); } - aPrimitiveDescrs.AppendElement(descr); + aPrimitiveDescrs.AppendElement(std::move(descr)); uint32_t primitiveDescrIndex = aPrimitiveDescrs.Length() - 1; nsAutoString str;