I've rewritten the SVG backend code to allow for native and cross-platform renderers. The code is checked in on the minibranch 'SVG_20020806_BRANCH'. It modifies 152 files, introduces 105 new files and obsoletes 17 files. Two rendering backends are implemented so far: A Windows-native backend using GDI+ and a cross-platform backend based on libart. The code also introduces some new features, most notably support for svg text, printing and presentation attributes. Text is currently only implemented for the gdi+ backend, but it is being worked on for the libart cross-platform backend by Leon Sha of SUN. The code fixes many issues with the current trunk SVG, including bugs #101300, #110979, #111152, #111317, #122363, #124921, #133964, #137382, #141252, #148623, #148914, #150070, #150120, #160882 and #166469. I hope that eventually this code will be chedked in on the trunk. Until then, this is how to build: libart build (crossplatform) ---------------------------- Currently we only have makefiles for windows & unix, not for mac. You'll need a .mozconfig containing ac_add_options --enable-svg ac_add_options --enable-svg-renderer-libart mk_add_options MOZ_INTERNAL_LIBART_LGPL=1 MOZ_INTERNAL_LIBART_LGPL=1 Then check out the code with cvs checkout -r SVG_20020806_BRANCH mozilla/client.mk cd mozilla make -f client.mk checkout and build: make -f client.mk build gdiplus build (Windows >=98 only ) ---------------------------------- You need to obtain a recent version of the microsoft platform sdk; one that contains the gdiplus dll & headers. It can be downloaded from www.microsoft.com/msdownload/platformsdk/sdkupdate/. After installing set up your $MOZCONFIG file to contain --enable-svg --enable-svg-renderer-gdiplus --disable-activex Then pull the source: cvs checkout -r SVG_20020806_BRANCH mozilla/client.mk cd mozilla make -f client.mk checkout Now, optionally edit your mozilla/config/myconfig.mk file to contain DEFINES += -DSVG_FORCE_PIXEL_ACCESS_SURFACES This should improve SVG performance substantially while moderately impacting 'normal' Mozilla rendering. Then build: make -f client.mk build Finally, if you're not running on Windows XP you'll need to copy the 'gdiplus.dll' redistributable from the SDK into your dist/bin folder.

hmm, sounds like goodies... :) Any chance or roadmap to get this checked into trunk soon? (it seems the upcoming "alpha" would be an ideal point for getting it in...)

Some more comments: - I've uploaded Windows and Linux builds of this branch to http://www.croczilla.com/svg/ - The libart backend currently doesn't support foreignObject tags.

How do you handle devices which do not support an alpha channel ?

roland: Neither libart nor gdi+ need devices to have an alpha channel

