From b7303e58bda90f9535a1c4f90fb1603065efe86b Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Thu, 3 May 2012 17:05:40 +0100 Subject: [PATCH] Bug 413960, part 1 - Clean up and document various parts of misleading viewBox code. r=longsonr. --HG-- extra : rebase_source : f9dc173b36d4afbce3a8e256f7aa90d9d231615a --- .../svg/content/src/nsSVGMarkerElement.cpp | 2 +- content/svg/content/src/nsSVGSVGElement.cpp | 12 +++---- content/svg/content/src/nsSVGSVGElement.h | 31 ++++++++++++++----- content/svg/content/src/nsSVGViewBox.h | 13 ++++++-- layout/svg/base/src/nsSVGInnerSVGFrame.cpp | 4 +-- layout/svg/base/src/nsSVGOuterSVGFrame.cpp | 2 +- layout/svg/base/src/nsSVGPatternFrame.cpp | 4 +-- layout/svg/base/src/nsSVGUtils.cpp | 14 --------- layout/svg/base/src/nsSVGUtils.h | 10 ------ 9 files changed, 47 insertions(+), 45 deletions(-) diff --git a/content/svg/content/src/nsSVGMarkerElement.cpp b/content/svg/content/src/nsSVGMarkerElement.cpp index e82830f7625..ed15af2157c 100644 --- a/content/svg/content/src/nsSVGMarkerElement.cpp +++ b/content/svg/content/src/nsSVGMarkerElement.cpp @@ -370,7 +370,7 @@ nsSVGMarkerElement::GetMarkerTransform(float aStrokeWidth, nsSVGViewBoxRect nsSVGMarkerElement::GetViewBoxRect() { - if (mViewBox.IsValid()) { + if (mViewBox.IsExplicitlySet()) { return mViewBox.GetAnimValue(); } return nsSVGViewBoxRect( diff --git a/content/svg/content/src/nsSVGSVGElement.cpp b/content/svg/content/src/nsSVGSVGElement.cpp index 04c47080fc5..ca421a1056e 100644 --- a/content/svg/content/src/nsSVGSVGElement.cpp +++ b/content/svg/content/src/nsSVGSVGElement.cpp @@ -976,7 +976,7 @@ nsSVGSVGElement::GetViewBoxTransform() const } nsSVGViewBoxRect viewBox; - if (mViewBox.IsValid()) { + if (HasViewBox()) { viewBox = mViewBox.GetAnimValue(); } else { viewBox.x = viewBox.y = 0.0f; @@ -1127,7 +1127,7 @@ nsSVGSVGElement::GetLength(PRUint8 aCtxType) { float h, w; - if (mViewBox.IsValid()) { + if (HasViewBox()) { const nsSVGViewBoxRect& viewbox = mViewBox.GetAnimValue(); w = viewbox.width; h = viewbox.height; @@ -1248,7 +1248,7 @@ nsSVGSVGElement::GetPreserveAspectRatio() bool nsSVGSVGElement::ShouldSynthesizeViewBox() const { - NS_ABORT_IF_FALSE(!HasValidViewbox(), + NS_ABORT_IF_FALSE(!HasViewBox(), "Should only be called if we lack a viewBox"); nsIDocument* doc = GetCurrentDoc(); @@ -1280,7 +1280,7 @@ nsSVGSVGElement:: "should only override preserveAspectRatio in images"); #endif - if (!HasValidViewbox() && ShouldSynthesizeViewBox()) { + if (!HasViewBox() && ShouldSynthesizeViewBox()) { // My non- clients will have been painting me with a synthesized // viewBox, but my client that's about to paint me now does NOT // want that. Need to tell ourselves to flush our transform. @@ -1288,7 +1288,7 @@ nsSVGSVGElement:: } mIsPaintingSVGImageElement = true; - if (!mViewBox.IsValid()) { + if (!HasViewBox()) { return; // preserveAspectRatio irrelevant (only matters if we have viewBox) } @@ -1320,7 +1320,7 @@ nsSVGSVGElement::ClearImageOverridePreserveAspectRatio() #endif mIsPaintingSVGImageElement = false; - if (!HasValidViewbox() && ShouldSynthesizeViewBox()) { + if (!HasViewBox() && ShouldSynthesizeViewBox()) { // My non- clients will want to paint me with a synthesized // viewBox, but my client that just painted me did NOT // use that. Need to tell ourselves to flush our transform. diff --git a/content/svg/content/src/nsSVGSVGElement.h b/content/svg/content/src/nsSVGSVGElement.h index c8f1b8e6a6b..ff7fae7fd1a 100644 --- a/content/svg/content/src/nsSVGSVGElement.h +++ b/content/svg/content/src/nsSVGSVGElement.h @@ -196,8 +196,31 @@ public: void SyncWidthOrHeight(nsIAtom* aName, nsSVGElement *aTarget) const; // public helpers: + + /** + * Returns true if this element has a base/anim value for its "viewBox" + * attribute that defines a viewBox rectangle with finite values. + * + * Note that this does not check whether we need to synthesize a viewBox, + * so you must call ShouldSynthesizeViewBox() if you need to check that too. + * + * Note also that this method does not pay attention to whether the width or + * height values of the viewBox rect are positive! + */ + bool HasViewBox() const { + return mViewBox.IsExplicitlySet(); + } + + /** + * Returns true if we should synthesize a viewBox for ourselves (that is, if + * we're the root element in an image document, and we're not currently being + * painted for an element). + * + * Only call this method if HasViewBox() returns false. + */ + bool ShouldSynthesizeViewBox() const; + gfxMatrix GetViewBoxTransform() const; - bool HasValidViewbox() const { return mViewBox.IsValid(); } // This services any pending notifications for the transform on on this root // node needing to be recalculated. (Only applicable in @@ -227,12 +250,6 @@ private: void ClearImageOverridePreserveAspectRatio(); const SVGPreserveAspectRatio* GetImageOverridePreserveAspectRatio() const; - // Returns true if we should synthesize a viewBox for ourselves (that is, - // if we're the outermost in an image document, and we're not currently - // being painted by an element). This method also assumes that we - // lack a valid viewBox attribute. - bool ShouldSynthesizeViewBox() const; - protected: // nsSVGElement overrides bool IsEventName(nsIAtom* aName); diff --git a/content/svg/content/src/nsSVGViewBox.h b/content/svg/content/src/nsSVGViewBox.h index 9d6c1436830..68a469973b8 100644 --- a/content/svg/content/src/nsSVGViewBox.h +++ b/content/svg/content/src/nsSVGViewBox.h @@ -71,8 +71,17 @@ public: void Init(); - // Used by element to tell if viewBox is defined - bool IsValid() const + /** + * Returns true if the corresponding "viewBox" attribute defined a rectangle + * with finite values. Returns false if the viewBox was set to an invalid + * string, or if any of the four rect values were too big to store in a + * float. + * + * This method does not check whether the width or height values are + * positive, so callers must check whether the viewBox rect is valid where + * necessary! + */ + bool IsExplicitlySet() const { return (mHasBaseVal || mAnimVal); } const nsSVGViewBoxRect& GetBaseValue() const diff --git a/layout/svg/base/src/nsSVGInnerSVGFrame.cpp b/layout/svg/base/src/nsSVGInnerSVGFrame.cpp index b2fc63b0412..c2611e96b78 100644 --- a/layout/svg/base/src/nsSVGInnerSVGFrame.cpp +++ b/layout/svg/base/src/nsSVGInnerSVGFrame.cpp @@ -134,7 +134,7 @@ nsSVGInnerSVGFrame::NotifySVGChanged(PRUint32 aFlags) if (!(aFlags & TRANSFORM_CHANGED) && (svg->mLengthAttributes[nsSVGSVGElement::X].IsPercentage() || svg->mLengthAttributes[nsSVGSVGElement::Y].IsPercentage() || - (svg->mViewBox.IsValid() && + (svg->HasViewBox() && (svg->mLengthAttributes[nsSVGSVGElement::WIDTH].IsPercentage() || svg->mLengthAttributes[nsSVGSVGElement::HEIGHT].IsPercentage())))) { @@ -175,7 +175,7 @@ nsSVGInnerSVGFrame::AttributeChanged(PRInt32 aNameSpaceID, aAttribute == nsGkAtoms::height) { nsSVGSVGElement* svg = static_cast(mContent); - if (svg->mViewBox.IsValid()) { + if (svg->HasViewBox()) { // make sure our cached transform matrix gets (lazily) updated mCanvasTM = nsnull; diff --git a/layout/svg/base/src/nsSVGOuterSVGFrame.cpp b/layout/svg/base/src/nsSVGOuterSVGFrame.cpp index 51e5c620ca8..e729bba5525 100644 --- a/layout/svg/base/src/nsSVGOuterSVGFrame.cpp +++ b/layout/svg/base/src/nsSVGOuterSVGFrame.cpp @@ -289,7 +289,7 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio() return ratio; } - if (content->mViewBox.IsValid()) { + if (content->HasViewBox()) { const nsSVGViewBoxRect viewbox = content->mViewBox.GetAnimValue(); float viewBoxWidth = viewbox.width; float viewBoxHeight = viewbox.height; diff --git a/layout/svg/base/src/nsSVGPatternFrame.cpp b/layout/svg/base/src/nsSVGPatternFrame.cpp index 7e1be50a761..2277d42187e 100644 --- a/layout/svg/base/src/nsSVGPatternFrame.cpp +++ b/layout/svg/base/src/nsSVGPatternFrame.cpp @@ -420,7 +420,7 @@ nsSVGPatternFrame::GetViewBox(nsIContent* aDefault) const nsSVGViewBox &thisViewBox = static_cast(mContent)->mViewBox; - if (thisViewBox.IsValid()) + if (thisViewBox.IsExplicitlySet()) return thisViewBox; AutoPatternReferencer patternRef(this); @@ -581,7 +581,7 @@ nsSVGPatternFrame::ConstructCTM(const gfxRect &callerBBox, } const nsSVGViewBox& viewBox = GetViewBox(); - if (!viewBox.IsValid()) { + if (!viewBox.IsExplicitlySet()) { return tCTM; } const nsSVGViewBoxRect viewBoxRect = GetViewBox().GetAnimValue(); diff --git a/layout/svg/base/src/nsSVGUtils.cpp b/layout/svg/base/src/nsSVGUtils.cpp index f4e84bbbd26..76280028c96 100644 --- a/layout/svg/base/src/nsSVGUtils.cpp +++ b/layout/svg/base/src/nsSVGUtils.cpp @@ -1711,20 +1711,6 @@ nsSVGUtils::PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents, // ---------------------------------------------------------------------- -/* static */ bool -nsSVGUtils::RootSVGElementHasViewbox(const nsIContent *aRootSVGElem) -{ - if (!aRootSVGElem->IsSVG(nsGkAtoms::svg)) { - NS_ABORT_IF_FALSE(false, "Expecting an SVG node"); - return false; - } - - const nsSVGSVGElement *svgSvgElem = - static_cast(aRootSVGElem); - - return svgSvgElem->HasValidViewbox(); -} - /* static */ void nsSVGUtils::GetFallbackOrPaintColor(gfxContext *aContext, nsStyleContext *aStyleContext, nsStyleSVGPaint nsStyleSVG::*aFillOrStroke, diff --git a/layout/svg/base/src/nsSVGUtils.h b/layout/svg/base/src/nsSVGUtils.h index b88e6638724..e0e0b1d0541 100644 --- a/layout/svg/base/src/nsSVGUtils.h +++ b/layout/svg/base/src/nsSVGUtils.h @@ -637,16 +637,6 @@ public: NS_MIN(double(PR_INT32_MAX), aVal))); } - /** - * Given a nsIContent* that is actually an nsSVGSVGElement*, this method - * checks whether it currently has a valid viewBox, and returns true if so. - * - * No other type of element should be passed to this method. - * (In debug builds, anything non- will trigger an abort; in non-debug - * builds, it will trigger a false return-value as a safe fallback.) - */ - static bool RootSVGElementHasViewbox(const nsIContent *aRootSVGElem); - static void GetFallbackOrPaintColor(gfxContext *aContext, nsStyleContext *aStyleContext, nsStyleSVGPaint nsStyleSVG::*aFillOrStroke,