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

Issue 1644403003: Some cleanup of fpdf_render_loadimage.cpp (Closed)

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

Description

Some cleanup of fpdf_render_loadimage.cpp - Generalise GetBits8() - Get rid of C-style casts. - Make CFX_DIBSource::SetDownSampleSize() non const. It's only overriden once and called in one place and it doesn't make sense for it to be const. - Get rid of a macro - Make public member vars of CPDF_DIBSource private - And others... R=thestig@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/487935f662ba4711caf1c2c06873b676fd3fba3e

Patch Set 1 #

Patch Set 2 : typo #

Total comments: 9

Patch Set 3 : comments #

Total comments: 2

Patch Set 4 : more constructor cleanup #

Unified diffs Side-by-side diffs Delta from patch set Stats (+209 lines, -188 lines) Patch
M core/include/fxge/fx_dib.h View 1 chunk +1 line, -1 line 0 comments Download
M core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp View 1 2 3 52 chunks +195 lines, -175 lines 0 comments Download
M core/src/fpdfapi/fpdf_render/render_int.h View 1 2 4 chunks +12 lines, -11 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
Oliver Chang
ptal
4 years, 10 months ago (2016-01-29 18:15:40 UTC) #1
Lei Zhang
https://codereview.chromium.org/1644403003/diff/20001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right): https://codereview.chromium.org/1644403003/diff/20001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp#newcode124 core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:124: CPDF_DIBSource* pSource = new CPDF_DIBSource; Do you want to ...
4 years, 10 months ago (2016-01-30 00:39:25 UTC) #2
Oliver Chang
https://codereview.chromium.org/1644403003/diff/20001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right): https://codereview.chromium.org/1644403003/diff/20001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp#newcode124 core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:124: CPDF_DIBSource* pSource = new CPDF_DIBSource; On 2016/01/30 00:39:25, Lei ...
4 years, 10 months ago (2016-01-30 01:48:19 UTC) #3
Lei Zhang
lgtm https://codereview.chromium.org/1644403003/diff/20001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right): https://codereview.chromium.org/1644403003/diff/20001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp#newcode159 core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:159: m_pDocument = nullptr; On 2016/01/30 01:48:18, Oliver Chang ...
4 years, 10 months ago (2016-01-30 02:08:33 UTC) #4
Oliver Chang
https://codereview.chromium.org/1644403003/diff/40001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right): https://codereview.chromium.org/1644403003/diff/40001/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp#newcode183 core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:183: m_bpp = 0; On 2016/01/30 02:08:33, Lei Zhang wrote: ...
4 years, 10 months ago (2016-01-30 02:18:33 UTC) #5
Oliver Chang
And thanks for the review, as always :)
4 years, 10 months ago (2016-01-30 02:19:08 UTC) #6
Oliver Chang
4 years, 10 months ago (2016-01-30 02:25:10 UTC) #9
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as
487935f662ba4711caf1c2c06873b676fd3fba3e (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698