Alex Fritze wrote: > Neither libart nor gdi+ need devices to have an alpha channel Is the "solution" still to try to render SVG as one large image (which isn't a good idea for printers which operate at huge resolutions like 6600x6600 pixels (=DIN-A4/600DPI)) ?

roland: yes, libart works in that way and I think gdi+ does behind the scenes as well. AFAIKS it is the only generic way of printing SVG and it works well for most situations.

Alex Fritze wrote: > AFAIKS it is the only generic way of printing SVG and it works well > for most situations. Mhhh, what about larger paper sizes ? Printing on DIN-A1 and DIN-A0 would result in a "problem" here for 32bit machines since the resulting image would be larger than the memory addressable by a 32bit pointer...

roland: Have you got any ideas on how to handle these cases in a generic way? Maybe banded printing? Or a specialised postscript rendering backend (for those rare cases where the user actually has a postscript printer)?

roland: I suppose we should continue the printing discussion on bug#133964. See my comments there.

how close to checkin is this - since libart now works this seems to be at parity with the existing SVG in the trunk - checking in would help prevent bitrot we spin off 1.3a on Dec4 getting this merged in the next days would be a big win

It is _nearly_ at parity. <ForeignObject> doesn't work for libart yet (that's the price we pay for getting bug#111152 fixed). Also there are no makefiles for Mac yet. Then there's the 'small' issue of getting the code reviewed. It's all not part of the default build, but still there are some changes to makefiles and the configure file that should be looked at carefully.

Nominating for 1.3.

Comment on attachment 113803 [details] [diff] [review] other cheges from trunk to branch drop this patch - not needed or duplicate on branch

What's the current status of this? If I checkout from that branch will I get a really old mozilla or is it merged with the current trunk?

the branch is a "partial" branch and is normally resynced at least once a week. if you have problems the client.mk on the branch has the "merge_svg" target that merges you local tree up to the tip of the main tree, this normally is all that is needed, but occasionally changes are made on the main tree that require some hacking on the branch. alex usually catches that anc checks in as needed

I'd like to get the branch landed on the trunk soon, so I've attached a few patches. The SVG code is all #ifdef MOZ_SVG'ed, so it should be sufficient to get only get shared files reviewed at this stage. The six patches above cover all modifications made on the SVG branch in the respective areas. All other changes are confined to content/svg, layout/svg and other-licenses/libart-lgpl. Now I just need to find some volunteers to review the patches and we should be ready to go...

Comment on attachment 137493 [details] [diff] [review] Changes to dom/ >Index: dom/public/nsIDOMClassInfo.h >@@ -238,7 +238,14 @@ > eDOMClassInfo_SVGPathSegCurvetoQuadraticSmoothRel_id, > eDOMClassInfo_SVGRect_id, > eDOMClassInfo_SVGAnimatedRect_id, >-#endif >+ eDOMClassInfo_SVGAnimatedLengthList_id, >+ eDOMClassInfo_SVGLengthList_id, >+ eDOMClassInfo_SVGNumber_id, >+ eDOMClassInfo_SVGTextElement_id, >+ eDOMClassInfo_SVGTSpanElement_id, >+ eDOMClassInfo_SVGAnimatedString_id, >+ eDOMClassInfo_SVGImageElement_id, >+#endif //MOZ_SVG Unfortunatly i think you need to add these to the end of the list to avoid breaking binary compatibility. Though I've never understood how we can have #ifdefs in that list without breaking binary compatibility. The RightThing is probably to remove the #ifdefs entierly and allocate the id's even for non-svg builds. If you do that you can proabably put all the svg-ids at the bottom of the list. r=me assuming you get peterv or jst to sr

Comment on attachment 137490 [details] [diff] [review] Changes to content/ hmm.. i think i'd be more comfortable having someone that knows the style-code look at this patch

Comment on attachment 137493 [details] [diff] [review] Changes to dom/ btw, we might want to use another scripthelper for things like lengthlists so that you can access members in them using [] in javascript. But that can be done separatly IMHO.

Updated patch incorporating Jonas's comments. I've moved all the SVG classinfo ids to the end of the lists, but kept them in #ifdef's for now.

Comment on attachment 137489 [details] [diff] [review] Changes to allmakefiles.sh/configure.in/autoconf.mk.in >Index: allmakefiles.sh >@@ -1215,6 +1215,11 @@ > layout/svg/Makefile > layout/svg/base/Makefile > layout/svg/base/src/Makefile >+ layout/svg/renderer/Makefile >+ layout/svg/renderer/public/Makefile >+ layout/svg/renderer/src/Makefile >+ layout/svg/renderer/src/gdiplus/Makefile >+ layout/svg/renderer/src/libart/Makefile Please be consisitent: one tab here, instead of spaces. >Index: configure.in > MOZ_ARG_ENABLE_BOOL(svg, >-[ --enable-svg Enable SVG support], >+[ --enable-svg Enable SVG support ], > MOZ_SVG=1, > MOZ_SVG= ) > if test -n "$MOZ_SVG"; then >- AC_DEFINE(MOZ_SVG) >+ AC_DEFINE(MOZ_SVG) > fi >+AC_SUBST(MOZ_SVG) Please leave the AC_SUBST where it was at the end. We try to group them all together. Is there any reason we need the extra space in the title? >+ >+MOZ_ARG_ENABLE_BOOL(svg-renderer-gdiplus, >+[ --enable-svg-renderer-gdiplus Enable SVG gdi+ renderer ], >+ MOZ_SVG_RENDERER_GDIPLUS=1, >+ MOZ_SVG_RENDERER_GDIPLUS= ) >+if test -n "$MOZ_SVG_RENDERER_GDIPLUS"; then >+ AC_DEFINE(MOZ_SVG_RENDERER_GDIPLUS) >+fi >+AC_SUBST(MOZ_SVG_RENDERER_GDIPLUS) >+ >+MOZ_ARG_ENABLE_BOOL(svg-renderer-libart, >+[ --enable-svg-renderer-libart Enable SVG libart renderer ], >+ MOZ_SVG_RENDERER_LIBART=1, >+ MOZ_SVG_RENDERER_LIBART= ) >+if test -n "$MOZ_SVG_RENDERER_LIBART"; then >+ AC_DEFINE(MOZ_SVG_RENDERER_LIBART) >+fi >+AC_SUBST(MOZ_SVG_RENDERER_LIBART) Is there any situation where you would build both of these? if not, we should use one configure option instead of two: MOZ_ARG_ENABLE_STRING(svg-renderer, [ --enable-svg-renderer=gdiplus|libart select the SVG rendering library], [MOZ_SVG_RENDERER_LBIRARY=`echo $withval | tr A-Z a-z`]) if test -z "$MOZ_CHROME_FILE_FORMAT"; then MOZ_CHROME_FILE_FORMAT=jar fi if test "$MOZ_CHROME_FILE_FORMAT" != "gdiplus" && test "$MOZ_CHROME_FILE_FORMAT" != "libart"; then AC_MSG_ERROR([--enable-svg-renderer must be set to either gdiplus or libart]) fi Also, please add a platform test so that we don't enable gdiplus on non-windows platforms.

bsmergberg: I guess the MOZ_CHROME_FILE_FORMAT in your post above is a copy/paste error ;-) Just wanted to note it before someone might take your lines and simply copy/paste them without checking if they're right ;-)

