Print Preview: Add option to rasterize PDFs and add JPEG compression.

pdf/pdfium/pdfium_engine.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "pdf/pdfium/pdfium_engine.h"
 
 #include <math.h>
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 39 matching lines @@
 #include "third_party/pdfium/public/fpdf_edit.h"
 #include "third_party/pdfium/public/fpdf_ext.h"
 #include "third_party/pdfium/public/fpdf_flatten.h"
 #include "third_party/pdfium/public/fpdf_ppo.h"
 #include "third_party/pdfium/public/fpdf_save.h"
 #include "third_party/pdfium/public/fpdf_searchex.h"
 #include "third_party/pdfium/public/fpdf_sysfontinfo.h"
 #include "third_party/pdfium/public/fpdf_transformpage.h"
 #include "third_party/pdfium/public/fpdfview.h"
 #include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/gfx/codec/jpeg_codec.h"
 #include "ui/gfx/geometry/rect.h"
 #include "v8/include/v8.h"
 
 using printing::ConvertUnit;
 using printing::ConvertUnitDouble;
 using printing::kPointsPerInch;
 using printing::kPixelsPerInch;
 
 namespace chrome_pdf {
 
@@ skipping 1276 matching lines @@
 
   DCHECK(defer_page_unload_);
   defer_page_unload_ = false;
   for (int page_index : deferred_page_unloads_)
     pages_[page_index]->Unload();
   deferred_page_unloads_.clear();
   return rv;
 }
 
 uint32_t PDFiumEngine::QuerySupportedPrintOutputFormats() {
   if (!HasPermission(PDFEngine::PERMISSION_PRINT_LOW_QUALITY))

[Tom Sepez, 2016/12/14 20:32:54]

nit: 
  Can a document have high_quality but not low_quality?  If so,
  can we assemble the permissions along the lines of

  uint32_t result = 0;
  if (HasPermission(PDFEngine::PERMISSION_PRINT_LOW_QUALITY))
    result |= PP_PRINTOUTPUTFORMAT_RASTER;
  if (HasPermission(PDFEngine::PERMISSION_PRINT_HIGH_QUALITY))
    result |= PP_PRINTOUTPUTFORMAT_PDF;
  return result;

which reads clearer to me -- and double check that I've got the right
permissions paired up above as I'm confused reading it.

[rbpotter, 2016/12/15 00:50:59]

Based on the logic in HasPermission, if a document has high quality it will
always also have low quality. I changed this anyway to make it more readable -
let me know if it is any better now.

[Tom Sepez, 2016/12/15 21:09:23]

Lots better, thanks.

     return 0;
-  return PP_PRINTOUTPUTFORMAT_PDF;
+  if (!HasPermission(PDFEngine::PERMISSION_PRINT_HIGH_QUALITY))
+    return PP_PRINTOUTPUTFORMAT_RASTER;
+  return PP_PRINTOUTPUTFORMAT_PDF | PP_PRINTOUTPUTFORMAT_RASTER;
 }
 
 void PDFiumEngine::PrintBegin() {
   FORM_DoDocumentAAction(form_, FPDFDOC_AACTION_WP);
 }
 
 pp::Resource PDFiumEngine::PrintPages(
     const PP_PrintPageNumberRange_Dev* page_ranges, uint32_t page_range_count,
     const PP_PrintSettings_Dev& print_settings) {
   ScopedSubstFont scoped_subst_font(this);
-  if (HasPermission(PDFEngine::PERMISSION_PRINT_HIGH_QUALITY))
+  if (HasPermission(PDFEngine::PERMISSION_PRINT_HIGH_QUALITY) &&
+      (print_settings.format & PP_PRINTOUTPUTFORMAT_PDF)) {
     return PrintPagesAsPDF(page_ranges, page_range_count, print_settings);
-  else if (HasPermission(PDFEngine::PERMISSION_PRINT_LOW_QUALITY))
+  } else if (HasPermission(PDFEngine::PERMISSION_PRINT_LOW_QUALITY)) {
     return PrintPagesAsRasterPDF(page_ranges, page_range_count, print_settings);
+  }
+
   return pp::Resource();
 }
 
