Bug 4883 – WebCore+SVG missing <text> support

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Eric Seidel (no email)]

WebCore+SVG missing <text> support

'nough said.  KDE's ksvg2/kcanvas has started work on this. (rwlbuis)  Hopefully we can just pick that up.

The original plan (and still likely eventual plan) is to merge the render trees and use the khtml::RenderText 
subclasses to support text.

[Rob Buis]

Hi, 
 
I'll attach my current text support (on my local system, not in kde 
svn yet). I would like some reactions on the design choices made, suggestions 
for improvements etc. 
Cheers, 
 
Rob. 

[Rob Buis]

Created attachment 4186 [details]
KCanvas side of text support

There are changes in here for ref counting, ignore them if
you arejust interested in text support.

[Rob Buis]

Created attachment 4187 [details]
Ksvg2 side of text support

[Rob Buis]

In case anyone is wondering, the patch lacks a few critical files, because
I made an eror with the diff :( I'll make a new patch soon.
Cheers,

Rob.

[Rob Buis]

Created attachment 4213 [details]
Ksvg2 side of text support

This time the new files should be in the patch

[Rob Buis]

Created attachment 4219 [details]
KCanvas side of text support

Ok, I am quite inept at the attachment and diff creation stuff :|
I hope this one is useful at last.

[Rob Buis]

Created attachment 4220 [details]
KCanvas side of text support

Ugh, no comment. Well, maybe just that it seems I need a vacation badly :)

[Maciej Stachowiak]

Created attachment 5510 [details]
Simple hack to use CSS text rendering for text.

This actually draws text, but upside down and positioned wrong.

[Eric Seidel (no email)]

Comment on attachment 5510 [details]
Simple hack to use CSS text rendering for text.

This is much better than nothing.  It sounds like darin and I agree, we should
just commit this.  It's horribly broken, but if nothing else it promotes
awareness of the <text> work. :)  r=me.

[Maciej Stachowiak]

Created attachment 5535 [details]
a patch that provides basic text support using CSS text rendering

[Eric Seidel (no email)]

Comment on attachment 5535 [details]
a patch that provides basic text support using CSS text rendering

Ok a few comments:

1.  tabs in ChangeLog
2.  Are you sure you want the LGPL on these? or shoudl it be BSD.  If you
copied LGPL code it's gotta be LGPL...
3. I think translate* shoudl take a KRenderingDeviceContext * instead of
looking it up.	Or alternatively, they could return a  QMatrix (KCanvasMatrix
is slated for death)

+void RenderSVGText::paint(PaintInfo &paintInfo, int parentX, int parentY)
should check to make sure it's in the foreground phase, and otherwise return.

PaintInfo & should be PaintInfo&

Just go ahead and kill the entire block of extern from 
khtml/rendering/render_canvasimage.h
That also could go in a separate patch, but doesn't need to.

missing newline from 
ksvg2/css/svg.css

+    return new (getDocument()->renderArena()) RenderSVGText(this);
in
+khtml::RenderObject *SVGTextElementImpl::createRenderer(RenderArena *arena,
khtml::RenderStyle *style)

should use the passed in areana instead.

Otherwise looks great.	I'm going to r- for these silly little nits.  But it's
not really necessary to post a new patch if that's all you change.

[Maciej Stachowiak]

Created attachment 5537 [details]
updated for many of Eric's comments

[Eric Seidel (no email)]

Comment on attachment 5537 [details]
updated for many of Eric's comments

Beyond words, amazing.	r=me.

Be sure to commit updated layout test results:

cd LayoutTests/svg
find . -name "*-expected.*" -delete
run-webkit-tests --svg

should do the trick.