final fixups

- add buffer load
- test under valgrind
- code sharing with the poppler pdf loader

ChangeLog

@@ -14,6 +14,7 @@
 - create funcs always make MULTIBAND (ie. no alpha)
 - use O_TMPFILE, if available [Alexander--]
 - set "interlaced=1" for interlaced JPG and PNG images
+- add PDFium PDF loader
 
 12/3/18 started 8.6.4
 - better fitting of fonts with overhanging edges [Adrià]

README.md

@@ -212,7 +212,7 @@ If PDFium is not detected, libvips will look for poppler-glib instead.
 ### libpoppler
 
 The usual PDF loader. If this is not present, vips will try to load PDFs
-via imagemagick instead.
+via imagemagick.
 
 ### libgsf-1
 

libvips/foreign/pdfload.c

@@ -55,6 +55,8 @@
 #include <vips/buf.h>
 #include <vips/internal.h>
 
+#include "pforeign.h"
+
 #ifdef HAVE_POPPLER
 
 #include <cairo.h>
@@ -91,7 +93,7 @@ typedef struct _VipsForeignLoadPdf {
 	 */
 	int n_pages;
 
-	/* We need to read out the side of each page we will render, and lay
+	/* We need to read out the size of each page we will render, and lay
 	 * them out in the final image.
 	 */
 	VipsRect image;
@@ -145,33 +147,6 @@ vips_foreign_load_pdf_get_flags( VipsForeignLoad *load )
 	return( VIPS_FOREIGN_PARTIAL );
 }
 
