Split kerning data out of `CPDF_TextObject::char_positions_` [pdfium : main]
0 views
Skip to first unread message
Andy Phan (Gerrit)
Mar 16, 2026, 1:56:41 PM (yesterday) Mar 16
to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Lei Zhang
Andy Phan voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Lei Zhang
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Phan (Gerrit)
Mar 16, 2026, 1:57:56 PM (yesterday) Mar 16
to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Lei Zhang
Andy Phan added 1 comment![]( "Open in Gerrit")
File core/fpdfapi/page/cpdf_textobject.h
Line 87, Patchset 4 (Latest):
// - `char_kernings_` has N elements, where each value is the adjustmentAndy Phan . resolved
Is the last kerning value necessary? Could this actually be N-1? Although keeping it as N seems to makes things simpler.
Related details
Attention is currently required from:
- Lei Zhang
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Mar 16, 2026, 5:54:39 PM (yesterday) Mar 16
to Lei Zhang, Andy Phan, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Andy Phan
Lei Zhang added 1 comment![]( "Open in Gerrit")
File core/fpdfapi/page/cpdf_textobject.h
Line 87, Patchset 4:
// - `char_kernings_` has N elements, where each value is the adjustmentAndy Phan . resolved
Is the last kerning value necessary? Could this actually be N-1? Although keeping it as N seems to makes things simpler.
Lei Zhang
Probably, but it makes code that loops through the elements more complicated. I actually went in the opporsite direction and changed `char_positions_` to have N elements as well. See patchsets 5..7.
Related details
Attention is currently required from:
- Andy Phan
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Mar 16, 2026, 5:58:56 PM (yesterday) Mar 16
to Lei Zhang, Andy Phan, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Lei Zhang
Lei Zhang added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 7 (Latest):
Lei Zhang . resolved
Lousy human can't refactor code correctly...
Related details
Attention is currently required from:
- Lei Zhang
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
1:19 PM (8 hours ago) 1:19 PM
to Lei Zhang, Andy Phan, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Lei Zhang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Lei Zhang
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Phan (Gerrit)
1:33 PM (8 hours ago) 1:33 PM
to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Lei Zhang
Andy Phan voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Lei Zhang
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
2:21 PM (7 hours ago) 2:21 PM
to Lei Zhang, Andy Phan, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Lei Zhang voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Pdfium LUCI CQ (Gerrit)
2:22 PM (7 hours ago) 2:22 PM
to Lei Zhang, Andy Phan, pdfium-...@googlegroups.com
Pdfium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Split kerning data out of `CPDF_TextObject::char_positions_`
Currently, `char_positions_` contains both positions and kerning data,
which have different units of measurement. CPDF_TextObject stores
`CPDF_Font::kInvalidCharCode` in placeholder `char_codes_` entries to
indicate when a `char_positions_` entry is really kerning data. Instead
of doing this complicated scheme, separate out the kerning data into its
own vector, and then adjust all `char_positions_` and `char_codes_`
usage accordingly.
At the same time, get rid of the scheme where `char_positions_` has one
fewer element than `char_codes_`, and just always store a 0 there. Then
code that reads from `char_positions_` can be simplified and just read
the value instead of sometimes having to fill in the implicit 0.
This CL is almost entirely AI-generated, with some human touchups.
Change-Id: I60ed29921b9b55914ac8ebb48e624250ff907cc9
Commit-Queue: Lei Zhang <the...@chromium.org>
Reviewed-by: Andy Phan <andy...@chromium.org>
Files:
- M core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
- M core/fpdfapi/font/cpdf_font.cpp
- M core/fpdfapi/page/cpdf_textobject.cpp
- M core/fpdfapi/page/cpdf_textobject.h
- M core/fpdfapi/render/cpdf_renderstatus.cpp
- M core/fpdfapi/render/cpdf_textrenderer.cpp
- M core/fpdftext/cpdf_textpage.cpp
- M fpdfsdk/fpdf_edittext.cpp
Change size: M
Delta: 8 files changed, 47 insertions(+), 81 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Andy Phan
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages