Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(412)

Unified Diff: third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp

Issue 2643413002: Fix 'text-underline-position: under' to use em height ascent/descent (Closed)
Patch Set: Rebaseline Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp b/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
index d3224097198037903690caeb0d420a23b89898bf..db238b4fcb8cde4b40212ce38746767bd1139039 100644
--- a/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
+++ b/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
@@ -39,6 +39,7 @@
#include "platform/fonts/VDMXParser.h"
#include "platform/fonts/skia/SkiaTextMetrics.h"
#include "platform/geometry/FloatRect.h"
+#include "platform/wtf/ByteOrder.h"
#include "platform/wtf/MathExtras.h"
#include "platform/wtf/PtrUtil.h"
#include "platform/wtf/allocator/Partitions.h"
@@ -379,6 +380,84 @@ PassRefPtr<SimpleFontData> SimpleFontData::CreateScaledFontData(
IsCustomFont() ? CustomFontData::Create() : nullptr);
}
+// Internal leadings can be distributed to ascent and descent.
+// -------------------------------------------
+// | - Internal Leading (in ascent)
+// |--------------------------------
+// Ascent - | |
+// | |
+// | | - Em height
+// ----------|--------------|
+// | |
+// Descent - |--------------------------------
+// | - Internal Leading (in descent)
eae 2017/05/01 16:21:48 This illustration really helps, thank you!
+// -------------------------------------------
+// How Gecko computes "em height":
+// https://github.com/whatwg/html/issues/2470#issuecomment-291425136
+LayoutUnit SimpleFontData::EmHeightAscent(FontBaseline baseline_type) const {
+ if (baseline_type == kAlphabeticBaseline) {
+ if (!em_height_ascent_)
+ ComputeEmHeightMetrics();
+ return em_height_ascent_;
+ }
+ LayoutUnit em_height = LayoutUnit::FromFloatRound(PlatformData().size());
+ return em_height - em_height / 2;
+}
+
+LayoutUnit SimpleFontData::EmHeightDescent(FontBaseline baseline_type) const {
+ if (baseline_type == kAlphabeticBaseline) {
+ if (!em_height_descent_)
+ ComputeEmHeightMetrics();
+ return em_height_descent_;
+ }
+ LayoutUnit em_height = LayoutUnit::FromFloatRound(PlatformData().size());
+ return em_height / 2;
+}
+
+static std::pair<int16_t, int16_t> TypoAscenderAndDescender(
+ SkTypeface* typeface) {
+ // TODO(kojii): This should move to Skia once finalized. We can then move
+ // EmHeightAscender/Descender to FontMetrics.
+ int16_t buffer[2];
+ size_t size = typeface->getTableData(SkSetFourByteTag('O', 'S', '/', '2'), 68,
+ sizeof(buffer), buffer);
+ if (size == sizeof(buffer)) {
+ return std::make_pair(static_cast<int16_t>(ntohs(buffer[0])),
+ -static_cast<int16_t>(ntohs(buffer[1])));
+ }
+ return std::make_pair(0, 0);
+}
+
+void SimpleFontData::ComputeEmHeightMetrics() const {
+ // Compute em height metrics from OS/2 sTypoAscender and sTypoDescender.
+ SkTypeface* typeface = platform_data_.Typeface();
+ int16_t typo_ascender, typo_descender;
+ std::tie(typo_ascender, typo_descender) = TypoAscenderAndDescender(typeface);
+ if (typo_ascender > 0 &&
+ NormalizeEmHeightMetrics(typo_ascender, typo_ascender + typo_descender)) {
+ return;
+ }
+
+ // As the last resort, compute em height metrics from our ascent/descent.
+ const FontMetrics& font_metrics = GetFontMetrics();
+ if (NormalizeEmHeightMetrics(font_metrics.FloatAscent(),
+ font_metrics.FloatHeight())) {
+ return;
+ }
+ NOTREACHED();
+}
+
+bool SimpleFontData::NormalizeEmHeightMetrics(float ascent,
eae 2017/05/01 16:21:48 A comment explaining why normalization might be ne
kojii 2017/05/02 22:56:52 Done.
+ float height) const {
+ if (height <= 0 || ascent < 0 || ascent > height)
+ return false;
+ float em_height = PlatformData().size();
+ em_height_ascent_ = LayoutUnit::FromFloatRound(ascent * em_height / height);
+ em_height_descent_ =
+ LayoutUnit::FromFloatRound(em_height) - em_height_ascent_;
+ return true;
+}
+
FloatRect SimpleFontData::PlatformBoundsForGlyph(Glyph glyph) const {
if (!platform_data_.size())
return FloatRect();

Powered by Google App Engine
This is Rietveld 408576698