Updated patch incorporating Benjamin's comments: 1. Whitespace in allmakefiles.sh should now be consistent. 2. I've moved all svg-related AC_SUBSTs to the list towards the end of configure.in 3. I've left the renderers as 2 separate options that can be built at the same time. I'm aiming for some sort of user preferences-driven selection of rendering backend, maybe using different backends for different output devices (screen, printer)... 4. I've added a test checking for the Gdiplus.h header.

Comment on attachment 137490 [details] [diff] [review] Changes to content/ >+const nsStyleStruct* >+nsRuleNode::ComputeSVGResetData(nsStyleStruct* aStartStruct, const nsCSSStruct& aData, The second parameter should be |const nsRuleDataStruct& aData|. >Index: content/base/src/nsStyleContext.cpp > (int)svg->mFill.mType, > svg->mFillOpacity); > fprintf(out, "\" />\n"); There are some additional properties in the SVG struct that you should print. >Index: content/shared/public/nsRuleNode.h >+ const nsStyleStruct* ComputeSVGResetData(nsStyleStruct* aStartSVG, const nsCSSStruct& aSVGData, >+ nsStyleContext* aContext, >+ nsRuleNode* aHighestNode, >+ const RuleDetail& aRuleDetail, PRBool aInherited); Likewise (comment before last). >Index: content/shared/src/nsCSSProps.cpp >=================================================================== >RCS file: /cvsroot/mozilla/content/shared/src/nsCSSProps.cpp,v >retrieving revision 3.92 >retrieving revision 3.69.8.23 >diff -u -r3.92 -r3.69.8.23 >--- content/shared/src/nsCSSProps.cpp 30 Oct 2003 01:50:59 -0000 3.92 >+++ content/shared/src/nsCSSProps.cpp 16 Nov 2003 07:46:58 -0000 3.69.8.23 >@@ -865,12 +865,50 @@ > > #ifdef MOZ_SVG > // keyword tables for SVG properties >+ >+const PRInt32 nsCSSProps::kDominantBaselineKTable[] = { >+ eCSSKeyword_text_top, NS_STYLE_DOMINANT_BASELINE_TEXT_TOP, >+ eCSSKeyword_text_bottom, NS_STYLE_DOMINANT_BASELINE_TEXT_BOTTOM, where did these values come from? >Index: content/shared/src/nsStyleStruct.cpp >+PRInt32 This should return nsChangeHint (header file too). >+nsStyleSVGReset::CalcDifference(const nsStyleSVGReset& aOther) const I'm skeptical (here and for nsStyleSVG) that everything should return NS_STYLE_HINT_VISUAL. r=dbaron with those changes and an explanation of where those extra value came from

