New issue
Advanced search Search tips

Issue 594058 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Cleanup SVGTextMetricsBuilder's SVGTextMetricsCalculator

Project Member Reported by pdr@chromium.org, Mar 11 2016

Issue description

There's a TODO in SVGTextMetricsBuilder.cpp to decouple SVGTextMetricsCalculator from the loop in measureTextLayoutObject. One approach is to turn SVGTextMetricsCalculator into a simple bidi run iterator and create a dummy bidi run for the common RTL case (credit to fs for this idea :).

I planned to do this as an immediate followup to https://codereview.chromium.org/1773403002 but would like to fix a couple more issues blocking  https://crbug.com/589525  first. Filing this bug so we don't forget about this useful cleanup.
 

Comment 1 by pdr@chromium.org, Mar 11 2016

Oops, that should read "common LTR case".
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9aa3054b0d2bacf5cc9774de798ebb35cfa88f01

commit 9aa3054b0d2bacf5cc9774de798ebb35cfa88f01
Author: fs <fs@opera.com>
Date: Wed Mar 23 22:12:27 2016

Move SVGTextMetrics::constructTextRun to SVGTextMetricsBuilder

New resting place is the SVGTextMetricsCalculator helper class.
This avoids using this function to create runs based on the wrong BiDi
context.
Also wrap the static bits of SVGTextMetricsBuilder.cpp in an unnamed
namespace, removing a few 'static' keywords.

BUG= 596721 ,  594058 

Review URL: https://codereview.chromium.org/1829713002

