New issue
Advanced search Search tips

Issue 666758 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 6122
issue 636993



Sign in to add a comment

Support context-preserving shaping for LayoutNG

Project Member Reported by e...@chromium.org, Nov 18 2016

Issue description

Support text shaping across element boundaries while maintaining context to allow natural text flow across element boundaries.

 
a (4).html
2.1 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 18 2016

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

commit 185f262ceb59340af223f08a89491beb71f30a2c
Author: eae <eae@chromium.org>
Date: Fri Nov 18 17:05:34 2016

Refactor HarfBuzzShaper to not retain font data

First part in a suite of changes to allow shaping of individual segments
in a TextRun using a single HarfBuzzShaper instance with the prospective
objective to support continuous text shaping across DOM node boundaries.

This change removes the Font argument from the constructor and moves all
font setup plus associated feature detection to the shapeResults method.

By moving the font and font feature settings from the constructor to the
shapeResults call each segment may specify a different font and features
while sharing the same normalized text buffer and thus allowing segments
to use the surrounding text as context while shaping.

Additionally it moves ownership of the holes queue and font feature list
from the class itself to the shapeResults method thereby simplifying the
life cycle management for those collections and ensures that the feature
settings list and the holes queue are both reset between each segment.

TEST=HarfBuzzShaperTest.cpp
BUG= 666758 
R=drott@chromium.org

Review-Url: https://codereview.chromium.org/2515493002
Cr-Commit-Position: refs/heads/master@{#433222}

[modify] https://crrev.com/185f262ceb59340af223f08a89491beb71f30a2c/third_party/WebKit/Source/platform/fonts/shaping/CachingWordShapeIterator.cpp
[modify] https://crrev.com/185f262ceb59340af223f08a89491beb71f30a2c/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
[modify] https://crrev.com/185f262ceb59340af223f08a89491beb71f30a2c/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.h
[modify] https://crrev.com/185f262ceb59340af223f08a89491beb71f30a2c/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp

Comment 2 by ebra...@gnu.org, Nov 19 2016

Cc: ebra...@gnu.org
future note to myself: test things like  Issue 661717  on --enable-blink-features=LayoutNG
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 21 2016

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

commit 6ed07c0f306b6995f1056ca51986bd3316d3a571
Author: eae <eae@chromium.org>
Date: Mon Nov 21 18:14:05 2016

Simplify exposed API of HarfBuzzShaper

Move some of the implementation details of HarfBuzzShaper into cpp file
from header file and turn shapeRange into a private helper function.

Mark remaining methods as const to enforce and communicate that they do
not mutate the state of the object in any way.

BUG= 666758 
R=drott@chromium.org

Review-Url: https://codereview.chromium.org/2516723002
Cr-Commit-Position: refs/heads/master@{#433586}

[modify] https://crrev.com/6ed07c0f306b6995f1056ca51986bd3316d3a571/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
[modify] https://crrev.com/6ed07c0f306b6995f1056ca51986bd3316d3a571/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.h

Comment 4 by e...@chromium.org, Nov 21 2016

Components: Blink>Layout
As you've already noticed we're finally getting around to work on this ebrahim. Are there any specific edge cases you think we should be aware of or is the description in issue 6122 sufficient? Thank you.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 21 2016

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

commit a15729345604624f36e2d9b4902ed6a0ca2a2b3b
Author: eae <eae@chromium.org>
Date: Mon Nov 21 23:17:53 2016

Moving string normalization out of HarfBuzz

Move logic to normalize and upconvert strings out of HarfBuzzShaper into
TextRun as it logically isn't a part of the text shaping process.

This would also allow us to do it earlier in the processing pipeline for
LayoutNG. Either prior to shaping or even at whitespace collapsing time.

BUG= 666758 
R=drott@chromium.org,kojii@chromium.org

Review-Url: https://codereview.chromium.org/2507063010
Cr-Commit-Position: refs/heads/master@{#433685}

[modify] https://crrev.com/a15729345604624f36e2d9b4902ed6a0ca2a2b3b/third_party/WebKit/Source/platform/fonts/shaping/CachingWordShapeIterator.cpp
[modify] https://crrev.com/a15729345604624f36e2d9b4902ed6a0ca2a2b3b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
[modify] https://crrev.com/a15729345604624f36e2d9b4902ed6a0ca2a2b3b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.h
[modify] https://crrev.com/a15729345604624f36e2d9b4902ed6a0ca2a2b3b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp
[modify] https://crrev.com/a15729345604624f36e2d9b4902ed6a0ca2a2b3b/third_party/WebKit/Source/platform/text/TextRun.cpp
[modify] https://crrev.com/a15729345604624f36e2d9b4902ed6a0ca2a2b3b/third_party/WebKit/Source/platform/text/TextRun.h

Comment 6 by ebra...@gnu.org, Nov 22 2016

@#4: I have different things in mind that I see Firefox couldn't get it right but don't like to delay this to them at all that might be open issue and not worth to hassle anyway.

*  Issue 661717  (which is an issue on Firefox also)

* Shape across Pseudoelements:
data:text/html;charset=utf8,<style>div:before { content: '%D8%B4'; }</style><div>%D9%86</div>

* Shape across different font size and colors as surveyed here: https://www.w3.org/International/tests/repo/results/css-text-shaping

* It is better to not shape these together (Firefox fails on this, usually is the case with semantic designed menubar/tabs):

data:text/html;charset=utf8,<style>li { display: inline; margin: 1em; }</style><ul><li>%D8%B4</li><li>%D8%B4</li></ul>

* Across text nodes:
data:text/html;charset=utf-8,<body onload="document.body.appendChild(document.createTextNode('%D8%B3')); document.body.appendChild(document.createTextNode('%D8%B3'));" style="font-size: 1000%" />

* Dividing width for multi color elements:

data:text/html;charset=utf8,<span style="font-size: 400%;">%D8%B3%D9%84<span style="color: red;">%D8%A7</span>%D9%85

* Worth to be remembered I guess,

data:image/svg+xml;charset=utf8,<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="400" height="400"> <text x="100" y="100" font-size="28">سلام</text> <path fill="transparent" id="f" d="m100 200 L 200 200Z"/> <text font-size="28"> <textPath xlink:href="%23f">سلام</textPath> </text> </svg>

Issue 374526

(one approach can be progressing https://bugs.chromium.org/p/chromium/issues/detail?id=296863 and not doing shape across bidi-isolate, which doesn't seem to be specs intention, to other approach is considering CSS margin and padding as space)

All in all, I see these just as very very edge cases, web suffers much more on not resolving the main issue than considering these very edge cases so better to just have the main issue then optimize those over the time, but I thought these can be useful for upcoming LayoutNG design decisions that why I tried to list all I 

Comment 7 by e...@chromium.org, Aug 1 2017

Status: Fixed (was: Assigned)

Sign in to add a comment