Updated patch incorporating David's changes: 1. Corrected signatures of ComputeSVGResetData(.) in nsRuleNode.{h,cpp}. (I'm surprised that my compiler didn't give me a warning here!). 2. Printing all SVG properties in nsStyleContext::DumpRegressionData(.) in nsStyleContext.cpp. 3. About eCSSKeyword_text_top, ..., eCSSKeyword_text_bottom ... : I presume your question is why these constants are commented out in nsCSSKeywordList.h? This is because they are already defined outside of the ifdef'ed SVG section in nsCSSKeywordList. I wanted to keep them in the ifdef'ed part just for completeness until the svg keys get spliced into the list properly. 4. In nsStyleStruct.{h,cpp}: Corrected signature of nsStyleSVGReset::CalcDifference(.). 5. I'm not sure whether SVG changes should return NS_STYLE_HINT_VISUAL or NS_STYLE_HINT_NONE. None of the changes cause framechanges or reflows and SVG doesn't use the nsIFrame::AttributeChanged() mechanism at all. (I think there was a reason for returning NS_STYLE_HINT_VISUAL, but I can't remember...).

> 3. About eCSSKeyword_text_top, ..., > eCSSKeyword_text_bottom ... : My question was about why they're there. I don't see them in the SVG spec. If they're not in a spec, they should be prefixed with -moz-. > 5. I'm not sure whether SVG changes should return NS_STYLE_HINT_VISUAL or > NS_STYLE_HINT_NONE. How are dynamic changes to these properties handled? If a dynamic change to these properties needs to cause a repaint, then you should return NS_STYLE_HINT_VISUAL. If it needs to cause a reflow, then NS_STYLE_HINT_REFLOW, etc. I find it hard to believe that you don't need a reflow for some of these properties. (Do you have testcases for dynamic changes to these properties?)

Comment on attachment 137497 [details] [diff] [review] Changes to layout/ >Index: layout/base/public/nsStyleConsts.h >+// Some of our constants must map to the same values as those defined in >+// nsISVG{,Path,Glyph}GeometrySource.idl/ >+// I don't want to add a dependency on the SVG module >+// everywhere by #include'ing nsISVG{,Path,Glyph}GeometrySource.h, so these consts >+// have to be kept in sync manually. You don't need to introduce a header dependency -- you can just define the constants as nsISVGGeometrySource::FOO and require anybody who uses the constants to include the appropriate header. Either way is fine with me, though. >Index: layout/html/base/src/nsFrameManager.cpp >+#ifdef MOZ_SVG >+ nsCOMPtr<nsISVGContent> svg(do_QueryInterface(aContent)); >+ if (svg) { >+ svg->IsPresentationAttribute(aAttribute, aResult); >+ if (*aResult) >+ return NS_OK; >+ } >+#endif There ought to be a more appropriate place for this. How do these atttributes influence style? Isn't there a stylesheet object somewhere in whose implementation of HasAttributeDependentStyle this belongs? Also, this will have merge conflicts with my patch in bug 15608. >Index: layout/html/style/src/nsCSSFrameConstructor.cpp I'm not wild about the changes to ConstructTextFrame, but it's probably simpler than writing a custom ProcessChildren.

The only properties (css or dom-attributes) that should need to cause a reflow are ones that affect the size of the image itself. All other changes happen in the existing imageframe, just as for gif-animations.

