New issue
Advanced search Search tips

Issue 784389 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 784386



Sign in to add a comment

Move OpenTypeVerticalData off of SimpleFontData

Project Member Reported by drott@chromium.org, Nov 13 2017

Issue description

For reasons originating back to when we had two font code paths, we still have OpenTypeVerticalData hanging off of SimpleFontData even though it's only used from HarfBuzzFace when HarfBuzz requests vertical layout information.

Instead of pre-initializing OpenTypeVerticalData in SimpleFontData when the font is potentially going to be used in a vertical context, we should move OpenTypeVerticalData to be managed or owned by HarfBuzzFace and initialized lazily when the first vertical metrics function is invoked.

Since OpenTypeVerticalData still requires fallback to regular (non-vertical layout) bounds from SimpleFontData - we first need to move bounds metrics off of SimpleFontData.


 
Components: Blink>Fonts

Comment 2 by drott@chromium.org, Nov 15 2017

Owner: drott@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 15 2017

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

commit 286d5f1bd727617d2a2ed464e3d5a267d0b52069
Author: Dominik Röttsches <drott@chromium.org>
Date: Wed Nov 15 12:57:22 2017

Move ascent & descent computation out from SimpleFontData to FontMetrics

OpenTypeVerticalData uses height (= ascent + descent) and ascent as
fallback metrics if no vertical layout specific information is found. In
order to be able to move OpenTypeVerticalData off of SimpleFontData, we
need to be able to access the same ascent & descent computation logic
from HarfBuzzFace. So this CL is a preparatory step for being able to do
that move.

No functional change.

Bug:  784389 
Change-Id: Ia3bce9dfa315a9a891ed970756b2c621d12d2090
Reviewed-on: https://chromium-review.googlesource.com/768820
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516678}
[modify] https://crrev.com/286d5f1bd727617d2a2ed464e3d5a267d0b52069/third_party/WebKit/Source/platform/BUILD.gn
[add] https://crrev.com/286d5f1bd727617d2a2ed464e3d5a267d0b52069/third_party/WebKit/Source/platform/fonts/FontMetrics.cpp
[modify] https://crrev.com/286d5f1bd727617d2a2ed464e3d5a267d0b52069/third_party/WebKit/Source/platform/fonts/FontMetrics.h
[modify] https://crrev.com/286d5f1bd727617d2a2ed464e3d5a267d0b52069/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
[modify] https://crrev.com/286d5f1bd727617d2a2ed464e3d5a267d0b52069/third_party/WebKit/Source/platform/fonts/SimpleFontData.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 15 2017

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

commit 45424ca29a623cba7f167cc1ca1651adcd291540
Author: Dominik Röttsches <drott@chromium.org>
Date: Wed Nov 15 14:22:40 2017

Move OpenTypeVerticalData off of SimpleFontData

OpenTypeVerticalData is only used from HarfBuzzFace for the two OpenType
vertical layout related callback functions. Since the plan is to clean
up the duality of FontPlatformData / SimpleFontData, the first step is
to clean up SimpleFontData of unnecessary semantics. This is the reason
for moving OpenTypeVerticalData. As a side effect it cleans up weird
caching logic and allows us to remove some adapter methods from
FontPlatformData as well.

Bug:  784389 
Change-Id: I3706024024e56d4efba781e923b0f2aea3cc6e48
Reviewed-on: https://chromium-review.googlesource.com/768822
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516690}
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/FontCache.cpp
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/FontCache.h
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/FontDataCache.cpp
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/FontDataCache.h
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/FontGlobalContext.h
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/FontPlatformData.cpp
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/FontPlatformData.h
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/SimpleFontData.h
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeVerticalData.cpp
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeVerticalData.h
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFontCache.h
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBloberizer.cpp
[modify] https://crrev.com/45424ca29a623cba7f167cc1ca1651adcd291540/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBloberizerTest.cpp

Comment 5 by drott@chromium.org, Nov 15 2017

Status: Fixed (was: Started)

Sign in to add a comment