Bug 45533

Summary: SVG text chunk concept needs to be integrated in the InlineBox structure
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Severity: Normal CC: abarth, dglazkov, eric, gustavo, morrita, morrita, webkit.review.bot, xan.lopez, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Description Flags
Patch krit: review+

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]

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]

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]

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