Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(15)

Issue 2066043002: Simplify CPDF_TextPage::FindTextlineFlowOrientation(). (Closed)

Created:
4 years, 7 months ago by Lei Zhang
Modified:
4 years, 7 months ago
Reviewers:
dsinclair
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium@master
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Simplify CPDF_TextPage::FindTextlineFlowOrientation(). Committed: https://pdfium.googlesource.com/pdfium/+/2fad11a8d9d2704cd9ee28b02373ad7ce19c65e3

Patch Set 1 #

Patch Set 2 : GetPageObjectList does not contain nullptrs #

Patch Set 3 : Fix tests #

Patch Set 4 : Fix bad refactoring some more #

Patch Set 5 : Slightly easier to read? #

Patch Set 6 : Better variable name #

Total comments: 14

Patch Set 7 : less loops #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -62 lines) Patch
M core/fpdftext/fpdf_text_int.cpp View 1 2 3 4 5 6 2 chunks +41 lines, -58 lines 0 comments Download
M fpdfsdk/fpdf_flatten.cpp View 1 1 chunk +1 line, -4 lines 0 comments Download

Messages

Total messages: 13 (3 generated)
Lei Zhang
4 years, 7 months ago (2016-06-14 21:41:41 UTC) #2
Lei Zhang
Nope, sent this too early as well. Got red bots.
4 years, 7 months ago (2016-06-14 21:42:10 UTC) #3
Lei Zhang
Ok, PTAL.
4 years, 7 months ago (2016-06-14 22:20:15 UTC) #4
dsinclair
Could you add a description of how this method works? As you can see by ...
4 years, 7 months ago (2016-06-15 13:37:50 UTC) #5
Lei Zhang
Kind of lazy about writing comments because a) this is getting more readable as is, ...
4 years, 7 months ago (2016-06-15 23:46:35 UTC) #6
dsinclair
https://codereview.chromium.org/2066043002/diff/40002/core/fpdftext/fpdf_text_int.cpp File core/fpdftext/fpdf_text_int.cpp (right): https://codereview.chromium.org/2066043002/diff/40002/core/fpdftext/fpdf_text_int.cpp#newcode715 core/fpdftext/fpdf_text_int.cpp:715: fLineHeight = pPageObj->m_Top - pPageObj->m_Bottom; On 2016/06/15 23:46:33, Lei ...
4 years, 7 months ago (2016-06-16 15:06:10 UTC) #7
Lei Zhang
https://codereview.chromium.org/2066043002/diff/40002/core/fpdftext/fpdf_text_int.cpp File core/fpdftext/fpdf_text_int.cpp (right): https://codereview.chromium.org/2066043002/diff/40002/core/fpdftext/fpdf_text_int.cpp#newcode720 core/fpdftext/fpdf_text_int.cpp:720: break; On 2016/06/16 15:06:10, dsinclair wrote: > On 2016/06/15 ...
4 years, 7 months ago (2016-06-16 22:47:04 UTC) #8
dsinclair
lgtm https://codereview.chromium.org/2066043002/diff/40002/core/fpdftext/fpdf_text_int.cpp File core/fpdftext/fpdf_text_int.cpp (right): https://codereview.chromium.org/2066043002/diff/40002/core/fpdftext/fpdf_text_int.cpp#newcode744 core/fpdftext/fpdf_text_int.cpp:744: const FX_FLOAT nSumH = MaskPercentFilled(nHorizontalMask, nStartH, nEndH); On ...
4 years, 7 months ago (2016-06-16 23:39:05 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2066043002/100001
4 years, 7 months ago (2016-06-16 23:39:09 UTC) #11
commit-bot: I haz the power
4 years, 7 months ago (2016-06-16 23:39:30 UTC) #13
Message was sent while issue was closed.
Committed patchset #7 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/2fad11a8d9d2704cd9ee28b02373ad7ce19c...

Powered by Google App Engine
This is Rietveld 408576698