(From update of attachment 236868)
My 2c, for what its worth.
I think my fundamental structural concern is that this does not really use either the GetNextGlyphFragmentChildNode or GetNextGlyphFragment to traverse the fragment trees using functions defined in the node/fragment interfaces and I think, if possible it should. I'm not absolutely sure which one you want. The first is recursive and skips nsSVGGlyphFrames, the second traverses rather than recurses and includes nsSVGGlyphFrames. I see signs of both in the code below.
I'm not sure why you implemented this here rather than using the implementation in nsSVGGlyphFrame.cpp. If you are going to change where it lives, I would have thought it made more sense to make this a member function of nsSVGTextContainerFrame it would not need any arguments then.
>- {
>- nsCOMPtr<nsIDOMSVGLengthList> list = GetX();
>- GetSingleValue(firstFragment, list, &x);
>+/*
>+ * Performs a pre-order traversal on the subtree rooted at “node” until an
>+ * absolutely positioned node is found or until the entire subtree has been
>+ * explored. Sum is increased by the total advance of all the nodes visited.
>+ * Returns PR_TRUE if the subtree contained an absolutely positioned node.
>+ */
>+PRBool
>+GetSubTreeGlyphAdvance(nsIFrame* node,
>+ nsSVGTextPathFrame** textPath,
>+ float *sum)
>+{
>+ if (!node)
>+ return PR_FALSE;
>+
>+ if (node->GetType() == nsLayoutAtoms::svgTextPathFrame) {
>+ *textPath = NS_STATIC_CAST(nsSVGTextPathFrame*, node);
>+ } else if (node->GetType() == nsLayoutAtoms::svgTSpanFrame) {
>+ nsSVGTextContainerFrame *cf = NS_STATIC_CAST(nsSVGTextContainerFrame*,node);
>+
>+ float dx = 0;
>+ nsCOMPtr<nsIDOMSVGLengthList> list = cf->GetDx();
>+ GetSingleValue(*textPath, list, &dx);
>+ *sum += dx;
>+
>+ } else if (node->GetType() == nsLayoutAtoms::svgGlyphFrame) {
>+ nsCOMPtr<nsISVGGlyphFragmentLeaf> fragment = do_QueryInterface(node);
>+ *sum += fragment->GetAdvance();
> }
>- {
>- nsCOMPtr<nsIDOMSVGLengthList> list = GetY();
>- GetSingleValue(firstFragment, list, &y);
>+
>+ nsIFrame* nextNode = node->GetFirstChild(nsnull);
Could you work with GetFirstGlyphFragmentChildNode and GetNextGlyphFragmentChildNode. Can GetSubTreeGlyphAdvance be part of the nsSVGGlyphFragmentNode interface?
>+/*
>+ * Returns the total advancement for all glyphs between “node” and the next
>+ * absolutely positioned node.
>+ */
>+float
>+GetChunkLength(nsIFrame* node, nsSVGTextPathFrame* textPath, nsIFrame* root)
>+{
>+ float sum = 0;
>+ while (node) {
>+ if (GetSubTreeGlyphAdvance(node, &textPath, &sum)) {
>+ return sum;
>+ //The subtree rooted at NODE doesn't contain any absolutely positioned nodes
>+ if (node->GetType() == nsLayoutAtoms::svgTextPathFrame)
>+ textPath = nsnull;
>+
>+ nsIFrame* nextNode = node->GetNextSibling();
>+ if (!nextNode) {
>+ //The entire subtree rooted at NODE's parent has been explored. Go up
>+ //the tree until a parent with siblings is found or until we reach the
>+ //root textFrame element
This kind of navigation suggests you should be traversing with GetNextGlyphFragment and working with glyph fragments.
>+ /*
>+ * Performs a pre-order traversal on the subtree rooted at “node” and updates
>+ * the position of each glyph fragment as its svgGlyphFrame is visited.
>+ * node – may be any type of frame that can be contained within an
>+ * svgTextFrame
>+ * textPath – node's most recent ancestor of type svgTextPathFrame. nsnull
>+ * if no such ancestor exists.
>+ * fragment - If node has an ancestor of type svgTextPathFrame, then fragment
>+ * is the most recent glyph fragment to have drawn at least 1
>+ * character onto the path.
Shouldn't this function argument be an ordinary pointer rather than an XPCOM thing?
>+ * x, y – absolute coordinates containing the current text position.
>+ */
>+ void UpdateGlyphPositioningHelper(nsIFrame* node,
>+ nsSVGTextPathFrame *textPath,
>+ nsCOMPtr<nsISVGGlyphFragmentLeaf> *fragment,
>+ float *x, float *y);
(From update of attachment 236868)
My 2c, for what its worth.
I think my fundamental structural concern is that this does not really use either the GetNextGlyphFra gmentChildNode or GetNextGlyphFra gment to traverse the fragment trees using functions defined in the node/fragment interfaces and I think, if possible it should. I'm not absolutely sure which one you want. The first is recursive and skips nsSVGGlyphFrames, the second traverses rather than recurses and includes nsSVGGlyphFrames. I see signs of both in the code below.
> *val = data->GetLength ()*percent/ 100.0f; >GetContent( )->HasAttr( kNameSpaceID_ None, >GetContent( )->HasAttr( kNameSpaceID_ None, :pathLength) ) { nsIDOMSVGPathEl ement> pathElement = nsIDOMSVGPathEl ement> pathElement = ce(pathFrame- >GetContent( ));
>- } else if (pathFrame-
>+ } else if (pathFrame-
> nsGkAtoms:
>- nsCOMPtr<
>+ nsCOMPtr<
> do_QueryInterfa
These are the same to my eye. Could you have inserted DOS style LF/CR into these lines?
> pathLength- >GetAnimVal( &pl);
>- if (pl)
>+ if (pl)
> *val *= data->GetLength() / pl;
>- else
>+ else
> *val = 0;
As above.
>+PRBool ositioned( nsIFrame* frame)
>+IsAbsolutelyP
I'm not sure why you implemented this here rather than using the implementation in nsSVGGlyphFrame .cpp. If you are going to change where it lives, I would have thought it made more sense to make this a member function of nsSVGTextContai nerFrame it would not need any arguments then.
>- { nsIDOMSVGLength List> list = GetX(); firstFragment, list, &x); phAdvance( nsIFrame* node, ame** textPath, :svgTextPathFra me) { CAST(nsSVGTextP athFrame* , node); :svgTSpanFrame) { nerFrame *cf = NS_STATIC_ CAST(nsSVGTextC ontainerFrame* ,node); nsIDOMSVGLength List> list = cf->GetDx(); *textPath, list, &dx); :svgGlyphFrame) { nsISVGGlyphFrag mentLeaf> fragment = do_QueryInterfa ce(node) ; >GetAdvance( ); nsIDOMSVGLength List> list = GetY(); firstFragment, list, &y); hild(nsnull) ;
>- nsCOMPtr<
>- GetSingleValue(
>+/*
>+ * Performs a pre-order traversal on the subtree rooted at “node” until an
>+ * absolutely positioned node is found or until the entire subtree has been
>+ * explored. Sum is increased by the total advance of all the nodes visited.
>+ * Returns PR_TRUE if the subtree contained an absolutely positioned node.
>+ */
>+PRBool
>+GetSubTreeGly
>+ nsSVGTextPathFr
>+ float *sum)
>+{
>+ if (!node)
>+ return PR_FALSE;
>+
>+ if (node->GetType() == nsLayoutAtoms:
>+ *textPath = NS_STATIC_
>+ } else if (node->GetType() == nsLayoutAtoms:
>+ nsSVGTextContai
>+
>+ float dx = 0;
>+ nsCOMPtr<
>+ GetSingleValue(
>+ *sum += dx;
>+
>+ } else if (node->GetType() == nsLayoutAtoms:
>+ nsCOMPtr<
>+ *sum += fragment-
> }
>- {
>- nsCOMPtr<
>- GetSingleValue(
>+
>+ nsIFrame* nextNode = node->GetFirstC
Could you work with GetFirstGlyphFr agmentChildNode and GetNextGlyphFra gmentChildNode. Can GetSubTreeGlyph Advance be part of the nsSVGGlyphFragm entNode interface?
>+ while (nextNode) { sitioned( nextNode) || Advance( nextNode, textPath, sum)) >GetNextSibling ();
>+ if (IsAbsolutelyPo
>+ GetSubTreeGlyph
>+ return PR_TRUE;
>+ nextNode = nextNode-
> }
>+ return PR_FALSE;
>+}
>
>+/* h(nsIFrame* node, nsSVGTextPathFrame* textPath, nsIFrame* root) hAdvance( node, &textPath, &sum)) { :svgTextPathFra me) bling() ;
>+ * Returns the total advancement for all glyphs between “node” and the next
>+ * absolutely positioned node.
>+ */
>+float
>+GetChunkLengt
>+{
>+ float sum = 0;
>+ while (node) {
>+ if (GetSubTreeGlyp
>+ return sum;
>+ //The subtree rooted at NODE doesn't contain any absolutely positioned nodes
>+ if (node->GetType() == nsLayoutAtoms:
>+ textPath = nsnull;
>+
>+ nsIFrame* nextNode = node->GetNextSi
>+ if (!nextNode) {
>+ //The entire subtree rooted at NODE's parent has been explored. Go up
>+ //the tree until a parent with siblings is found or until we reach the
>+ //root textFrame element
This kind of navigation suggests you should be traversing with GetNextGlyphFra gment and working with glyph fragments.
>+ nsIFrame* parent = node->GetParent(); :svgTextPathFra me) >GetNextSibling (); >GetParent( );
>+ while (parent && node != root) {
>+ if (parent->GetType() == nsLayoutAtoms:
>+ textPath = nsnull;
>+ nextNode = parent-
>+ if (nextNode)
> break;
>+ parent = parent-
> }
> }
>+void e::UpdateGlyphP ositioningHelpe r(nsIFrame* node, nsISVGGlyphFrag mentLeaf> *fragment,
>+nsSVGTextFram
>+ nsSVGTextPathFrame *textPath,
>+ nsCOMPtr<
>+ float *x, float *y)
>+{
>+ if (node->GetType() == nsLayoutAtoms: :svgTextFrame || :svgTSpanFrame || :svgTextPathFra me)
>+ node->GetType() == nsLayoutAtoms:
>+ node->GetType() == nsLayoutAtoms:
Use CallQueryInterface and see if the node supports nsISVGTextConte ntMetrics instead of the above tests.
>+ { nerFrame *cf = NS_STATIC_ CAST(nsSVGTextC ontainerFrame* ,node);
>+ nsSVGTextContai
Nit: space after comma.
>+ if (node->GetType() == nsLayoutAtoms: :svgTextPathFra me) { CAST(nsSVGTextP athFrame* , node); nsIDOMSVGLength List> list = fragment->GetDx(); fragment, list, &dx); nsIDOMSVGLength List> list = cf->GetX(); textPath, list, x); nsIDOMSVGLength List> list = fragment->GetDy(); fragment, list, &dy); nsIDOMSVGLength List> list = cf->GetY(); textPath, list, y); nsIDOMSVGLength List> list = cf->GetDx(); textPath, list, &dx); nsIDOMSVGLength List> list = cf->GetDy(); textPath, list, &dy); VG()->mTextAnch or; TEXT_ANCHOR_ START && IsAbsolutelyPos itioned( node)) { node, textPath, this); TEXT_ANCHOR_ MIDDLE) TEXT_ANCHOR_ END) :svgGlyphFrame) { nsISVGGlyphFrag mentLeaf> curFrag = do_QueryInterfa ce(node) ; >GetBaselineOff set(GetBaseline ()); >SetGlyphPositi on(*x, *y - baseline_offset); >GetAdvance( ); >GetNumberOfCha rs() > 0) { >GetEndPosition OfChar( 0, &pt); >GetBaselineOff set(baseline) ; >SetGlyphPositi on(x + dx, y + dy - baseline_offset); hild(nsnull) ; tioningHelper( nextNode, textPath, fragment, x, y); >GetNextSibling (); >GetAdvance( ); >GetNextGlyphFr agment( ); >IsAbsolutelyPo sitioned( )) :svgTextPathFra me) { ->GetNumberOfCh ars(); ->GetEndPositio nOfChar( i, &pt);
>+ textPath = NS_STATIC_
>+ *fragment = nsnull;
>+ *x = *y = 0;
>+ } else {
> {
>- nsCOMPtr<
>- GetSingleValue(
>+ nsCOMPtr<
>+ GetSingleValue(
> }
> {
>- nsCOMPtr<
>- GetSingleValue(
>+ nsCOMPtr<
>+ GetSingleValue(
>+ }
>+ }
>+ {
>+ float dx = 0;
>+ nsCOMPtr<
>+ GetSingleValue(
>+ *x += dx;
>+ }
>+ {
>+ float dy = 0;
>+ nsCOMPtr<
>+ GetSingleValue(
>+ *y += dy;
>+ }
>+
>+ PRUint8 anchor = node->GetStyleS
>+ if (anchor != NS_STYLE_
>+ float chunkLength = GetChunkLength(
>+ if (anchor == NS_STYLE_
>+ *x -= chunkLength/2.0f;
>+ else if (anchor == NS_STYLE_
>+ *x -= chunkLength;
>+ }
>+ } else if (node->GetType() == nsLayoutAtoms:
>+ nsCOMPtr<
>+ float baseline_offset = curFrag-
>+ curFrag-
>+ *x += curFrag-
>+
>+ if (textPath && curFrag-
>+ nsIDOMSVGPoint* pt;
>+ curFrag-
>+ if (pt) {
>+ *fragment = curFrag;
> }
>+ }
>+ }
>
>- float baseline_offset = fragment-
>- fragment-
>+ nsIFrame* nextNode = node->GetFirstC
>+ while (nextNode) {
>+ UpdateGlyphPosi
>+ nextNode = nextNode-
>+ }
>
>- x += dx + fragment-
>- y += dy;
>- fragment = fragment-
>- if (fragment && fragment-
>- break;
>+ if (node->GetType() == nsLayoutAtoms:
>+ if (*fragment) {
>+ nsIDOMSVGPoint* pt;
>+ PRUint32 numChars = (*fragment)
>+ for (PRUint32 i = numChars - 1; i >= 0; --i) {
>+ (*fragment)
This is going to be very slow. I think you need a function to all the positions at once and return them in an array if you need to do this.
>+ if (pt) {
>+ pt->GetX(x);
>Index: layout/ svg/base/ src/nsSVGTextFr ame.h ======= ======= ======= ======= ======= ======= ======= ======= ===== mozilla/ layout/ svg/base/ src/nsSVGTextFr ame.h,v svg/base/ src/nsSVGTextFr ame.h 28 Jun 2006 15:23:40 -0000 1.1 svg/base/ src/nsSVGTextFr ame.h 5 Sep 2006 21:09:22 -0000 inerFrame. h" erver.h" rame.h"
>======
>RCS file: /cvsroot/
>retrieving revision 1.1
>diff -u -8 -p -r1.1 nsSVGTextFrame.h
>--- layout/
>+++ layout/
>@@ -37,16 +37,17 @@
> * ***** END LICENSE BLOCK ***** */
>
> #ifndef NS_SVGTEXTFRAME_H
> #define NS_SVGTEXTFRAME_H
>
> #include "nsSVGTextConta
> #include "nsISVGValueObs
> #include "nsWeakReference.h"
>+#include "nsSVGTextPathF
You only need class nsSVGTextPathFrame here.
>+ /*
>+ * Performs a pre-order traversal on the subtree rooted at “node” and updates
>+ * the position of each glyph fragment as its svgGlyphFrame is visited.
>+ * node – may be any type of frame that can be contained within an
>+ * svgTextFrame
>+ * textPath – node's most recent ancestor of type svgTextPathFrame. nsnull
>+ * if no such ancestor exists.
>+ * fragment - If node has an ancestor of type svgTextPathFrame, then fragment
>+ * is the most recent glyph fragment to have drawn at least 1
>+ * character onto the path.
Shouldn't this function argument be an ordinary pointer rather than an XPCOM thing?
>+ * x, y – absolute coordinates containing the current text position. tioningHelper( nsIFrame* node, nsISVGGlyphFrag mentLeaf> *fragment,
>+ */
>+ void UpdateGlyphPosi
>+ nsSVGTextPathFrame *textPath,
>+ nsCOMPtr<
>+ float *x, float *y);