> 3. About eCSSKeyword_text_top, ..., > eCSSKeyword_text_bottom ... : These were valid values for 'dominant-baseline' in the SVG 1.0 standard, but it looks like they have since been removed in SVG 1.1 to make 'dominant-baseline' "consistent with XSL definitions".I've removed them from our implementation now. Affected files: content/shared/src/nsCSSProps.cpp, content/shared/public/nsCSSKeywordList.h, layout/svg/base/src/nsSVGTextFrame.cpp. > How are dynamic changes to these properties handled? SVG implements its own observer-based notification system to handle dynamic changes to attributes/dom properties. As a typical example let's e.g. assume a change to the "r" (radius) attribute of a <circle> element. The nsSVGCircleElement object, has a child "mR" of type nsSVGAnimatedLength. This child holds the state of the "r" attribute in native form and implements the required interfaces for the SVG DOM, in this case 'nsIDOMSVGAnimatedLength', so that it can be accessed as something like circle.r.baseVal.value = 5. nsSVGAnimatedLength also implements the nsISVGValue interface which defines facilities for setting/getting the attribute's value as a string and facilities for adding/removing observers (of type nsISVGValueObserver). A call such as circle.setAttribute("r",5) will ultimately feed through to mR's SetValueString(). This method, in turn, will inform all observers of the value change with a call to nsISVGValueObserver::WillModify() and nsISVGValueObserver::DidModify(). Directly setting the value through the SVG DOM (with a call such as circle.r.baseVal.value=5) has the same effect, only the conversion from string to native value is not performed. The circle element's corresponding frame (nsSVGCircleFrame) sets itself up as a listener to mR when constructed. When the frame is informed of a change in mR through a call to nsSVGCircleFrame::DidModifySVGObservable, it updates its associated (renderer-specific) path information, collecting information about the areas that need to be redrawn from the renderer. Finally, the frame informs the outer svg frame (nsSVGOuterSVGFrame) of the dirty areas. The outer svg frame will then call UpdateView() for these areas on its associated view. (This is somewhat simplified - there is also a mechanism for batching attribute changes) > If a dynamic change to > these properties needs to cause a repaint, then you should return > NS_STYLE_HINT_VISUAL. If it needs to cause a reflow, then > NS_STYLE_HINT_REFLOW, etc. Most svg attribs work in the way described above, and on further reflection I think they should return NS_STYLE_HINT_NONE, since afaics all that NS_STYLE_HINT_VISUAL does is to call UpdateView(), which for SVG is done by the outer svg frame. > I find it hard to believe that you don't need a > reflow for some of these properties. As Jonas says, the only attributes that require reflow are those that affect the size of the outer svg element (i.e. "width" and "height", but *only* in the context of an *outer* svg element). The way this is handled at the moment is by the outer svg element creating a reflow command and posting it with mPresShell->AppendReflowCommand(). > (Do you have testcases for dynamic > changes to these properties?) Not formal ones, but some of the examples on croczilla.com are dynamic (e.g. http://www.croczilla.com/svg/xbl-shapes.xml or http://www.croczilla.com/svg/xbl1.xml or http://www.croczilla.com/svg/svgtetris.xml).

> How are dynamic changes to these properties handled? Please ignore my reply in comment 37. I got confused between SVG style properties and non-style properties, describing how the latter work in SVG, which was of course irrelvant in the context of your comment. At the moment style properties are simply handled by updating a frame's associated path data when DidSetStyleContext() is being called, e.g.: NS_IMETHODIMP nsSVGPathGeometryFrame::DidSetStyleContext(nsIPresContext* aPresContext) { // XXX: we'd like to use the style_hint mechanism and the // ContentStateChanged/AttributeChanged functions for style changes // to get slightly finer granularity, but unfortunately the // style_hints don't map very well onto svg. Here seems to be the // best place to deal with style changes: UpdateGraphic(nsISVGGeometrySource::UPDATEMASK_ALL); return NS_OK; } This is of course not an optimal solution, but it's the way in which it already works in the trunk SVG code now. Again, I haven't got any formal testcases, but some of the examples on croczilla.com have dynamic style (e.g. http://www.croczilla.com/svg/polygons3.xml or http://www.croczilla.com/svg/circles2.xml). >>Index: layout/base/public/nsStyleConsts.h >>+// Some of our constants must map to the same values as those defined in >>+// nsISVG{,Path,Glyph}GeometrySource.idl/ >>+// I don't want to add a dependency on the SVG module >>+// everywhere by #include'ing nsISVG{,Path,Glyph}GeometrySource.h, so these >consts >>+// have to be kept in sync manually. > >You don't need to introduce a header dependency -- you can just define the >constants as nsISVGGeometrySource::FOO and require anybody who uses the >constants to include the appropriate header. Either way is fine with me, >though. Since you don't mind it's probably easiest if we keep the code as it is for now. The constants are currently used by nsCSSProps.cpp and nsStyleStruct.cpp, so we would need to include the appropriate headers there and add the appropriate include path to the corresponding makefiles... >>Index: layout/html/base/src/nsFrameManager.cpp >>+#ifdef MOZ_SVG >>+ nsCOMPtr<nsISVGContent> svg(do_QueryInterface(aContent)); >>+ if (svg) { >>+ svg->IsPresentationAttribute(aAttribute, aResult); >>+ if (*aResult) >>+ return NS_OK; >>+ } >>+#endif > >There ought to be a more appropriate place for this. How do these atttributes >influence style? Isn't there a stylesheet object somewhere in whose >implementation of HasAttributeDependentStyle this belongs? Yeah, there has to be a better way of doing this. Style-affecting SVG attribs such as "stroke-width" get mapped into a content style rule which gets picked up in nsSVGElement::WalkContentStyleRules(). For this to work dynamically we need to identify these attributes as influencing style in some HasAttributeDependentStyle() method. We haven't got any SVG stylesheets, hence the test in nsFrameManager. I don't know the style system very well at all, so I'd appreciate any pointers as to how to implement this correctly. >>Index: layout/html/style/src/nsCSSFrameConstructor.cpp > >I'm not wild about the changes to ConstructTextFrame, but it's probably simpler >than writing a custom ProcessChildren. I think a custom ProcessChildren wouldn't be enough, since frames might get inserted dynamically in which case we'd end up in the regular ConstructFrame/ConstructTextFrame again if there's no special casing somewhere.

I changed the occurrences of $(NSNULL) in layout/build/Makefile.in to $(NULL). Thanks to biesi for spotting this. Also removed the comment line in nsCSSFrameConstructor.cpp: //XXX uh oh. the block we need to reframe has no parent!"); and left in the equivalent NS_ERROR() instead. SVG code occasionally hits this error when manipulating content in iframes in foreignObject tags IIRC. New patch coming shortly.

Comment on attachment 140699 [details] [diff] [review] Changes to layout >Index: layout/html/base/src/nsFrameManager.cpp >+#ifdef MOZ_SVG >+#include "nsISVGContent.h" >+#endif This file is not in the content or layout diffs.... I assume you didn't bother to attach changes to svg-only code? >+ svg->IsPresentationAttribute(aAttribute, &isPresAttr); Like dbaron said, we really want to make this happen "naturally" and kill this ifdef. Please talk to sicking about attribute mapping and sharing the generic attribute implementation, ok? sr=bzbarsky on the layout changes.

(In reply to comment #41) > (From update of attachment 140699 [details] [diff] [review]) > >Index: layout/html/base/src/nsFrameManager.cpp > >+#ifdef MOZ_SVG > >+#include "nsISVGContent.h" > >+#endif > > This file is not in the content or layout diffs.... I assume you didn't bother > to attach changes to svg-only code? No, the patches would have become too chunky and the thinking was that only the changes to non-svg-exclusive files could affect stability of non-svg builds. At some point, of course, (definitly before this code has any change of being switched on by default), the whole svg code will need to get reviewed. > >+ svg->IsPresentationAttribute(aAttribute, &isPresAttr); > > Like dbaron said, we really want to make this happen "naturally" and kill this > ifdef. I'm really going to need help from you content/layout-savvy people, so the best way to proceed is probably if I open a specific bug on this after the branch lands. > Please talk to sicking about attribute mapping and sharing the generic > attribute implementation, ok? Yeah, I am doing that already. > sr=bzbarsky on the layout changes. > Thanks! 1 "r" & 2 "sr"s to go! We might just make it for 1.7!

Comment on attachment 137858 [details] [diff] [review] Changes to content/ >Index: content/base/src/nsRuleNode.cpp >+nsRuleNode::ComputeSVGResetData(nsStyleStruct* aStartStruct, >+ parentSVGReset = NS_STATIC_CAST(const nsStyleSVGReset*, >+ parentContext->GetStyleData(eStyleStruct_SVGReset)); Shouldn't nsStyleContext have a GetStyleSVGReset() method that returns a |const nsStyleSVGReset*|? You want to use that here. >Index: content/base/src/nsStyleContext.cpp >+ const nsStyleSVGReset* svgReset = (const nsStyleSVGReset*)GetStyleData(eStyleStruct_SVGReset); Same. sr=bzbarsky with those two changes.

Comment on attachment 137494 [details] [diff] [review] Changes to gfx/ sr=bzbarsky on this too, so once it gets review you can land this....

Comment on attachment 137494 [details] [diff] [review] Changes to gfx/ r=tor, but keep in mind that we hope/plan to remove the freetype code in the future.

The branch has now landed, so marking FIXED. Thanks to all reviewers for the quick reviews and to David Avery for helping to keep this branch going for so long.

I filed bug 233370 on the attribute-mapping code.

On the monkeypox LinuxPPC tinderbox, I disabled SVG because of: /builds/tinderbox/SeaMonkey/Linux_2.2.15pre3_Depend/mozilla/layout/svg/base/src/nsSVGOuterSVGFrame.cpp:313: #error "No SVG renderer."

The AIX tinderbox (laredo) on Seamonkey-Ports is failing with: art_affine.c "/usr/include/math.h", line 236.9: 1506-213 (S) Macro name M_PI cannot be redefined. "/usr/include/math.h", line 242.9: 1506-213 (S) Macro name M_SQRT2 cannot be redefined. In the directory /home/tbox/builds/tinderbox/AIX_5.1_Clobber/mozilla/obj-tinder/other-licenses/libart_lgpl The following command failed to execute properly: xlc_r -o art_affine.o -c -DOSTYPE="AIX5.1" -DOSARCH="AIX" -DLIBART_COMPILATION -I../../dist/include/libart_lgpl -I../../dist/include -I/home/tbox/builds/tinderbox/AIX_5.1_Clobber/mozilla/obj-tinder/dist/include/nspr -qflag=w:w -DNDEBUG -DTRIMMED -O2 -qmaxmem=-1 -qalias=noansi -DAIX=1 -DHAVE_SYS_INTTYPES_H=1 -DNSCAP_DISABLE_DEBUG_PTR_TYPES=1 -DD_INO=d_ino -DSTDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGINFO_T=1 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_INT64=1 -DHAVE_UINT=1 -DHAVE_UINT_T=1 -DHAVE_UINT16_T=1 -DHAVE_DIRENT_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_MALLOC_H=1 -DHAVE_X11_XKBLIB_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATFS_H=1 -DHAVE_LIBC_R=1 -DHAVE_LIBM=1 -DHAVE_LIBDL=1 -DHAVE_LIBC_R=1 -DFUNCPROTO=15 -DHAVE_XSHM=1 -D_REENTRANT=1 -DHAVE_RANDOM=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_NL_LANGINFO=1 -DHAVE_FLOCKFILE=1 -DHAVE_LOCALTIME_R=1 -DHAVE_STRTOK_R=1 -DHA! VE_RES_NINI T=1 -DHAVE_I18N_LC_MESSAGES=1 -DMOZ_DEFAULT_TOOLKIT="gtk" -DMOZ_WIDGET_GTK=1 -DMOZ_ENABLE_XREMOTE=1 -DMOZ_X11=1 -DMOZ_ENABLE_COREXFONTS=1 -DMOZ_EXTRA_X11CONVERTERS=1 -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DMOZ_MATHML=1 -DMOZ_SVG=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=".mozilla" -DMOZ_XUL=1 -DMOZ_PROFILESHARING=1 -DMOZ_PROFILELOCKING=1 -DSUNCTL=1 -DMOZ_BYPASS_PROFILE_AT_STARTUP=1 -DMOZ_DLL_SUFFIX=".so" -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DJS_THREADSAFE=1 -DNS_PRINT_PREVIEW=1 -DNS_PRINTING=1 -DMOZILLA_VERSION="1.7a" -DMOZILLA_LOCALE_VERSION="1.7a" -DMOZILLA_REGION_VERSION="1.7a" -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /home/tbox/builds/tinderbox/AIX_5.1_Clobber/mozilla/other-licenses/libart_lgpl/art_affine.c gmake[3]: *** [art_affine.o] Error 1 I guess on monkeypox and otaku what I should really do is add: mk_add_options MOZ_INTERNAL_LIBART_LGPL=1 MOZ_INTERNAL_LIBART_LGPL=1

Er, they already had that, but I added --enable-svg-renderer-libart . Should have read comment 0, although it would have been nice to have a heads up that we'd need tinderbox configuration changes for the machines building SVG.

While the includes in art_affine.c mentioned in comment 49 were reordered, the same problem is now happening in another file. Isn't the real problem that art_misc.h does: #ifndef M_PI #define M_PI 3.14159265358979323846 without including math.h? Shouldn't art_misc.h just include math.h?

(In reply to comment #51) > While the includes in art_affine.c mentioned in comment 49 were reordered, the > same problem is now happening in another file. Isn't the real problem that > art_misc.h does: > > #ifndef M_PI > #define M_PI 3.14159265358979323846 > > without including math.h? Shouldn't art_misc.h just include math.h? Done.