Bug 4883

Summary:

WebCore+SVG missing <text> support

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Bug Depends on:

Bug Blocks:

5579

Attachments:

Description	Flags
KCanvas side of text support	none
Ksvg2 side of text support	none
Ksvg2 side of text support	none
KCanvas side of text support	none
KCanvas side of text support	none
Simple hack to use CSS text rendering for text.	mjs: review-
a patch that provides basic text support using CSS text rendering	eric: review-
updated for many of Eric's comments	eric: review+

Eric Seidel (no email)

Reported 2005-09-08 01:25:06 PDT

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.

Attachments
KCanvas side of text support (24.43 KB, patch) 2005-10-03 14:12 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Ksvg2 side of text support (10.84 KB, patch) 2005-10-03 14:17 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Ksvg2 side of text support (10.84 KB, patch) 2005-10-05 08:53 PDT, Rob Buis	no flags	Details Formatted Diff Diff
KCanvas side of text support (24.43 KB, patch) 2005-10-05 11:19 PDT, Rob Buis	no flags	Details Formatted Diff Diff
KCanvas side of text support (41.17 KB, patch) 2005-10-05 11:22 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Simple hack to use CSS text rendering for text. (2.25 KB, patch) 2006-01-06 03:24 PST, Maciej Stachowiak	mjs: review-	Details Formatted Diff Diff
a patch that provides basic text support using CSS text rendering (14.44 KB, patch) 2006-01-07 17:34 PST, Maciej Stachowiak	eric: review-	Details Formatted Diff Diff
updated for many of Eric's comments (14.47 KB, patch) 2006-01-07 21:26 PST, Maciej Stachowiak	eric: review+	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Rob Buis

Comment 1 2005-10-03 14:11:54 PDT

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

Comment 2 2005-10-03 14:12:59 PDT

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

Comment 3 2005-10-03 14:17:02 PDT

Created attachment 4187 [details] Ksvg2 side of text support

Rob Buis

Comment 4 2005-10-05 03:20:30 PDT

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

Comment 5 2005-10-05 08:53:16 PDT

Created attachment 4213 [details] Ksvg2 side of text support This time the new files should be in the patch

Rob Buis

Comment 6 2005-10-05 11:19:13 PDT

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

Comment 7 2005-10-05 11:22:09 PDT

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

Comment 8 2006-01-06 03:24:26 PST

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 9 2006-01-06 17:23:02 PST

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

Comment 10 2006-01-07 17:34:23 PST

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

Eric Seidel (no email)

Comment 11 2006-01-07 17:48:20 PST

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

Comment 12 2006-01-07 21:26:32 PST

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

Eric Seidel (no email)

Comment 13 2006-01-07 22:12:44 PST

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.

Note You need to log in before you can comment on or make changes to this bug.