Skip to content

Commit

Permalink
Add a use_color field to PdfRenderSettings.
Browse files Browse the repository at this point in the history
Set it properly in callers that require PDF rendering, and read it in
PDFiumEngine to pass the right rendering flags to PDFium.

BUG=805926,824643

Change-Id: Ia2e8a22259bef1ca78fe425592eeb407bbc4ba35
Reviewed-on: https://chromium-review.googlesource.com/1000555
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Jianzhou Feng <jzfeng@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550078}
  • Loading branch information
leizleiz authored and Commit Bot committed Apr 12, 2018
1 parent bd0ce3b commit 5c50048
Show file tree
Hide file tree
Showing 21 changed files with 114 additions and 44 deletions.
7 changes: 5 additions & 2 deletions chrome/browser/printing/cloud_print/privet_http_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -506,10 +506,13 @@ void PrivetLocalPrintOperationImpl::StartConvertToPWG() {
if (!pwg_raster_converter_)
pwg_raster_converter_ = PwgRasterConverter::CreateDefault();

printing::PwgRasterSettings bitmap_settings =
PwgRasterConverter::GetBitmapSettings(capabilities_, ticket_);
pwg_raster_converter_->Start(
data_.get(),
PwgRasterConverter::GetConversionSettings(capabilities_, page_size_),
PwgRasterConverter::GetBitmapSettings(capabilities_, ticket_),
PwgRasterConverter::GetConversionSettings(capabilities_, page_size_,
bitmap_settings.use_color),
bitmap_settings,
base::BindOnce(&PrivetLocalPrintOperationImpl::OnPWGRasterConverted,
weak_factory_.GetWeakPtr()));
}
Expand Down
39 changes: 32 additions & 7 deletions chrome/browser/printing/pdf_to_emf_converter_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, FailureBadPdf) {
IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, EmfBasic) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, PdfRenderSettings::Mode::NORMAL);
/*autorotate=*/false,
/*use_color=*/true, PdfRenderSettings::Mode::NORMAL);
constexpr int kNumberOfPages = 3;

ASSERT_TRUE(GetTestInput("pdf_converter_basic.pdf"));
Expand All @@ -276,7 +277,8 @@ IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, EmfBasic) {
IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, PostScriptLevel2Basic) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2);
/*autorotate=*/false, /*use_color=*/true,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2);
constexpr int kNumberOfPages = 3;

ASSERT_TRUE(GetTestInput("pdf_converter_basic.pdf"));
Expand All @@ -294,7 +296,8 @@ IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, PostScriptLevel2Basic) {
IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, PostScriptLevel3Basic) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
/*autorotate=*/false, /*use_color=*/true,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
constexpr int kNumberOfPages = 3;

ASSERT_TRUE(GetTestInput("pdf_converter_basic.pdf"));
Expand All @@ -309,11 +312,30 @@ IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, PostScriptLevel3Basic) {
}
}

IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, PostScriptLevel2Mono) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, /*use_color=*/false,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2);
RunSinglePagePdfToPostScriptConverterTest(pdf_settings, "bug_767343.pdf",
"bug_767343_mono.emf");
}

IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest, PostScriptLevel3Mono) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, /*use_color=*/false,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
RunSinglePagePdfToPostScriptConverterTest(pdf_settings, "bug_767343.pdf",
"bug_767343_mono.emf");
}

IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest,
PostScriptLevel2WithZeroSizedText) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2);
/*autorotate=*/false, /*use_color=*/true,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2);
RunSinglePagePdfToPostScriptConverterTest(pdf_settings, "bug_767343.pdf",
"bug_767343.emf");
}
Expand All @@ -322,7 +344,8 @@ IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest,
PostScriptLevel3WithZeroSizedText) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
/*autorotate=*/false, /*use_color=*/true,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
RunSinglePagePdfToPostScriptConverterTest(pdf_settings, "bug_767343.pdf",
"bug_767343.emf");
}
Expand All @@ -331,7 +354,8 @@ IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest,
PostScriptLevel2WithNegativeSizedText) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2);
/*autorotate=*/false, /*use_color=*/true,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2);
RunSinglePagePdfToPostScriptConverterTest(pdf_settings, "bug_806746.pdf",
"bug_806746.emf");
}
Expand All @@ -340,7 +364,8 @@ IN_PROC_BROWSER_TEST_F(PdfToEmfConverterBrowserTest,
PostScriptLevel3WithNegativeSizedText) {
const PdfRenderSettings pdf_settings(
kLetter200DpiRect, gfx::Point(0, 0), k200DpiSize,
/*autorotate=*/false, PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
/*autorotate=*/false, /*use_color=*/true,
PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
RunSinglePagePdfToPostScriptConverterTest(pdf_settings, "bug_806746.pdf",
"bug_806746.emf");
}
Expand Down
7 changes: 4 additions & 3 deletions chrome/browser/printing/print_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ void PrintJob::StartPdfToEmfConversion(
std::make_unique<PdfConversionState>(page_size, content_area);
PdfRenderSettings render_settings(
content_area, gfx::Point(0, 0), settings().dpi_size(),
/*autorotate=*/true,
/*autorotate=*/true, settings_.color() == COLOR,
print_text_with_gdi ? PdfRenderSettings::Mode::GDI_TEXT
: PdfRenderSettings::Mode::NORMAL);
pdf_conversion_state_->Start(
Expand Down Expand Up @@ -333,7 +333,8 @@ void PrintJob::StartPdfToTextConversion(
gfx::Rect page_area = gfx::Rect(0, 0, page_size.width(), page_size.height());
PdfRenderSettings render_settings(
page_area, gfx::Point(0, 0), settings().dpi_size(),
/*autorotate=*/true, PdfRenderSettings::Mode::TEXTONLY);
/*autorotate=*/true,
/*use_color=*/true, PdfRenderSettings::Mode::TEXTONLY);
pdf_conversion_state_->Start(
bytes, render_settings,
base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
Expand All @@ -349,7 +350,7 @@ void PrintJob::StartPdfToPostScriptConversion(
gfx::Size(), gfx::Rect());
PdfRenderSettings render_settings(
content_area, physical_offsets, settings().dpi_size(),
/*autorotate=*/true,
/*autorotate=*/true, settings_.color() == COLOR,
ps_level2 ? PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2
: PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
pdf_conversion_state_->Start(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,9 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
total_height_in_pixels += height_in_pixels;
gfx::Rect rect(width_in_pixels, height_in_pixels);
PdfRenderSettings settings(rect, gfx::Point(0, 0), gfx::Size(kDpi, kDpi),
true, PdfRenderSettings::Mode::NORMAL);
/*autorotate=*/false,
/*use_color=*/true,
PdfRenderSettings::Mode::NORMAL);

int int_max = std::numeric_limits<int>::max();
if (settings.area.width() > int_max / kColorChannels ||
Expand All @@ -385,7 +387,8 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
ASSERT_TRUE(chrome_pdf::RenderPDFPageToBitmap(
pdf_data.data(), pdf_data.size(), i, page_bitmap_data.data(),
settings.area.size().width(), settings.area.size().height(),
settings.dpi.width(), settings.dpi.height(), settings.autorotate));
settings.dpi.width(), settings.dpi.height(), settings.autorotate,
settings.use_color));
FillPng(&page_bitmap_data, width_in_pixels, max_width_in_pixels,
settings.area.size().height());
bitmap_data.insert(bitmap_data.end(),
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/printing/pwg_raster_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ std::unique_ptr<PwgRasterConverter> PwgRasterConverter::CreateDefault() {
// static
PdfRenderSettings PwgRasterConverter::GetConversionSettings(
const cloud_devices::CloudDeviceDescription& printer_capabilities,
const gfx::Size& page_size) {
const gfx::Size& page_size,
bool use_color) {
gfx::Size dpi = gfx::Size(kDefaultPdfDpi, kDefaultPdfDpi);
cloud_devices::printer::DpiCapability dpis;
if (dpis.LoadFrom(printer_capabilities))
Expand All @@ -264,7 +265,7 @@ PdfRenderSettings PwgRasterConverter::GetConversionSettings(
gfx::Rect area(final_page_size.width() * scale_x,
final_page_size.height() * scale_y);
return PdfRenderSettings(area, gfx::Point(0, 0), dpi,
/*autorotate=*/true,
/*autorotate=*/true, use_color,
PdfRenderSettings::Mode::NORMAL);
}

Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/printing/pwg_raster_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class PwgRasterConverter {
// TODO(vitalybuka): Extract page size from pdf document data.
static PdfRenderSettings GetConversionSettings(
const cloud_devices::CloudDeviceDescription& printer_capabilities,
const gfx::Size& page_size);
const gfx::Size& page_size,
bool use_color);

// Generates pwg bitmap settings to be used with the converter from
// device capabilites and printing ticket.
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/printing/pwg_raster_converter_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ IN_PROC_BROWSER_TEST_F(PdfToPwgRasterBrowserTest, TestSuccessColor) {
PdfRenderSettings pdf_settings(gfx::Rect(0, 0, 500, 500), gfx::Point(0, 0),
/*dpi=*/gfx::Size(1000, 1000),
/*autorotate=*/false,
/*use_color=*/true,
PdfRenderSettings::Mode::NORMAL);
PwgRasterSettings pwg_settings;
pwg_settings.odd_page_transform = PwgRasterTransformType::TRANSFORM_NORMAL;
Expand Down Expand Up @@ -145,6 +146,7 @@ IN_PROC_BROWSER_TEST_F(PdfToPwgRasterBrowserTest, TestSuccessMono) {
PdfRenderSettings pdf_settings(gfx::Rect(0, 0, 500, 500), gfx::Point(0, 0),
/*dpi=*/gfx::Size(1000, 1000),
/*autorotate=*/false,
/*use_color=*/false,
PdfRenderSettings::Mode::NORMAL);
PwgRasterSettings pwg_settings;
pwg_settings.odd_page_transform = PwgRasterTransformType::TRANSFORM_NORMAL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,16 @@ void ExtensionPrinterHandler::ConvertToPWGRaster(
const gfx::Size& page_size,
std::unique_ptr<extensions::PrinterProviderPrintJob> job,
PrintJobCallback callback) {
if (!pwg_raster_converter_) {
if (!pwg_raster_converter_)
pwg_raster_converter_ = PwgRasterConverter::CreateDefault();
}

printing::PwgRasterSettings bitmap_settings =
PwgRasterConverter::GetBitmapSettings(printer_description, ticket);
pwg_raster_converter_->Start(
data.get(),
PwgRasterConverter::GetConversionSettings(printer_description, page_size),
PwgRasterConverter::GetBitmapSettings(printer_description, ticket),
PwgRasterConverter::GetConversionSettings(printer_description, page_size,
bitmap_settings.use_color),
bitmap_settings,
base::BindOnce(&UpdateJobFileInfo, std::move(job), std::move(callback)));
}

Expand Down
10 changes: 6 additions & 4 deletions chrome/service/cloud_print/print_system_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@ class JobSpoolerWin : public PrintSystem::JobSpooler {
printer_dc_.Set(dc);
saved_dc_ = SaveDC(printer_dc_.Get());
delegate_ = delegate;
RenderPDFPages(print_data_file_path);
RenderPDFPages(print_data_file_path,
printing::IsDevModeWithColor(dev_mode.get()));
return true;
}

Expand Down Expand Up @@ -415,7 +416,7 @@ class JobSpoolerWin : public PrintSystem::JobSpooler {
delegate_ = nullptr;
}

void RenderPDFPages(const base::FilePath& pdf_path) {
void RenderPDFPages(const base::FilePath& pdf_path, bool use_color) {
gfx::Size printer_dpi =
gfx::Size(::GetDeviceCaps(printer_dc_.Get(), LOGPIXELSX),
::GetDeviceCaps(printer_dc_.Get(), LOGPIXELSY));
Expand All @@ -425,14 +426,15 @@ class JobSpoolerWin : public PrintSystem::JobSpooler {
PostIOThreadTask(
FROM_HERE,
base::BindOnce(&JobSpoolerWin::Core::RenderPDFPagesInSandbox, this,
pdf_path, render_area, printer_dpi,
pdf_path, render_area, printer_dpi, use_color,
base::ThreadTaskRunnerHandle::Get()));
}

void RenderPDFPagesInSandbox(
const base::FilePath& pdf_path,
const gfx::Rect& render_area,
const gfx::Size& render_dpi,
bool use_color,
const scoped_refptr<base::SingleThreadTaskRunner>& client_task_runner) {
DCHECK(CurrentlyOnServiceIOThread());
auto utility_host = std::make_unique<ServiceUtilityProcessHost>(
Expand All @@ -445,7 +447,7 @@ class JobSpoolerWin : public PrintSystem::JobSpooler {
if (utility_host->StartRenderPDFPagesToMetafile(
pdf_path, printing::PdfRenderSettings(
render_area, gfx::Point(0, 0), render_dpi,
/*autorotate=*/false,
/*autorotate=*/false, use_color,
printing::PdfRenderSettings::Mode::NORMAL))) {
// The object will self-destruct when the child process dies.
ignore_result(utility_host.release());
Expand Down
3 changes: 2 additions & 1 deletion chrome/services/printing/pdf_to_emf_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ bool PdfToEmfConverter::RenderPdfPageToMetafile(int page_number,
pdf_render_settings_.area.x() - offset_x,
pdf_render_settings_.area.y() - offset_y,
pdf_render_settings_.area.width(), pdf_render_settings_.area.height(),
true, false, true, true, pdf_render_settings_.autorotate)) {
true, false, true, true, pdf_render_settings_.autorotate,
pdf_render_settings_.use_color)) {
return false;
}
metafile.FinishPage();
Expand Down
2 changes: 1 addition & 1 deletion chrome/services/printing/pdf_to_pwg_raster_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ bool RenderPdfPagesToPwgRaster(base::File pdf_file,
if (!chrome_pdf::RenderPDFPageToBitmap(
data.data(), data_size, page_number, image.pixel_data(),
image.size().width(), image.size().height(), settings.dpi.width(),
settings.dpi.height(), settings.autorotate)) {
settings.dpi.height(), settings.autorotate, settings.use_color)) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ struct PdfRenderSettings {
gfx.mojom.Point offsets;
gfx.mojom.Size dpi;
bool autorotate;
bool use_color;
Mode mode;
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ bool StructTraits<printing::mojom::PdfRenderSettingsDataView,
Read(printing::mojom::PdfRenderSettingsDataView data,
printing::PdfRenderSettings* out) {
out->autorotate = data.autorotate();
out->use_color = data.use_color();
return data.ReadArea(&out->area) && data.ReadOffsets(&out->offsets) &&
data.ReadDpi(&out->dpi) && data.ReadMode(&out->mode);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ class StructTraits<printing::mojom::PdfRenderSettingsDataView,
static bool autorotate(const printing::PdfRenderSettings& settings) {
return settings.autorotate;
}
static bool use_color(const printing::PdfRenderSettings& settings) {
return settings.use_color;
}
static printing::PdfRenderSettings::Mode mode(
const printing::PdfRenderSettings& settings) {
return settings.mode;
Expand Down
Binary file added chrome/test/data/printing/bug_767343_mono.emf
Binary file not shown.
7 changes: 4 additions & 3 deletions headless/lib/headless_web_contents_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -505,14 +505,15 @@ class HeadlessWebContentsPDFTest : public HeadlessAsyncDevTooledBrowserTest {

gfx::Rect rect(kPaperWidth * kDpi, kPaperHeight * kDpi);
printing::PdfRenderSettings settings(
rect, gfx::Point(0, 0), gfx::Size(kDpi, kDpi), true,
printing::PdfRenderSettings::Mode::NORMAL);
rect, gfx::Point(0, 0), gfx::Size(kDpi, kDpi), /*autorotate=*/true,
/*use_color=*/true, printing::PdfRenderSettings::Mode::NORMAL);
std::vector<uint8_t> page_bitmap_data(kColorChannels *
settings.area.size().GetArea());
EXPECT_TRUE(chrome_pdf::RenderPDFPageToBitmap(
pdf_data.data(), pdf_data.size(), i, page_bitmap_data.data(),
settings.area.size().width(), settings.area.size().height(),
settings.dpi.width(), settings.dpi.height(), settings.autorotate));
settings.dpi.width(), settings.dpi.height(), settings.autorotate,
settings.use_color));
EXPECT_EQ(0x56, page_bitmap_data[0]); // B
EXPECT_EQ(0x34, page_bitmap_data[1]); // G
EXPECT_EQ(0x12, page_bitmap_data[2]); // R
Expand Down
10 changes: 6 additions & 4 deletions pdf/pdf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ bool RenderPDFPageToDC(const void* pdf_buffer,
bool stretch_to_bounds,
bool keep_aspect_ratio,
bool center_in_bounds,
bool autorotate) {
bool autorotate,
bool use_color) {
if (!IsSDKInitializedViaPepper()) {
if (!InitializeSDK()) {
return false;
Expand All @@ -42,7 +43,7 @@ bool RenderPDFPageToDC(const void* pdf_buffer,
dpi_x, dpi_y,
pp::Rect(bounds_origin_x, bounds_origin_y, bounds_width, bounds_height),
fit_to_bounds, stretch_to_bounds, keep_aspect_ratio, center_in_bounds,
autorotate);
autorotate, use_color);
bool ret = engine_exports->RenderPDFPageToDC(pdf_buffer, buffer_size,
page_number, settings, dc);
if (!IsSDKInitializedViaPepper())
Expand Down Expand Up @@ -108,15 +109,16 @@ bool RenderPDFPageToBitmap(const void* pdf_buffer,
int bitmap_height,
int dpi_x,
int dpi_y,
bool autorotate) {
bool autorotate,
bool use_color) {
if (!IsSDKInitializedViaPepper()) {
if (!InitializeSDK())
return false;
}
PDFEngineExports* engine_exports = PDFEngineExports::Get();
PDFEngineExports::RenderingSettings settings(
dpi_x, dpi_y, pp::Rect(bitmap_width, bitmap_height), true, false, true,
true, autorotate);
true, autorotate, use_color);
bool ret = engine_exports->RenderPDFPageToBitmap(
pdf_buffer, pdf_buffer_size, page_number, settings, bitmap_buffer);
if (!IsSDKInitializedViaPepper())
Expand Down
Loading

0 comments on commit 5c50048

Please sign in to comment.