Bug 45533 - SVG text chunk concept needs to be integrated in the InlineBox structure
Summary: SVG text chunk concept needs to be integrated in the InlineBox structure
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-10 05:04 PDT by Nikolas Zimmermann
Modified: 2010-09-30 04:53 PDT (History)
9 users (show)

See Also:


Attachments
Patch (182.62 KB, patch)
2010-09-10 05:40 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch (182.69 KB, patch)
2010-09-10 05:47 PDT, Nikolas Zimmermann
krit: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2010-09-10 05:04:59 PDT
We currently hack around SVGs text chunk concept, by allowing a single InlineTextBox to contain multiple chunks
<text x="20 30 40">ABC</text> just creates one InlineTextBox, but maps to three text chunks (see SVG spec for the text chunk definition, short story: any absolute position defines a new text chunk)
A problem is that the spec demands that BiDi reordering only happens for a single text chunk -> in order to implement that we have to move the text chunk concept directly into the InlineBox structure.
That means a single InlineTextBox is only allowed to contain max. 1 chunk.

For the example above we have to create three SVGInlineTextBoxes.
This will allow us to throw away 70% of the complexity in the SVG text layout code: the whole text chunk & text chunk part concept is just a hack around the fact that our inline box structure doesn't refelect text chunks -> fix that.

Will make the whole text code much simplier.
I will try to do it in small chunks.
Comment 1 Nikolas Zimmermann 2010-09-10 05:40:12 PDT
Created attachment 67171 [details]
Patch

Tried to keep this patch as small as possible. We're now computing the text layout twice. The new code path does all compuations before the inline box tree is created, the old code does it again, after the creation of the inline box tree. This duplication is intended, and will be removed in follow-up patches.

I didn't want to mix removing/rewriting the old layout text code, otherwhise the patch size would get huge.
Comment 2 WebKit Review Bot 2010-09-10 05:41:48 PDT
Attachment 67171 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/rendering/RenderBlockLineLayout.cpp:45:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/rendering/RenderBlockLineLayout.cpp:1653:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/rendering/svg/SVGTextLayoutBuilder.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/rendering/svg/SVGTextLayoutAttributes.cpp:23:  You should add a blank line after implementation file's own header.  [build/include_order] [4]
WebCore/rendering/RenderSVGText.cpp:49:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/rendering/svg/SVGTextLayoutBuilder.h:25:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 6 in 53 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Nikolas Zimmermann 2010-09-10 05:47:21 PDT
Created attachment 67172 [details]
Patch

Fixed style issues.
Comment 4 WebKit Review Bot 2010-09-10 07:30:24 PDT
Attachment 67172 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3905371
Comment 5 Dirk Schulze 2010-09-10 08:16:49 PDT
Comment on attachment 67172 [details]
Patch

Please make sure that the gtk bot builds on landing. The code is still complex, but at least understandable. Lookgin forward for the follow ups. r=me
Comment 6 WebKit Review Bot 2010-09-10 08:25:48 PDT
Attachment 67172 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3935371
Comment 7 Nikolas Zimmermann 2010-09-10 08:36:41 PDT
Committed r67200: <http://trac.webkit.org/changeset/67200>
Comment 8 WebKit Review Bot 2010-09-10 09:19:10 PDT
http://trac.webkit.org/changeset/67200 might have broken GTK Linux 64-bit Debug