New issue
Advanced search Search tips

Issue 596721 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

SVGTextLayoutEngine needs to know about bidi runs

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

Issue description

SVGTextLayoutEngine currently doesn't know about bidi runs and processes everything in the direction of the layout object. This would be fine except SVGTextLayoutEngine calls computeGlyphOverflow which requires that the direction used is correct.

The call to compute glyph overflow doesn't re-shape the entire text run (due to caching) but it's still not ideal and will produce the wrong result in the presence of bidi runs with glyph overflow.

There are two potential solutions I can think of:
1) Change SVGTextLayoutEngine to use a bidi run iterator
2) Add a bit to SVGTextLayoutAttributes to have a bit for direction, set it in SVGTextMetricsBuilder, and create a new fragment (shouldStartNewFragment) when the direction changes in SVGTextLayoutEngine. The refactoring in https://codereview.chromium.org/1822513002 builds towards making this cleaner.

This is a real bug (though existing and low priority) and can be seen on combining-character-queries.html.
 

Comment 1 by f...@opera.com, Mar 22 2016

1) Not needed, we can use the direction of the line box involved. (This is what computeGlyphOverflow should do... sorry for missing that during review.)
2) Not needed for essentially the same reason as (1). (SVGTextLayoutEngine process per line box, and line boxes have only a single direction.)

That being said though, there are parts of the layout algorithm that should consider direction.
Project Member

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

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

commit eb829d2e605994528ec3f6cf1edd69a3e6f3fb97
Author: fs <fs@opera.com>
Date: Tue Mar 22 19:54:24 2016

Use the line box's direction in computeGlyphOverflow

Use SVGInlineTextBox::constructTextRun in order to get the direction as
determined by the BiDi algorithm rather than the what is specified on
the element. (This should also get the right override value for similar
reasons.)

BUG= 596721 

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

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

[modify] https://crrev.com/eb829d2e605994528ec3f6cf1edd69a3e6f3fb97/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/eb829d2e605994528ec3f6cf1edd69a3e6f3fb97/third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.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/+/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19

commit a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19
Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org>
Date: Wed Mar 23 00:15:52 2016

Auto-rebaseline for r382655

https://chromium.googlesource.com/chromium/src/+/eb829d2e6

BUG= 596721 
TBR=fs@opera.com

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

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

[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/LayoutTests/platform/android/svg/W3C-I18N/text-anchor-dirRTL-anchorEnd-expected.txt
[delete] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/LayoutTests/platform/android/svg/W3C-I18N/text-anchor-dirRTL-anchorMiddle-expected.txt
[delete] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/LayoutTests/platform/android/svg/W3C-I18N/text-anchor-dirRTL-anchorStart-expected.txt
[delete] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/LayoutTests/platform/android/svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorEnd-expected.txt
[delete] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/LayoutTests/platform/android/svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorMiddle-expected.txt
[delete] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/LayoutTests/platform/android/svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorStart-expected.txt
[delete] https://crrev.com/557d64542a3d0eda9c8f4cbd445a9ecfd88c8622/third_party/WebKit/LayoutTests/platform/android/svg/W3C-I18N/tspan-dirLTR-ubEmbed-in-rtl-context-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-I18N/text-anchor-dirRTL-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-I18N/text-anchor-dirRTL-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-I18N/text-anchor-dirRTL-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/W3C-I18N/tspan-dirLTR-ubEmbed-in-rtl-context-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-intro-05-t-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac-mac10.9/svg/text/combining-character-queries-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-dirLTR-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-dirLTR-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-dirLTR-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-dirNone-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-dirNone-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-dirNone-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/text-anchor-no-markup-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/W3C-I18N/tspan-direction-ltr-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/text/bidi-text-query-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/text/combining-character-queries-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-intro-05-t-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win/svg/W3C-I18N/tspan-direction-ltr-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win/svg/text/bidi-text-query-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-dirLTR-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-dirLTR-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-dirLTR-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-dirNone-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-dirNone-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-dirNone-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorEnd-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorMiddle-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorStart-expected.txt
[modify] https://crrev.com/a2e42791f5f91fef4ad2b4b27eb9648be9fb1b19/third_party/WebKit/LayoutTests/platform/win7/svg/W3C-I18N/text-anchor-no-markup-expected.txt

Project Member

Comment 4 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

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

pdr, Anything more you think we should be doing here?

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

Owner: f...@opera.com
Status: Fixed (was: Available)
Nope, I think we just need to update these owner and status fields.

Sign in to add a comment