commit a9d080a7f822f1106126e6d576c0c81aa4377b8c
author Lei Zhang <thestig@chromium.org> Sat Dec 16 16:36:26 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Sat Dec 16 16:36:26 2023 +0000
tree f27654742e947126d55c27a212bb13289cefd09e
parent 4870bd3e2d6461848e1552a887ebc26919adf70a

Deduplicate constants / macros between cfx_face.cpp and cfx_font.cpp

Move more CFX_Font::GetAscent() and CFX_Font::GetDescent() code into
CFX_Face. Then some temporarily duplicated code can be merged together
again. As part of this, change some if-statements into CHECKs, since the
if-statements in question cannot ever evaluate to true if one assumes
int is at least 32-bit.

Change-Id: I0374e361df9275e9f846ae5a9fe8c80a7c9e6928
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/114791
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>

core/fxge/cfx_face.cpp

diff --git a/core/fxge/cfx_face.cpp b/core/fxge/cfx_face.cpp
index d6f9fcb..075ae82 100644
--- a/core/fxge/cfx_face.cpp
+++ b/core/fxge/cfx_face.cpp
@@ -20,6 +20,7 @@
 #include "core/fxge/fx_fontencoding.h"
 #include "core/fxge/scoped_font_transform.h"
 #include "third_party/base/check.h"
+#include "third_party/base/check_op.h"
 #include "third_party/base/numerics/safe_conversions.h"
 #include "third_party/base/numerics/safe_math.h"
 
@@ -289,6 +290,20 @@
   return pdfium::base::checked_cast<int16_t>(GetRec()->descender);
 }
 
+int CFX_Face::GetAdjustedAscender() const {
+  int ascender = GetAscender();
+  CHECK_GE(ascender, kThousandthMinInt);
+  CHECK_LE(ascender, kThousandthMaxInt);
+  return EM_ADJUST(GetUnitsPerEm(), ascender);
+}
+
+int CFX_Face::GetAdjustedDescender() const {
+  int descender = GetDescender();
+  CHECK_GE(descender, kThousandthMinInt);
+  CHECK_LE(descender, kThousandthMaxInt);
+  return EM_ADJUST(GetUnitsPerEm(), descender);
+}
+
 #if BUILDFLAG(IS_ANDROID)
 int16_t CFX_Face::GetHeight() const {
   return pdfium::base::checked_cast<int16_t>(GetRec()->height);

core/fxge/cfx_face.h

diff --git a/core/fxge/cfx_face.h b/core/fxge/cfx_face.h
index 9534488..5b85a46 100644
--- a/core/fxge/cfx_face.h
+++ b/core/fxge/cfx_face.h
@@ -57,6 +57,8 @@
   uint16_t GetUnitsPerEm() const;
   int16_t GetAscender() const;
   int16_t GetDescender() const;
+  int GetAdjustedAscender() const;
+  int GetAdjustedDescender() const;
 #if BUILDFLAG(IS_ANDROID)
   int16_t GetHeight() const;
 #endif

core/fxge/cfx_font.cpp

diff --git a/core/fxge/cfx_font.cpp b/core/fxge/cfx_font.cpp
index e48a6bc..7bc1469 100644
--- a/core/fxge/cfx_font.cpp
+++ b/core/fxge/cfx_font.cpp
@@ -31,13 +31,8 @@
 #include "third_party/base/containers/span.h"
 #include "third_party/base/numerics/safe_conversions.h"
 
-#define EM_ADJUST(em, a) (em == 0 ? (a) : (a)*1000 / em)
-
 namespace {
 
-constexpr int kThousandthMinInt = std::numeric_limits<int>::min() / 1000;
-constexpr int kThousandthMaxInt = std::numeric_limits<int>::max() / 1000;
-
 FX_RECT FXRectFromFTPos(FT_Pos left, FT_Pos top, FT_Pos right, FT_Pos bottom) {
   return FX_RECT(pdfium::base::checked_cast<int32_t>(left),
                  pdfium::base::checked_cast<int32_t>(top),
@@ -280,25 +275,11 @@
 }
 
 int CFX_Font::GetAscent() const {
-  if (!m_Face)
-    return 0;
-
-  int ascender = m_Face->GetAscender();
-  if (ascender < kThousandthMinInt || ascender > kThousandthMaxInt)
-    return 0;
-
-  return EM_ADJUST(m_Face->GetUnitsPerEm(), ascender);
+  return m_Face ? m_Face->GetAdjustedAscender() : 0;
 }
 
 int CFX_Font::GetDescent() const {
-  if (!m_Face)
-    return 0;
-
-  int descender = m_Face->GetDescender();
-  if (descender < kThousandthMinInt || descender > kThousandthMaxInt)
-    return 0;
-
-  return EM_ADJUST(m_Face->GetUnitsPerEm(), descender);
+  return m_Face ? m_Face->GetAdjustedDescender() : 0;
 }
 
 absl::optional<FX_RECT> CFX_Font::GetGlyphBBox(uint32_t glyph_index) {