Cr-Commit-Position: refs/heads/master@{#382938}

[modify] https://crrev.com/9aa3054b0d2bacf5cc9774de798ebb35cfa88f01/third_party/WebKit/Source/core/layout/svg/SVGTextMetrics.cpp
[modify] https://crrev.com/9aa3054b0d2bacf5cc9774de798ebb35cfa88f01/third_party/WebKit/Source/core/layout/svg/SVGTextMetrics.h
[modify] https://crrev.com/9aa3054b0d2bacf5cc9774de798ebb35cfa88f01/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08441660b5ba7c44f233e34379ef1dda562d046a

commit 08441660b5ba7c44f233e34379ef1dda562d046a
Author: fs <fs@opera.com>
Date: Wed Mar 23 22:22:08 2016

More explicit SVGTextMetrics construction

This makes SVGTextMetrics dumber - essentially POD - leaving all
measuring etc. to whoever creates one (SVGTextMetricsBuilder/Calculator)
for a minor "cost" in complexity.
This makes SVGTextMetrics not depend on LineLayoutSVGInlineText.

Drop SVGTextMetrics::setWidth too since it's unused.

BUG= 594058 

Review URL: https://codereview.chromium.org/1825613005

Cr-Commit-Position: refs/heads/master@{#382944}

[modify] https://crrev.com/08441660b5ba7c44f233e34379ef1dda562d046a/third_party/WebKit/Source/core/layout/svg/SVGTextMetrics.cpp
[modify] https://crrev.com/08441660b5ba7c44f233e34379ef1dda562d046a/third_party/WebKit/Source/core/layout/svg/SVGTextMetrics.h
[modify] https://crrev.com/08441660b5ba7c44f233e34379ef1dda562d046a/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c53c160e1391842324a60c1391db9b6ff7ccf5bf

commit c53c160e1391842324a60c1391db9b6ff7ccf5bf
Author: fs <fs@opera.com>
Date: Fri Mar 25 02:03:51 2016

Always create a BidiRun in SVGTextMetricsBuilder

Create a BidiRun for the 'override' case too, to avoid a bunch of
special cases. Since we always have a BidiRun now, null-checks can be
removed, and code simplified a bit. (Hopefully even more in the future.)
Also make "8-bit" (latin1) strings take this code-path. (This was
handled by the SimpleShaper path previously.)

BUG= 594058 

Review URL: https://codereview.chromium.org/1826263002

Cr-Commit-Position: refs/heads/master@{#383224}

[modify] https://crrev.com/c53c160e1391842324a60c1391db9b6ff7ccf5bf/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp

Comment 5 by f...@opera.com, Mar 30 2016

While looking at  issue 597312  (which contained one more "but" than I had anticipated...) I developed the opinion that computation of "value (attribute) position" ought to be separated from metrics computation. I.e rather than having them both intermingled, it should be like:

<compute metrics/assert computed>
<compute attribute map>

(per LayoutSVGInlineText; tree-walk still similar to previously)

That ought to make the computation of the attribute map more "consistent" wrt character -> glyph mapping and whitespace collapsing (hopefully the latter would only need to done in one place...)

Seems to fit under this umbrella, because it ought to simplify SVGTextMetricsCalculator.

This would also beg for tying metrics (even) closer to LayoutSVGInlineText, and subsequently simplify the attributes setup and handling. (After all, the most common case is to have a single <x, y> positioning pair.)

Comment 6 by pdr@chromium.org, Mar 30 2016

@fs, do you mind expanding on your design just a little more? I think we're on the same page but I'd like to make sure.

Mapping code units post-whitespace-collapsing, mapping code unit to unicode character (aka subtracting one per surrogate pair), and bidi ordering are all mixed together today. At the moment we have one SVGTextMetrics per code unit in post-bidi order. SVGTextLayoutAttributes also has one SVGCharacterData per code unit in post-bidi order, except whitespace has been collapsed. SVGTextMetricsBuilder and SVGTextQuery need to know about all of this in great detail, and SVGTextLayoutEngine knows about most. We don't yet have a concept of typographic character unit but that's another mapping with multiple entries per ligature but only a single entry for combined diacritics.

This is all very confusing to me and I'd like to bury it behind interfaces.

https://codereview.chromium.org/1844723003 moves metrics and attributes further apart which could be good since they are barely related. At the same time, metrics and attributes are still tightly coupled for the above reasons. Can you sketch out (with lots of handwaving) how this would work in your ideal world?
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b38c00ea369443e37bcc94ef59ed2b5874d555c7

commit b38c00ea369443e37bcc94ef59ed2b5874d555c7
Author: fs <fs@opera.com>
Date: Wed Mar 30 23:11:26 2016

Move SVGTextPositioningElement::elementFromLayoutObject

...to SVGTextLayoutAttributesBuilder.cpp, since that is where it's used.
Turn the entry if into an ASSERT (because it's trivial to see that the
condition always holds in this context.)

BUG= 594058 

Review URL: https://codereview.chromium.org/1846633002

Cr-Commit-Position: refs/heads/master@{#384136}

[modify] https://crrev.com/b38c00ea369443e37bcc94ef59ed2b5874d555c7/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.cpp
[modify] https://crrev.com/b38c00ea369443e37bcc94ef59ed2b5874d555c7/third_party/WebKit/Source/core/svg/SVGTextPositioningElement.cpp
[modify] https://crrev.com/b38c00ea369443e37bcc94ef59ed2b5874d555c7/third_party/WebKit/Source/core/svg/SVGTextPositioningElement.h

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b44c6b7b5e08fcd235cabc068cb01d88210f4c74

commit b44c6b7b5e08fcd235cabc068cb01d88210f4c74
Author: fs <fs@opera.com>
Date: Wed Mar 30 23:11:51 2016

Pass LineLayoutSVGInlineText to SVGTextMetricsCalculator

Slightly more on the Layout API bandwagon.

BUG= 594058 

Review URL: https://codereview.chromium.org/1838363004

Cr-Commit-Position: refs/heads/master@{#384137}

[modify] https://crrev.com/b44c6b7b5e08fcd235cabc068cb01d88210f4c74/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp

Comment 9 by f...@opera.com, Mar 31 2016

Just to clarify something first, about: "bidi ordering". In the SVGTextMetricsBuilder/Calculator, we don't actually perform any (BiDi) reordering, so the runs are still in what I will refer to as "logical order" (as opposed to "visual order" which would be what we have after reordering). So that's one thing less in the mix (we need the BiDi bits in order to compute the BiDi levels so that we can derive the direction of a run.)

Now on to the handwaving...

So, what I would like to achieve is a model where we first compute the metrics and then - based on the metrics - resolve the attributes/compute the attribute list/map. The first step that I imagine would be to take the loop in walkInlineText() and split it to essentially:

  for ( each tcu )
    add to metrics list

  for ( each metrics entry )
    update layout attributes

I'm still pondering the best way to handle white-space collapsing here, so that somewhat up in the air - this should only be considered an intermediate stage though. The next step would be to push (sink) the layout attributes setup out and just have the metrics computation.
At this point it will probably make sense to get rid of the SVGTextMetricsBuilder - moving the metrics calculation to LayoutSVGInlineText (optionally make the current calculator the interface object and keep that in the file/rename) and walkTree w/ wrappers to SVGTextLayoutAttributesBuilder (which is the only thing interfacing to SVGTextMetricsBuilder). Not unlikely there'll need to be some interface flurry in SVGTextLayoutAttributesBuilder too.

By now, SVGTextLayoutAttributesBuilder will actually have all the "attribute building" code. (Hopefully processLayoutSVGInlineText is either gone at this point, or it can be removed now - removing the other white-space collapsing "implementation".)
At this point, we can turn our sight to SVGTextLayoutAttributesBuilder. This is (even) more vague, but things that could be done at this point is to make SVGTextLayoutAttributesBuilder compute the SVGTextLayoutAttributes map (for each text node) directly, rather than first doing a global char map (and positions map; m_textPositions). This would essentially mean merging the two tree walks (walkTree and collectTextPositioningElements with current naming).
Further things to consider at this point would be getting rid of the all the "logical metrics/attributes" stuff in SVGTextLayoutEngine. Maybe even move the SVGTextLayoutAttributes resolving here, doing it on the fly (it's a "linear" structure, and the lists are often single-entry.) This far into the future things are a bit more hazy though...

So, to sum up ("executive summary"):

 1. Split walkInlineText.
 2. Refactor/split SVGTextMetricsBuilder moving its bits elsewhere.
 3. Cleanup SVGTextLayoutAttributesBuilder.
 (4? Compute SVGTextLayoutAttributes directly.)
 (5? Simplify "logical attributes" handling in SVGTextLayoutEngine.)
 (6?? Compute SVGTextLayoutAttributes on-the-fly/during layout; SVGTextLayoutAttributesBuilder likely bites the dust.)

Points 1 and 2 could like be split further and even re-arranged a bit (i.e we could probably move the bits that fit better in SVGTextLayoutAttributesBuilder right away and then gradually peel more stuff off from there.)

As for interactions with other parts of the code (most prominently SVGTextQuery), I think we could move more of the "extents" computation code to LayoutSVGInlineText, to let that be the "managing entity" of text metrics in general.

This would leave us with separate metrics computation (and possibly white-space collapsing) that would feed into attribute computation/index resolving, that in turn would feed into the "layout engine".

Comment 10 by f...@opera.com, Mar 31 2016

Addendum: I expect that we would/could get rid of the "attribute list reordering" step (SVGRootInlineBox::reorderValueLists) around step 5 or so.

Comment 11 by f...@opera.com, Mar 31 2016

Been hacking a bit on a "PoC" for this today, and I think it looks pretty ok so far. I'm beginning to develop a grudge towards SVGTextLayoutAttributesBuilder::buildLayoutAttributesForTextNode though, so I probably ought to add a "step 0: Remove ..." =)
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91d29a30053008ec263fdeeda7e51d0c7cadc1a8

commit 91d29a30053008ec263fdeeda7e51d0c7cadc1a8
Author: fs <fs@opera.com>
Date: Fri Apr 01 00:43:51 2016

Move metrics list storage to LayoutSVGInlineText

It was previously stored in SVGTextLayoutAttributes (which is stored in
LayoutSVGInlineText). The connection between these two is very loose, so
letting the metrics be a part of the attributes structure doesn't feel
entirely logical. There's still a back-pointer in the attributes
structure, which means it's still reachable in the same way (albeit with
one additional indirection.)
Rename the various accessors to metricsList().

BUG= 594058 

Review URL: https://codereview.chromium.org/1844723003

Cr-Commit-Position: refs/heads/master@{#384450}

[modify] https://crrev.com/91d29a30053008ec263fdeeda7e51d0c7cadc1a8/third_party/WebKit/Source/core/layout/api/LineLayoutSVGInlineText.h
[modify] https://crrev.com/91d29a30053008ec263fdeeda7e51d0c7cadc1a8/third_party/WebKit/Source/core/layout/svg/LayoutSVGInlineText.h
[modify] https://crrev.com/91d29a30053008ec263fdeeda7e51d0c7cadc1a8/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributes.cpp
[modify] https://crrev.com/91d29a30053008ec263fdeeda7e51d0c7cadc1a8/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributes.h
[modify] https://crrev.com/91d29a30053008ec263fdeeda7e51d0c7cadc1a8/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp
[modify] https://crrev.com/91d29a30053008ec263fdeeda7e51d0c7cadc1a8/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp
[modify] https://crrev.com/91d29a30053008ec263fdeeda7e51d0c7cadc1a8/third_party/WebKit/Source/core/layout/svg/SVGTextQuery.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/63680ba154b5839ee37ad744caf251ebe41bd3a3

commit 63680ba154b5839ee37ad744caf251ebe41bd3a3
Author: fs <fs@opera.com>
Date: Tue Apr 05 11:28:53 2016

Rebuild layout attributes on layout instead of on layout tree updates

What layout attributes are used (for a text node; LayoutSVGInlineText),
depends on how many "characters" precedes the node in question.
Layout attributes were updated on insertions and removals on the layout
tree, by find the node to update, and update the surrounding nodes.
It were however trying to depend on the order in which nodes were being
attached, which meant that a sequence of updates could lead to incorrect
layout attribute (indices) being computed. The process per node is also
essentially O(n) (albeit a fairly cheap such.)
Instead of updating on add/remove/update of nodes, just mark the position
data as invalid, and update on the next layout of the <text> root. This
also has the side-effect of simplifying the code quite significantly,
and should avoid repeatedly resolving the layout attribute indices.

Also take the opportunity to pass LayoutSVGText references and simplify
related code a bit.

BUG= 405966 ,  594058 

Review URL: https://codereview.chromium.org/1854123002

Cr-Commit-Position: refs/heads/master@{#385149}

[add] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/LayoutTests/svg/text/text-positioning-mutate-textnode-expected.html
[add] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/LayoutTests/svg/text/text-positioning-mutate-textnode.html
[modify] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/Source/core/layout/svg/LayoutSVGInline.cpp
[modify] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
[modify] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.h
[modify] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.cpp
[modify] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.h
[modify] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp
[modify] https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.h

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46068407adaa86bc96d8fffec53df7a0575cdcc9

commit 46068407adaa86bc96d8fffec53df7a0575cdcc9
Author: fs <fs@opera.com>
Date: Fri Apr 08 22:13:18 2016

Add test for metrics invalidation on textnode removal

This tests that text metrics (read: collapsing info) are updated as
needed when a text node is removed.

BUG= 299497 ,  594058 

Review URL: https://codereview.chromium.org/1872513005

Cr-Commit-Position: refs/heads/master@{#386223}

[add] https://crrev.com/46068407adaa86bc96d8fffec53df7a0575cdcc9/third_party/WebKit/LayoutTests/svg/text/text-collapsed-ws-remove-textnode-expected.html
[add] https://crrev.com/46068407adaa86bc96d8fffec53df7a0575cdcc9/third_party/WebKit/LayoutTests/svg/text/text-collapsed-ws-remove-textnode.html

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f19c8c6644e933e57a20d518b6ef62e2358ac1ad

commit f19c8c6644e933e57a20d518b6ef62e2358ac1ad
Author: fs <fs@opera.com>
Date: Sat Apr 09 16:38:54 2016

Invalidate text metrics when the <text> subtree is mutated

When the content of a text node is modified, we would only invalidate
positioning values and not text metrics. This would lead to incorrect or
inconsistent text metrics/fonts being used, which would lead to repaint
bugs and similar issues.
Make sure all mutations to the <text> subtree trigger text metrics re-
computation.

Also take this opportunity to move the definition of the
willBeDestroyed() method for slightly better grouping.

BUG= 299497 ,  594058 

Review URL: https://codereview.chromium.org/1865923002

Cr-Commit-Position: refs/heads/master@{#386300}

[modify] https://crrev.com/f19c8c6644e933e57a20d518b6ef62e2358ac1ad/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/f19c8c6644e933e57a20d518b6ef62e2358ac1ad/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14177d51219e4ba4d1381eb6a4486ec5d71020c0

commit 14177d51219e4ba4d1381eb6a4486ec5d71020c0
Author: fs <fs@opera.com>
Date: Sat Apr 09 18:02:13 2016

Separate metrics update and layout attribute resolving

This splits the walkInlineText() function from SVGTextMetricsBuilder.cpp
into one function for computing the Vector of SVGTextMetrics (called
via updateTextMetrics in LayoutSVGInlineText) and one function for
computing the mapping of layout attributes (updateLayoutAttributes in
SVGTextLayoutAttributesBuilder.cpp).
This in turn mean that the UpdateAttribute helper struct is split and
done away with, similarly TreeWalkTextState.

BUG= 594058 

Review URL: https://codereview.chromium.org/1861013003

Cr-Commit-Position: refs/heads/master@{#386301}

[modify] https://crrev.com/14177d51219e4ba4d1381eb6a4486ec5d71020c0/third_party/WebKit/Source/core/layout/svg/LayoutSVGInlineText.cpp
[modify] https://crrev.com/14177d51219e4ba4d1381eb6a4486ec5d71020c0/third_party/WebKit/Source/core/layout/svg/LayoutSVGInlineText.h
[modify] https://crrev.com/14177d51219e4ba4d1381eb6a4486ec5d71020c0/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
[modify] https://crrev.com/14177d51219e4ba4d1381eb6a4486ec5d71020c0/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.cpp
[modify] https://crrev.com/14177d51219e4ba4d1381eb6a4486ec5d71020c0/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.h
[modify] https://crrev.com/14177d51219e4ba4d1381eb6a4486ec5d71020c0/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp
[modify] https://crrev.com/14177d51219e4ba4d1381eb6a4486ec5d71020c0/third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.h

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9331211c22aee7b499131aa1c58ce515deceeb45

commit 9331211c22aee7b499131aa1c58ce515deceeb45
Author: fs <fs@opera.com>
Date: Sat Apr 09 19:13:43 2016

Use characters (not code units) when computing value list positions

The value list position is updated by one for each character, and not
at all when spaces are skipped (collapsed). When assigning value list
positions, we are currently counting surrogates as two (on for each
code unit.)
Use the text metrics data to count the number of (non-collapsed)
characters instead.

BUG= 597312 ,  594058 

Review URL: https://codereview.chromium.org/1866703002

Cr-Commit-Position: refs/heads/master@{#386305}

[add] https://crrev.com/9331211c22aee7b499131aa1c58ce515deceeb45/third_party/WebKit/LayoutTests/svg/text/surrogate-pair-attribute-positions-expected.html
[add] https://crrev.com/9331211c22aee7b499131aa1c58ce515deceeb45/third_party/WebKit/LayoutTests/svg/text/surrogate-pair-attribute-positions.html
[modify] https://crrev.com/9331211c22aee7b499131aa1c58ce515deceeb45/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.cpp
[modify] https://crrev.com/9331211c22aee7b499131aa1c58ce515deceeb45/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.h

Project Member

Comment 19 by bugdroid1@chromium.org, Apr 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50e8dd8ad929f02ecfa3f020a6971a9606fdb993

commit 50e8dd8ad929f02ecfa3f020a6971a9606fdb993
Author: fs <fs@opera.com>
Date: Tue Apr 12 08:01:49 2016

Don't persist the SVGTextLayoutAttributesBuilder

No partial updates are performed on the structures contained within
the builder, so keeping them around between layouts only amounts to
memory wasted. With this change the builder is now more of a proper
builder. buildLayoutAttributesForTextRoot() is folded into
buildLayoutAttributes().

BUG= 594058 

Review URL: https://codereview.chromium.org/1871393003

Cr-Commit-Position: refs/heads/master@{#386616}

[modify] https://crrev.com/50e8dd8ad929f02ecfa3f020a6971a9606fdb993/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
[modify] https://crrev.com/50e8dd8ad929f02ecfa3f020a6971a9606fdb993/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.h
[modify] https://crrev.com/50e8dd8ad929f02ecfa3f020a6971a9606fdb993/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.cpp
[modify] https://crrev.com/50e8dd8ad929f02ecfa3f020a6971a9606fdb993/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.h

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb4114c47acb1a944c5b68e9d6486c68e01fc311

commit eb4114c47acb1a944c5b68e9d6486c68e01fc311
Author: fs <fs@opera.com>
Date: Tue Apr 12 08:17:19 2016

Refactor SVGTextLayoutAttributesBuilder::collectTextPositioningElements

This moves the creation of a TextPosition element for the <text> into
collectTextPositioningElements too, getting rid of the special case.

BUG= 594058 

Review URL: https://codereview.chromium.org/1878583002

Cr-Commit-Position: refs/heads/master@{#386622}

[modify] https://crrev.com/eb4114c47acb1a944c5b68e9d6486c68e01fc311/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutAttributesBuilder.cpp

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd627c9a9d79a058525581fc88985da16082b8c7

commit bd627c9a9d79a058525581fc88985da16082b8c7
Author: fs <fs@opera.com>
Date: Thu Apr 14 11:29:55 2016

Reorder metrics iteration in LayoutSVGInlineText::updateMetricsList

This changes iteration to iterate BidiRuns and then collect metrics for
all characters in each run.

BUG= 594058 

Review URL: https://codereview.chromium.org/1880453002

Cr-Commit-Position: refs/heads/master@{#387283}

[modify] https://crrev.com/bd627c9a9d79a058525581fc88985da16082b8c7/third_party/WebKit/Source/core/layout/svg/LayoutSVGInlineText.cpp
[modify] https://crrev.com/bd627c9a9d79a058525581fc88985da16082b8c7/third_party/WebKit/Source/core/layout/svg/LayoutSVGInlineText.h

Comment 23 by f...@opera.com, Apr 14 2016

Owner: f...@opera.com
Status: Fixed (was: Available)
Closing this now. I think what we set out to achieve has been achieved in some form. I still have some "items" left from c#9, but I think those could logically go/be handled elsewhere.

Sign in to add a comment