80142 - get new svg code into cvs

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Alex Fritze]

I've started a rewrite of Mozilla's SVG code. See http://www.croczilla.com/svg 
for details.

The code touches the dom, content and layout modules. All modifications to 
existing Mozilla files have been put into #ifdef MOZ_SVG conditionals.
Also, the code needs libart for rendering (see bug #79312). So far it works on 
Windows and, thanks to Bradley Baetz, on Linux. Bradley has also already fixed 
numerous bugs and updated the code to the recent style system changes.

Comment 1

[Alex Fritze]

Attachment: zip of svg dom interfaces (mozilla/dom/public/idl/svg)

Comment 2

[Alex Fritze]

Attachment: svg dom diffs

Comment 3

[Alex Fritze]

Attachment: zip of new content module files

Comment 4

[Alex Fritze]

Attachment: content module diffs

Comment 5

[Alex Fritze]

Attachment: zip of new layout module files

Comment 6

[Alex Fritze]

Attachment: layout module diffs

Comment 7

[Alex Fritze]

All 6 (3 zip files / 3 diff files) are needed.

The dom module zip/patches are independent of the other files.

The content module zip/patches depend on the dom module zip/patches as well as 
on the layout module patches.

The layout module zip/patches depend on the other zips/patches as well as on 
libart (#79312). The layout-zip contains a complete replacement for the current 
mozilla/layout/svg - folder. The old folder needs to be deleted before 
unpacking the zip.

Comment 8

[Adam D. Moss]

This is so damn exciting.  Here, have a vote!

Comment 9

[Johnny Stenback (:jst)]

I had a look at the DOM diff and the new files, here's a couple of comments
about the DOM changes/additions, once you've addressed these issues feel free to
check these new files into the mozilla repository, they're not part of the build
so you can safely check them in, sr=jst if you like. Hold on with checking in
the makefile modifications for now.

Makefile.(in,win) files need to be cleaned up, be consistent in indentation,
don't use tab's, or if you do, use tabs consistently, if you break the DIRS line
into many lines in one file, do so in both files.

Be consistent with the indentation in the idl files, I see both 2 and 4 space
indentation used in the idl files, I'd encourage using 2 spaces for indentation
to match the other DOM interfaces.

If you include one (non SVG) nsIDOMxxx file, you don't need to include domstubs.idl.

What's the story behind nsIDOMSVGGElement (and nsIDOMSVGPolylineElement), why do
we need an empty interface in the SVG DOM?

No files can have names longer than 31 characters, the mac can't deal with
filenames longer than 31.

Why is this C++ block in nsIDOMSVGLength.idl?

%{C++
#include "nsString.h"
%}

The license headers should match exactly the ones found on mozilla.org, they
should not refere to Mozilla SVG n' such in the middle of the license if you're
placing these files under MPL (see the other DOM interfaces for examples). Below
the actual license header you're free to mention that these files are part of
the Mozilla SVG project. And mark yourself as the original author of these new
files (again, see the other DOM interfaces for examples)

template.idl, huh?

You need to add a MANIFEST_IDL file to dom/public/idl/svg, see other dom idl
directories for an example


Stupid style nit:

Make all IID's in the interface use lowercase hex number as the rest of the DOM
interfaces do.

If I'd write these interfaces I'd write:

  nsIDOMElement         getElementById(in DOMString elementId);

in stead of:

    nsIDOMElement         getElementById ( in DOMString elementId );

for consistency with the rest of the DOM interfaces...

Comment 10

[Bradley Baetz]

So, I'm going to attach a new patch, subject to the following assumptions/notes:

1. Given the discussion in bug 79312, we're going to require people
building/using SVG to install libart themselves. The CVS version. (we crash on
anything earlier due to a libart bug involving non-alpha images). Maybe we can
get Raph to release a new version (with that one .def change required for
windows)? At some point precompiled libs may be provided on either ftp.m.o
(subject to staff@m.o approval), or maybe somewhere else. This is probably a
bigger issue for mac, but I really don't know.

2. peterv has agreed to look at the Mac stuff, and hopefully get it both
building and running. Running is always a plus.

3. The patch which I'll attach in a bit doesn't have makefile.win changes for
layout/build, or for the change in include path. Or for content/svg/document
alex, can you supply those?

4. jst's nits have all been addressed or discussed with him (MI is why there are
those empty interfaces - its a contract thing)

5. This is a work in progress - apart from lots of stuff not being implemented,
at least the following needs to be done:
  - the DOM/classinfo stuff has to be reviewed to check that everything
implements everything which it should
  - the nsIImage we use needs to store alpha bits as well, else we do stuff
incorrectly (think group opacity, nested). We should possibly use the gfxI*
stuff instead.
  - the interface to the path stuff is bad, and needs to be redone to use the
DOM interfaces. (I think alex has that in his tree though)
  - runtime version check for libart, so we don't crash if running on an
unsupported version.
  - yes, there are tons of printfs. Work In Progress, and all that.
  - other stuff I can't quite recall at the moment

6. Alex has some more patches in his tree (he mentioned supporting SVG-based XBL
widgets to me). However, its becoming really difficult for us to merge 680K of
diffs though, since interdiff doesn't appear to like new files.

7. I grabbed libart.m4 directly from the libart sources, which are under the
LGPL. It appears that we've done the same thing for all the other .m4 files,
however, and all the .m4 files are identical except for the name of the actual
library (in fact, their headers mention that they are "shamlessly stolen" from
each other). They appear to be identical to the .m4 files which are installed by
the actual library packages. shaver, is this OK, license-wise?

The patches are all by Alex, except for the SVGDocument stuff and the unix build
stuff (and a few other minor changes to keep up with changes to the trunk) which
are mine. He wants cvs access, and since SVG isn't part of the default build, he
only needs one sr for that, IIRC. shaver? :)

I'm doing the diffs because I can use cvs add, and then get cvs -N diffs.

Everything except for some bits of the configure changes are in an ifdef
MOZ_SVG, so once the unix tinderboxes are upgraded (are there any windows ones
building SVG?), and it builds on mac, we'd like to get this in ASAP, although
its probably not going to make 0.9.1. shaver, is the lack of additional review
OK? (Not part of build, and all that)

If the dom demos don't work after you've applied and built, clobber dom and
rebuild. I think there are some dependancy problems somewhere...

Comment 11

[Bradley Baetz]

Attachment: hopefully I got everything...

Comment 12

[Bradley Baetz]

OK, I found out why the dom stuff didn't work always - see bug 81495.

Mini-diff for svg (description only, cause I'm lazy :-) put:

MODULE          = dom
XPIDL_MODULE    = dom_svg

in dom/public/idl/svg/Makefile.in, and

MODULE          = svg_doc

in content/svg/document/src/Makefile.in

Alex - please change that in the makefile.win's as well. Thanks.

Comment 13

[Adam D. Moss]

Woohoo, 17 votes!  More, more!

Comment 14

[mop]

Somebody should hack bugzilla so that this "bug" gets the highest priority

:D

Comment 15

[chris]

you need to request module owner review and perhaps super-reviews, to allow
checkin into cvs
See http://www.mozilla.org/get-involved.html in the section called 'fix bugs'

Comment 16

[Aaron Leventhal]

My vote for this bug is conditional upon serious consideration for SVG's 
accessibility features! http://www.w3.org/TR/SVG-access/. Has anyone taken a 
look?

Comment 17

[Alex Fritze]

I've updated the code to work with Hyatt's style changes and added a few new 
features. For details see http://www.croczilla.com/svg. A new set of 3 zip and 
3 diffs (POSIXLY_CORRECT) follows.

Unfortunately I've had no luck in merging in Bradley's document and linux stuff 
yet :-( These 680K patches are not very nice to handle...
Maybe we could get someone with write-access to volunteer to open a cvs branch 
to make things more manageable.

For getting <foreignObject> to work, I had to change the rendering backend to 
render to nsIDrawingSurface's instead of nsIImages (a better idea anyway). To 
get this to work, we need some more gfx functionality, see bugs
[#83631] Need ability to create drawing surfaces with given pixel format
and
[#83517] nsTransform2D's translation part can't be set in transformed coords.
Unfortunately there are only Windows patches available for these atm.

Comment 18

[Alex Fritze]

Attachment: new zip of svg dom interfaces (mozilla/dom/public/idl/svg)

Comment 19

[Alex Fritze]

Attachment: new svg dom diffs

Comment 20

[Alex Fritze]

Attachment: new zip of content module svg files (content/shared/ and content/svg)

Comment 21

[Alex Fritze]

Attachment: new content module diffs

Comment 22

[Alex Fritze]

Attachment: new zip of layout module svg files (layout/svg)

Comment 23

[Alex Fritze]

Attachment: new layout module diffs

Comment 24

[Alex Fritze]

Argh - some of the diffs ended up '-c' others '-u'. Sorry!
Again all 3 zips and all 3 diffs are needed. The old set of 6 files is now 
obsolete (not Bradley's patch though, which I still have to somehow merge in).  

You'll also need the patches from #83517 and #83631 and the Libart library
(#79312). To get libart to work with the latest code you need to add the lines 
  #include "art_bpath.h"
  #include "art_vpath_bpath.h"
to mozilla/modules/libart_lgpl/libart-incs.h and add art_bpath.h and 
art_vpath_bpath.h to the exports in the makefile.

Comment 25

[mop]

Is there any chance that this patch will make it into the 0.9.3 release? What's
the status?

Comment 26

[Nisheeth Ranjan]

r=nisheeth for the content module diffs after fixing the following minor nit:

- The indentation in one of the patches to nsCSSDeclaration.cpp is irregular
(Search for "3297,3323" in
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=38058 to find the patch)
I haven't gone through the new files in the content module zip.  I don't want to
hold up the check in of those files into the CVS repository, though.  Heikki
Toivonen is out on vacation this week.  He and I will divvy up the code review
of those files between us and attack it next week.
So, you please feel free to check in the content module diffs and new files.

Thanks a lot for all your hard work.  I checked out http://www.croczilla.com/svg
and the SVG in Mozilla screenshots look awesome!

Comment 27

[Bradley Baetz]

nisheeth - you've just r=d patches which don't apply to the current trunk any 
more :)

I have patches agaist the current trunk - I'm on vacation now, and I'll attach 
them when I get back. There were no major changes to the content stuff apart 
from getting it to actually apply, IIRC.

Comment 28

[Alex Fritze]

Sorry guys - the patches were indeed out of date. It's difficult to stay up-to-
date with Mozilla. I hope this code can get in soon...

A new set of zips/patches against a trunk snapshot of 11.07.01 follows.

Bug fixes / new features: (details at www.croczilla.com/svg)
- Corrected transformations for circles and paths. 
- Corrected stroke scaling 
- Fixed some relative coordinate bugs in <path>s.
- Implemented h, H, v and V path-commands. 
- Implemented shorthand (smooth) cubic path commands (s, S). 
- Implemented elliptical arc path-commands (a, A).  
- The svg-element's viewport property is set correctly now. 
- Fixed a bug in the redrawing when the style-attribute is changed by script. 
- Implemented translating and scaling for <foreignObject>s.

Comment 29

[Bradley Baetz (:bbaetz)]

And I was going to attach updated patches later today... I'll merge, and then do
so. Alex, if your patches are against the 11th, then you've probably missed
jst's latest content update. I'll deal with it.

Comment 30

[Alex Fritze]

Attachment: zip of svg dom interfaces, version 3

Comment 31

[Alex Fritze]

Attachment: svg dom diffs, version 3

Comment 32

[Alex Fritze]

Attachment: zip of content module svg files, version 3

Comment 33

[Alex Fritze]

Attachment: content module diffs , version 3

Comment 34

[Alex Fritze]

Attachment: zip of layout module svg files, version 3

Comment 35

[Alex Fritze]

Attachment: layout module diffs, version 3

Comment 36

[Bradley Baetz (:bbaetz)]

OK, I'm going to attach a diff of the previous set of alex's changes. This may
or may not build on windows, but probably doesn't. It includes the patch for bug
83517.

I _think_ have all the svg bits for unix. If it doesn't build, let me know.

I know that going to a .svg document will cause an infinite number of browser
windows to open. I lost the change to make that work, somewhere. Its a one line
fix, once I find it, I think. I just haven't had time.

I really want to get this on a branch ASAP, and then merge with alex's latest
patches, then get it building on the mac.

Note that you need cvs libart, or you will crash on startup. The version which
comes with gnome is not new enough.

Comment 37

[Bradley Baetz (:bbaetz)]

Attachment: 12 Jun version + changes. Will build on unix, assuming I didn't miss anything

Comment 38

[Roland Mainz]

Alex: Wanna take a look at bug 85803, please ?

Comment 39

[Bradley Baetz (:bbaetz)]

OK, so I got told not to attach 700K diffs to a bug.

This is now on a semi-branch, tag SVG_20010721_BRANCH, with alex's june source
drop, + some of my changes. It was much easier than I expected (except for that
svg file that somehow ended up on the trunk in mailnews....)

Note that this is a mini-branch. Not all files are tagged, because this will
probably be on the branch for a while. This means that sometimes it won't build,
if there are incompatible changes. This should hopefully be rare, though.
The list of what is tagged can be found in client.mk (I've also tagged
client.mak, although haven't changed it)

I decided to do it this way because this branch may be reasonably long lived,
and almost everything is self contained. We don't have to worry about conflicts
in the svg specific bits, because noone will be editing those. I'll update to
the trunk regularly, I hope, anyway.

On unix:

1. cvs -d :pserver:[e-mail] co -r SVG_20010721_BRANCH
mozilla/client.mk (all on one line)
2. cd mozilla
3. gmake -f client.mk
On windows:

1. cvs -d :pserver:[e-mail] co -r SVG_20010721_BRANCH
mozilla/client.mak (all on one line)
2. Edit client.mak to pull the bits which are on the branch like client.mak does
3. Fix makefile.win problems as you come across them.
4. Check your fixes in....

On mac:

No idea. I hope that the mac build scripts can be hacked like I hacked the unix
ones (or do I just add every file to MozillaCheckoutList.txt?)

My plan now is to:

1. work out why colors don't work, and everything is in grey. I have no idea
when this broke, so help would be appreciated.
2. Update to alex's latest stuff
3. update to the draft SVG CR (a few dom changes, and so on)
4. Rewrite/fix nsSVGRenderingContext.cpp to not be really really slow on unix.

Somewhere in there it needs to pull/build on mac + windows.

I plan on closing this bug in a couple of days, and creating a new meta bug for
svg issues fixed by this branch. We can then use the svg component to file bugs
against the stuff on the branch. If you do so, please assign it to
alex/me/yourself (if you plan on fixing it). However, please do not do that
unless the bug also affects alex's latest code.

Comments This shoudl build on unix, and work, assuming that I haven't forgotton
a file. I really want to hear if that is not the case.

Comment 40

[Daniel Jensen]

Has branch landing been scheduled for this one? Shouldn't it be before we ship
1.0? Shouldn't a keyword/target milestone be added for such?

Comment 41

[Gervase Markham [:gerv]]

Yeah, lets aim for 0.9.5. Whats stopping this is:

1) nsSVGRenderingContext needs a rewrite to support linux + <foreignobject>
2) All the dependant bugs not #ifdef MOZ_SVG need review

Comment 42

[Bradley Baetz (:bbaetz)]

Oops - that was me, not gerv. Thats what I get for not logging him out first...
Sorry

Comment 43

[James Turner]

The 're-write' necessary to support foreign object in Linux is *sort of* done; I
have new libart code that can support different native pixel formats (at the
moment, 32-bit RGBA / RGBA and 16-bit 555). Adding in new formats is trivial
(565, etc). The code is not wonderful but it does work, and with these changes
we can simply create our drawing surface in the native format, and tell the
(modified) libArt to the pixel format to use.

This makes the 24-bit flag option to CreateDrawingSurface unecessary, and should
give some speed enhancemnts all around, since the platform native blitting
functions (CopyBits on Mac) will be doing native -> native, instead of 24-bit ->
native.

One caveat is that my libArt changes have only be the the 'composite' renders
(not the clears, and not the solid renderer - both of these are easy to fix
though). More important is that the code for doing affine transformations of
surfaces is totally seperate and I haven't touched it. At present we don't use
that code, but Alex says we will need to in the future.

Comments?
BTW several people on the libArt list seem highly interested in the changes, but
thus far there has been no comment from Raph.

Comment 44

[Daniele Nicolodi]

Where is new libart code ??

Comment 45

[Daniele Nicolodi]

Now we have SVG code into CVS but not in the HEAD branch.
Can we mark this bug as FIXED so all the bugs that depend on it ??

Merging SVG code in the main branch is marked for 0.9.5. 
Is now the moment to do this ?? The source is stable and work fine.

Comment 46

[Thomas Swan]

Did this make it in?

Comment 47

[Oscar van Vliet]

SVG is getting some wide coverage these days:
http://www.iht.com/articles/34869.html

Comment 48

[Roland Mainz]

What's the current status of SVG in Mozilla5 ?

Can I download a nightly source tarball, "configure --enable-svg; gmake" and get
a SVG-able Mozilla ?

Comment 49

[Thomas Swan]

Any chance of landing this anytime soon?

Comment 50

[Tim Larson]

I don't see r/sr, and status=new, so I'm guessing this missed the 0.9.5 branch.
 Maybe we can pull really hard for 0.9.6.  This seems pretty close from the
discussion.

Comment 51

[Thomas Swan]

I'm afraid it did :-(

Comment 52

[Thomas Swan]

Err... typos and email... bad combination...

I meant to say, I'm afraid it didn't :-(

Comment 53

[Asa Dotzler [:asa]]

0.9.5 is out the door. bumping TM up by one.

Comment 54

[Slimy]

roll on mozilla0.9.6
i'm really excited to see mozilla with svg support

Comment 55

[Daniel Jensen]

With 96 votes, almost full readiness, and a landing target of .9.5, I think a
lot of people will be disappointed if this doesn't happen before .9.7 (I hear
.9.6 is frozen). Hopefully the libart problems will be resolved before then.
IMHO, this and related bugs should receive an extremely high priority in the
.9.7 dev cycle.

Comment 56

[Oscar van Vliet]

Actually, together with bug 26186 this is the third most popular bug. The only ones
that got more votes are a PGP plugin (also RFE), and using cached pages for
save/print/view source, etc (dataloss). Try:

http://bugzilla.mozilla.org/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&email1=&emailtype1=substring&emailassigned_to1=1&email2=&emailtype2=substring&emailreporter2=1&bugidtype=include&bug_id=&changedin=&votes=40&chfieldfrom=&chfieldto=Now&chfieldvalue=&short_desc=&short_desc_type=allwordssubstr&long_desc=&long_desc_type=allwordssubstr&bug_file_loc=&bug_file_loc_type=allwordssubstr&status_whiteboard=&status_whiteboard_type=allwordssubstr&keywords=&keywords_type=anywords&field0-0-0=noop&type0-0-0=noop&value0-0-0=&cmdtype=doit&newqueryname=&order=Reuse+same+sort+as+last+time

And not single mention in mostfrequently :).

Comment 57

[Jon Hall]

Does the Apple/Rand SVG patent action have any affect here?
http://www.w3.org/2001/07/SVG10-IPR-statements.html

Comment 58

[chris]

There is no 'patent action'. 

There was an early disclosure of a patent claim that Apple holds, so thatthe 
SVG working group could see whether it might appl and, during the design of 
SVG, work around it if needed. The SVG working group does not believe that this 
patent claim, (even if valid, on which we take no position), is necessary to 
implement SVG. 

I agree that Apple made a RAND license, if anyone feels like licensing it for 
some other purpose ;-) but also, they more recently made a public commitment to 
RF licensing
http://lists.w3.org/Archives/Public/www-patentpolicy-comment/2001Oct/1488.html
and I have asked them to update their SVG license in conformance with that.

Kodak also disclosed a patent claim, and also gave a RAND license, but they 
themselves say that it is not essential for implementing SVG either.

Comment 59

[Bradley Baetz (:bbaetz)]

Getting the code into the trunk (NOT on by default) is being worked on. Theres a
branch, with more up to date code.

-> 0.9.7.

Comment 60

[basic]

are all the patches here (if any) still valid?

Comment 61

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Would someone mind explaining briefly how the foreignObject stuff works in these
patches?

Comment 62

[Bradley Baetz (:bbaetz)]

These patches are obsolete. They're not marked that way because there is no
"obsolete all" button. Once I get the mac build stuff, I will be uplaoding a
patch for review, and I'll obsolete them all then, rather than spamming people
with 20 spearate bugmails.

The current SVG stuff is on the SVG_20010721_BRANCH, which is only a branch of
some files, so it does break every so often. Note the patch on bug 111152, as well.

OTOH, you can get just that file out of bonsai:

http://bonsai.mozilla.org/cvsview2.cgi?command=DIFF&subdir=mozilla%2Flayout%2Fsvg%2Fbase%2Fsrc%2FAttic&file=nsSVGForeignObjectFrame.cpp&rev1=1.1&rev2=1.1.2.11&whitespace_mode=show&diff_mode=context

Thats a diff by rev number, though, so it won't necessarily be the latest
version. For more info, stop by #svg and look for afri during sane hours (in UK
time)

Comment 63

[Bradley Baetz (:bbaetz)]

Checked in!!!!!!

To build, you also need to set the variable for libart.

On unix, add:

mk_add_options MOZ_INTERNAL_LIBART_LGPL=1
MOZ_INTERNAL_LIBART_LGPL=1

to your .mozconfig.

On windows, set MOZ_INTERNAL_LIBART_LGPL=1

On mac, the build option is called libart_lgpl. Since the mac does not
support conditional pulls, you will also have to uncomment the line in
MozillaCheckoutList.txt. This additional step isn't required for
unix/windows - client.mk/client.mak handles it automatically.

Enjoy!

(Also note that bug 95383 was backed out, because it failed to build on osx. Mac
users may see incorrect colors until that is fixed)

Again, this is _not_ on by default - you need your own build, with --enable-svg
(unix), MOZ_SVG=1 (win), and options svg 1 (mac) as well as teh libart option above.

Thanks to everyone!

Comment 64

[James Turner]

Anyone wanting to try a Mac build, please email me or drop into #svg  on irc,
there are a few extra steps required right now, should all be better in  a few days.

Comment 65

[Jim Dunn]

Now that the new version is checked in (and working)
should the test in mozilla/layout/svg/tests be updated
(or removed).  It doesn't appear to be working anymore.

Comment 66

[r0polach]

Why I cannot see any svg graphics in 0.9.8 Release (Windows)?

Comment 67

[Christian :Biesinger (don't email me, ping me on IRC)]

rpolach: SVG isn't turned on by default; either download a build which has SVG
activated (I think no such 0.9.8 build is available yet), or compile it yourself.