Replace the EM_ADJUST macro with a function [pdfium : main]

0 views
Skip to first unread message

Aryan Krishnan (Gerrit)

unread,
12:15 PM (4 hours ago) 12:15 PM
to Lei Zhang, Tom Sepez, Andy Phan, pdfium-...@googlegroups.com
Attention needed from Andy Phan, Lei Zhang and Tom Sepez

Aryan Krishnan voted and added 1 comment

Votes added by Aryan Krishnan

Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Aryan Krishnan . resolved

Hey guys! Working on continuing removing the defines. Have kept the CFX_Face::EmAdjust() function public for now so that other files that I have found can also use this function. Thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Andy Phan
  • Lei Zhang
  • Tom Sepez
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: pdfium
Gerrit-Branch: main
Gerrit-Change-Id: Id5f1b472391a4da169e977188ea6d46db6a8c129
Gerrit-Change-Number: 149110
Gerrit-PatchSet: 2
Gerrit-Owner: Aryan Krishnan <aryankr...@gmail.com>
Gerrit-Reviewer: Andy Phan <andy...@chromium.org>
Gerrit-Reviewer: Aryan Krishnan <aryankr...@gmail.com>
Gerrit-Reviewer: Lei Zhang <the...@chromium.org>
Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>
Gerrit-Attention: Lei Zhang <the...@chromium.org>
Gerrit-Attention: Tom Sepez <tse...@chromium.org>
Gerrit-Attention: Andy Phan <andy...@chromium.org>
Gerrit-Comment-Date: Thu, 04 Jun 2026 16:15:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
unsatisfied_requirement
open
diffy

Andy Phan (Gerrit)

unread,
1:56 PM (2 hours ago) 1:56 PM
to Aryan Krishnan, pdfium...@luci-project-accounts.iam.gserviceaccount.com, Lei Zhang, Tom Sepez, pdfium-...@googlegroups.com
Attention needed from Aryan Krishnan, Lei Zhang and Tom Sepez

Andy Phan added 2 comments

Patchset-level comments
Andy Phan . unresolved

The CL seems fine as a mechanical change, but maybe this should actually use NormalizeFontMetric() instead. There's multiple uses of it in the same file.

File core/fxge/cfx_face.cpp
Line 9, Patchset 2 (Latest):#include <cmath>
Andy Phan . resolved

Unrelated to this CL, but maybe for std::abs()?

Open in Gerrit

Related details