+struct simple_buf {
+  uint32_t size;
+  uint8_t *data;

[Tom Sepez, 2016/12/14 20:32:54]

nit: uint8_t* data; per style guide (several places, e.g. 1391, 1437).

[rbpotter, 2016/12/15 00:50:58]

Done.

+};
+
+int GetBlockForJpeg(void *param,

[Tom Sepez, 2016/12/14 20:32:54]

the return type should probably be unsigned long to match |size|.

[rbpotter, 2016/12/15 00:50:59]

Unfortunately the signature is determined by the FPDF_FILEACCESS structure, so
it has to return an int. However it just needs to return something non-zero for
success, so I changed this to 1 to prevent the unsigned long to int cast.

+                    unsigned long pos,
+                    unsigned char* pBuf,

[Tom Sepez, 2016/12/14 20:32:55]

nit: uint8_t consistently throughout, hungarian notation only in pdfium itself
(eg. buf, not pBuf here), also this is probably const uint8_t* buf;

[rbpotter, 2016/12/15 00:50:59]

The signature for this function is defined by the FPDF_FILEACCESS structure as
using unsigned char* (and unsigned long). It will compile at least on Linux with
uint8_t* and size_t, but not sure if this is guaranteed to compile on all
platforms this way. Should I stick to the function signature from PDFium or
change it to be consistent with other variables?

+                    unsigned long size) {
+  simple_buf * buf_p = static_cast<simple_buf *>(param);
+  if (pos + size < pos || pos + size > buf_p->size)

[dsinclair, 2016/12/14 19:54:38]

The pos + size < pos seems like it would always be false? Should the second pos
be related to the buffer?

[Tom Sepez, 2016/12/14 20:32:54]

This is checking for overflow, and is allowed in this manner so long as the
types are unsigned, which they are.

+    return 0;
+  memcpy(pBuf, buf_p->data, size);

[dsinclair, 2016/12/14 19:54:38]

Should this be copying from buf_p->data + pos? Won't this always copy from the
beginning of the buffer?

[dsinclair, 2016/12/14 19:54:38]

Is it safe to assume pBuf is always big enough here?

[rbpotter, 2016/12/15 00:50:58]

Since the buffer is provided by the caller I don't think there is any way to
check from here; the assumption for the FPDF_FILEACCESS seems to be that the
caller will ensure the size of the buffer is at least equal to size.

[rbpotter, 2016/12/15 00:50:58]

Done.

[Tom Sepez, 2016/12/15 21:09:23]

I think this is still an issue.

[rbpotter, 2016/12/16 00:06:31]

This is now copying size bytes from buf_p->data.size + pos, so an offset of pos
bytes into the "file", to the user provided buffer. I believe this is correct
based on the description of this function in the definition of FPDF_FILEACCESS,
but let me know if I am missing something.

+  return size;
+}

[dsinclair, 2016/12/14 19:54:38]

Move up into the anonymous namespace.

[rbpotter, 2016/12/15 00:50:59]

Done.

+
 FPDF_DOCUMENT PDFiumEngine::CreateSinglePageRasterPdf(
     double source_page_width,
     double source_page_height,
     const PP_PrintSettings_Dev& print_settings,
     PDFiumPage* page_to_print) {
   FPDF_DOCUMENT temp_doc = FPDF_CreateNewDocument();
   if (!temp_doc)
     return temp_doc;
 
   const pp::Size& bitmap_size(page_to_print->rect().size());
@@ skipping 19 matching lines @@
   pp::Rect page_rect = page_to_print->rect();
   FPDF_RenderPageBitmap(bitmap,
                         page_to_print->GetPrintPage(),
                         page_rect.x(),
                         page_rect.y(),
                         page_rect.width(),
                         page_rect.height(),
                         print_settings.orientation,
                         FPDF_ANNOT | FPDF_PRINTING | FPDF_NO_CATCH);
 
+  unsigned char * bitmap_data =
+      static_cast<unsigned char *>(FPDFBitmap_GetBuffer(bitmap));
   double ratio_x = ConvertUnitDouble(bitmap_size.width(),
                                      print_settings.dpi,
                                      kPointsPerInch);
   double ratio_y = ConvertUnitDouble(bitmap_size.height(),
                                      print_settings.dpi,
                                      kPointsPerInch);
 
   // Add the bitmap to an image object and add the image object to the output
   // page.
   FPDF_PAGEOBJECT temp_img = FPDFPageObj_NewImgeObj(temp_doc);
-  FPDFImageObj_SetBitmap(&temp_page, 1, temp_img, bitmap);
+
+  std::vector <unsigned char> compressed_bitmap_data;

[Tom Sepez, 2016/12/14 20:32:54]

nit: std::vector<uint8_t>

Did you "git cl format" this?  I thought format would take care of these style
issues.

[rbpotter, 2016/12/15 00:50:58]

Done.

+  int quality = 40;
+  bool encode_success = gfx::JPEGCodec::Encode(bitmap_data,

[dsinclair, 2016/12/14 19:54:38]

Do we always want to do the JPEG encode, or only when the user selected the UI
option?

[Tom Sepez, 2016/12/14 20:32:54]

nit: local not needed

[rbpotter, 2016/12/15 00:50:58]

Not sure. The JPEG encode saves space for big/image heavy documents. However, it
is probably safer to keep the existing behavior unless the UI option is
selected, as a JPEG image printout will probably be expected in that case, while
it might not be if the document permissions are what is preventing a standard
PDF print. So I have changed it to only encode when the UI option is selected,
thanks for the suggestion.

+                                               gfx::JPEGCodec::FORMAT_BGRA,
+                                               FPDFBitmap_GetWidth(bitmap),
+                                               FPDFBitmap_GetHeight(bitmap),
+                                               FPDFBitmap_GetStride(bitmap),
+                                               quality,
+                                               &compressed_bitmap_data);
+
+  if (encode_success) {
+    simple_buf buffer;
+    buffer.size = compressed_bitmap_data.size();

[Tom Sepez, 2016/12/14 20:32:54]

beware, this truncates a size_t to an uint32_t.  You might want to make the
lenghts size_t throughout.

[rbpotter, 2016/12/15 00:50:58]

Done.

+    buffer.data = new uint8_t[buffer.size];

[Tom Sepez, 2016/12/14 20:36:28]

actually, we can set buffer.data to compressed_bitmap_data.data(), since vector
is guaranteed to be contiguous and avoid the alloc/copy/free.

[rbpotter, 2016/12/15 00:50:58]

Done.

+    memcpy(buffer.data, &(compressed_bitmap_data[0]), buffer.size);
+
+    FPDF_FILEACCESS file_access;
+    memset(&file_access, '\0', sizeof(file_access));
+    file_access.m_FileLen =
+        static_cast<unsigned long>(compressed_bitmap_data.size());
+    file_access.m_GetBlock = &GetBlockForJpeg;
+    file_access.m_Param = &buffer;
+
+    FPDFImageObj_LoadJpegFileInline(&temp_page, 1, temp_img, &file_access);
+    delete buffer.data;

[Tom Sepez, 2016/12/14 20:32:54]

delete[] buffer.data?

[rbpotter, 2016/12/15 00:50:59]

N/A

+  } else {
+    FPDFImageObj_SetBitmap(&temp_page, 1, temp_img, bitmap);
+  }
+
   FPDFImageObj_SetMatrix(temp_img, ratio_x, 0, 0, ratio_y, 0, 0);
   FPDFPage_InsertObject(temp_page, temp_img);
   FPDFPage_GenerateContent(temp_page);
   FPDF_ClosePage(temp_page);
 
   page_to_print->ClosePrintPage();
   FPDFBitmap_Destroy(bitmap);
 
   return temp_doc;
 }
@@ skipping 2702 matching lines @@
   FPDF_DOCUMENT doc =
       FPDF_LoadMemDocument(pdf_buffer, pdf_buffer_size, nullptr);
   if (!doc)
     return false;
   bool success = FPDF_GetPageSizeByIndex(doc, page_number, width, height) != 0;
   FPDF_CloseDocument(doc);
   return success;
 }
 
 }  // namespace chrome_pdf