-static gboolean
-vips_foreign_load_pdf_is_a_buffer( const void *buf, size_t len )
-{
-	const guchar *str = (const guchar *) buf;
-
-	if( len >= 4 &&
-		str[0] == '%' && 
-		str[1] == 'P' &&
-		str[2] == 'D' &&
-		str[3] == 'F' )
-		return( 1 );
-
-	return( 0 );
-}
-
-static gboolean
-vips_foreign_load_pdf_is_a( const char *filename )
-{
-	unsigned char buf[4];
-
-	if( vips__get_bytes( filename, buf, 4 ) == 4 &&
-		vips_foreign_load_pdf_is_a_buffer( buf, 4 ) )
-		return( 1 );
-
-	return( 0 );
-}
-
 static int
 vips_foreign_load_pdf_get_page( VipsForeignLoadPdf *pdf, int page_no )
 {
@@ -429,7 +404,7 @@ vips_foreign_load_pdf_load( VipsForeignLoad *load )
 	 * (again) keep the number of calls to page_render low. 
 	 */
 	if( vips_linecache( t[0], &t[1],
-		"tile_height", 5000,
+		"tile_height", VIPS_MIN( 5000, pdf->pages[0].height ), 
 		NULL ) ) 
 		return( -1 );
 	if( vips_image_write( t[1], load->real ) ) 
@@ -658,6 +633,37 @@ vips_foreign_load_pdf_buffer_init( VipsForeignLoadPdfBuffer *buffer )
 
 #endif /*HAVE_POPPLER*/
 
+/* Also used by the pdfium loader.
+ */
+gboolean
+vips_foreign_load_pdf_is_a_buffer( const void *buf, size_t len )
+{
+	const guchar *str = (const guchar *) buf;
+
+	if( len >= 4 &&
+		str[0] == '%' && 
+		str[1] == 'P' &&
+		str[2] == 'D' &&
+		str[3] == 'F' )
+		return( 1 );
+
+	return( 0 );
+}
+
+/* Also used by the pdfium loader.
+ */
+gboolean
+vips_foreign_load_pdf_is_a( const char *filename )
+{
+	unsigned char buf[4];
+
+	if( vips__get_bytes( filename, buf, 4 ) == 4 &&
+		vips_foreign_load_pdf_is_a_buffer( buf, 4 ) )
+		return( 1 );
+
+	return( 0 );
+}
+
 /**
  * vips_pdfload:
  * @filename: file to load

libvips/foreign/pdfload_pdfium.c

@@ -33,14 +33,8 @@
 
 /* TODO 
  *
- * - more code sharing with pdfload.c, eg. vips_foreign_load_pdf_is_a_buffer()
- *   and get_flags etc.
- * - could share the page layout code too
- * - make pdf.c with base stuff in?
  * - what about filename encodings
- * - I guess we must write RGBA to match poppler output
+ * - need to test on Windows
- * - new_from_buffer stuff
- *
  */
 
 /*
@@ -57,14 +51,12 @@
 #include <string.h>
 #include <errno.h>
 
-/* Just until we get rid of the final bits of poppler
- */
-#include <cairo.h>
-
 #include <vips/vips.h>
 #include <vips/buf.h>
 #include <vips/internal.h>
 
+#include "pforeign.h"
+
 #ifdef HAVE_PDFIUM
 
 #include <fpdfview.h>
@@ -97,7 +89,7 @@ typedef struct _VipsForeignLoadPdf {
 	 */
 	int n_pages;
 
-	/* We need to read out the side of each page we will render, and lay
+	/* We need to read out the size of each page we will render, and lay
 	 * them out in the final image.
 	 */
 	VipsRect image;
@@ -137,8 +129,8 @@ vips_foreign_load_pdf_dispose( GObject *gobject )
 {
 	VipsForeignLoadPdf *pdf = (VipsForeignLoadPdf *) gobject;
 
-	VIPS_FREEF( FPDF_CloseDocument, pdf->doc ); 
 	VIPS_FREEF( FPDF_ClosePage, pdf->page ); 
+	VIPS_FREEF( FPDF_CloseDocument, pdf->doc ); 
 
 	G_OBJECT_CLASS( vips_foreign_load_pdf_parent_class )->dispose( gobject );
 }
@@ -180,7 +172,8 @@ vips_foreign_load_pdf_build( VipsObject *object )
 static VipsForeignFlags
 vips_foreign_load_pdf_get_flags_filename( const char *filename )
 {
-	/* We can render any part of the page on demand.
+	/* We can't render any part of the page on demand, but we can render
+	 * separate pages. Might as well call ourselves partial.
 	 */
 	return( VIPS_FOREIGN_PARTIAL );
 }
@@ -191,33 +184,6 @@ vips_foreign_load_pdf_get_flags( VipsForeignLoad *load )
 	return( VIPS_FOREIGN_PARTIAL );
 }
 
-static gboolean
-vips_foreign_load_pdf_is_a_buffer( const void *buf, size_t len )
-{
-	const guchar *str = (const guchar *) buf;
-
-	if( len >= 4 &&
-		str[0] == '%' && 
-		str[1] == 'P' &&
-		str[2] == 'D' &&
-		str[3] == 'F' )
-		return( 1 );
-
-	return( 0 );
-}
-
-static gboolean
-vips_foreign_load_pdf_is_a( const char *filename )
-{
-	unsigned char buf[4];
-
-	if( vips__get_bytes( filename, buf, 4 ) == 4 &&
-		vips_foreign_load_pdf_is_a_buffer( buf, 4 ) )
-		return( 1 );
-
-	return( 0 );
-}
-
 static int
 vips_foreign_load_pdf_get_page( VipsForeignLoadPdf *pdf, int page_no )
 {
@@ -246,8 +212,8 @@ vips_foreign_load_pdf_get_page( VipsForeignLoadPdf *pdf, int page_no )
 /* String-based metadata fields we extract.
  */
 typedef struct _VipsForeignLoadPdfMetadata {
-	char *tag;		// as understood by PDFium
+	char *tag;		/* as understood by PDFium */
-	char *field;		// as understood by libvips
+	char *field;		/* as understood by libvips */
 } VipsForeignLoadPdfMetadata;
 
 static VipsForeignLoadPdfMetadata vips_foreign_load_pdf_metadata[] = {
@@ -257,7 +223,7 @@ static VipsForeignLoadPdfMetadata vips_foreign_load_pdf_metadata[] = {
 	{ "Keywords", "pdf-keywords" },
 	{ "Creator", "pdf-creator" },
 	{ "Producer", "pdf-producer" },
-	// poppler has "metadata" as well, but pdfium does not support this
+	/* poppler has "metadata" as well, but pdfium does not support this */
 };
 static int n_metadata = VIPS_NUMBER( vips_foreign_load_pdf_metadata );
 
@@ -440,6 +406,8 @@ vips_foreign_load_pdf_generate( VipsRegion *or,
 	}
 
 	/* PDFium writes BRGA, we must swap.
+	 *
+	 * FIXME ... this is a bit slow.
 	 */
 	for( y = 0; y < r->height; y++ ) {
 		VipsPel *p;
@@ -634,17 +602,17 @@ G_DEFINE_TYPE( VipsForeignLoadPdfBuffer, vips_foreign_load_pdf_buffer,
 static int
 vips_foreign_load_pdf_buffer_header( VipsForeignLoad *load )
 {
-	/*
 	VipsForeignLoadPdf *pdf = (VipsForeignLoadPdf *) load;
 	VipsForeignLoadPdfBuffer *buffer = 
 		(VipsForeignLoadPdfBuffer *) load;
 
-	if( !(pdf->doc = poppler_document_new_from_data( 
+	if( !(pdf->doc = FPDF_LoadMemDocument( buffer->buf->data, 
-		buffer->buf->data, buffer->buf->length, NULL, &error )) ) { 
+		buffer->buf->length, NULL )) ) { 
-		vips_g_error( &error );
+		vips_pdfium_error();
+		vips_error( "pdfload", 
+			"%s", _( "unable to load from buffer" ) );
 		return( -1 ); 
 	}
-	 */
 
 	return( vips_foreign_load_pdf_header( load ) );
 }

libvips/foreign/pforeign.h

@@ -239,6 +239,9 @@ int vips__openslide_read( const char *filename, VipsImage *out,
 int vips__openslide_read_associated( const char *filename, VipsImage *out, 
 	const char *associated );
 
+gboolean vips_foreign_load_pdf_is_a_buffer( const void *buf, size_t len );
+gboolean vips_foreign_load_pdf_is_a( const char *filename );
+
 #ifdef __cplusplus
 }
 #endif /*__cplusplus*/