Attention is currently required from:
  • Aryan Krishnan
  • Lei Zhang
  • Tom Sepez
Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: pdfium
    Gerrit-Branch: main
    Gerrit-Change-Id: Id5f1b472391a4da169e977188ea6d46db6a8c129
    Gerrit-Change-Number: 149110
    Gerrit-PatchSet: 2
    Gerrit-Owner: Aryan Krishnan <aryankr...@gmail.com>
    Gerrit-Reviewer: Andy Phan <andy...@chromium.org>
    Gerrit-Reviewer: Aryan Krishnan <aryankr...@gmail.com>
    Gerrit-Reviewer: Lei Zhang <the...@chromium.org>
    Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>
    Gerrit-Attention: Lei Zhang <the...@chromium.org>
    Gerrit-Attention: Tom Sepez <tse...@chromium.org>
    Gerrit-Attention: Aryan Krishnan <aryankr...@gmail.com>
    Gerrit-Comment-Date: Thu, 04 Jun 2026 17:56:05 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    open
    diffy

    Aryan Krishnan (Gerrit)

    unread,
    2:18 PM (2 hours ago) 2:18 PM
    to pdfium...@luci-project-accounts.iam.gserviceaccount.com, Lei Zhang, Tom Sepez, Andy Phan, pdfium-...@googlegroups.com
    Attention needed from Andy Phan, Lei Zhang and Tom Sepez

    Aryan Krishnan voted and added 2 comments

    Votes added by Aryan Krishnan

    Commit-Queue+1

    2 comments

    Patchset-level comments

    The CL seems fine as a mechanical change, but maybe this should actually use NormalizeFontMetric() instead. There's multiple uses of it in the same file.

    Aryan Krishnan

    I assume you meant replace EmAdjust with NormalizeFontMetric. Done, also repurposed the <cmath> include for `std::roundll()` as a way to combine the + 0.5 and static_cast<> steps.

    Aryan Krishnan

    Done

    Aryan Krishnan

    Replacing it with NormalizeFontMetric is causing a couple tests to fail. Undoing this.

    File core/fxge/cfx_face.cpp
    Line 9, Patchset 2:#include <cmath>
    Andy Phan . resolved

    Unrelated to this CL, but maybe for std::abs()?

    Aryan Krishnan

    Not sure what you meant but the import was just me experimenting with `std::roundll()`, which I now added in the latest patchset.

    Aryan Krishnan

    Nevermind, removed.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Andy Phan
    • Lei Zhang
    • Tom Sepez
    Submit Requirements:
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: pdfium
      Gerrit-Branch: main
      Gerrit-Change-Id: Id5f1b472391a4da169e977188ea6d46db6a8c129
      Gerrit-Change-Number: 149110
      Gerrit-PatchSet: 5
      Gerrit-Owner: Aryan Krishnan <aryankr...@gmail.com>
      Gerrit-Reviewer: Andy Phan <andy...@chromium.org>
      Gerrit-Reviewer: Aryan Krishnan <aryankr...@gmail.com>
      Gerrit-Reviewer: Lei Zhang <the...@chromium.org>
      Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>
      Gerrit-Attention: Lei Zhang <the...@chromium.org>
      Gerrit-Attention: Tom Sepez <tse...@chromium.org>
      Gerrit-Attention: Andy Phan <andy...@chromium.org>
      Gerrit-Comment-Date: Thu, 04 Jun 2026 18:18:33 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Comment-In-Reply-To: Aryan Krishnan <aryankr...@gmail.com>
      Comment-In-Reply-To: Andy Phan <andy...@chromium.org>
      unsatisfied_requirement
      open
      diffy

      Andy Phan (Gerrit)

      unread,
      2:19 PM (2 hours ago) 2:19 PM
      to Aryan Krishnan, pdfium...@luci-project-accounts.iam.gserviceaccount.com, Lei Zhang, Tom Sepez, pdfium-...@googlegroups.com
      Attention needed from Aryan Krishnan, Lei Zhang and Tom Sepez

      Andy Phan added 1 comment

      Patchset-level comments
      Andy Phan . resolved

      The CL seems fine as a mechanical change, but maybe this should actually use NormalizeFontMetric() instead. There's multiple uses of it in the same file.

      Aryan Krishnan

      I assume you meant replace EmAdjust with NormalizeFontMetric. Done, also repurposed the <cmath> include for `std::roundll()` as a way to combine the + 0.5 and static_cast<> steps.

      Aryan Krishnan

      Done

      Aryan Krishnan

      Replacing it with NormalizeFontMetric is causing a couple tests to fail. Undoing this.

      Andy Phan

      Any idea on why they fail?

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Aryan Krishnan
      • Lei Zhang
      • Tom Sepez
      Submit Requirements:
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: pdfium
      Gerrit-Branch: main
      Gerrit-Change-Id: Id5f1b472391a4da169e977188ea6d46db6a8c129
      Gerrit-Change-Number: 149110
      Gerrit-PatchSet: 5
      Gerrit-Owner: Aryan Krishnan <aryankr...@gmail.com>
      Gerrit-Reviewer: Andy Phan <andy...@chromium.org>
      Gerrit-Reviewer: Aryan Krishnan <aryankr...@gmail.com>
      Gerrit-Reviewer: Lei Zhang <the...@chromium.org>
      Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>
      Gerrit-Attention: Lei Zhang <the...@chromium.org>
      Gerrit-Attention: Tom Sepez <tse...@chromium.org>
      Gerrit-Attention: Aryan Krishnan <aryankr...@gmail.com>
      Gerrit-Comment-Date: Thu, 04 Jun 2026 18:19:48 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages