Hey Adrian & Carlos,
Thanks for your comments!
Carlos wrote:
[...] we don't need to pass a TextPage to CairoOutputDev when rendering, since
we are not interested in the text.
Removed TextPage instance.
Adrian wrote:
1. PS/PDF should output all pages to the one file as these file types
support multiple pages. SVG also supports multipage if
cairo_svg_surface_restrict_to_version() is called to set the version to
1.2.
Agreed, but this is a feature request, not a bug. Once this patchset has
been accepted, I'll consider adding a patch to add a "-multipage"
command line option - or you're welcome to contribute it if you like.
I believe the default should be individual files per page so that the
tool behaves the same for all formats unless multipage files are
specifically requested in which case we can output an error if the
target format does not support multipage output.
2. Memory leak - a surface is created for each page in start_page() but
only the last page is destroyed.
Fixed.
3. For correct handling of PDFs with transparency, the jpeg surface
should use ARGB32 the same as PNG. Remove the code that paints a white
background if the output is jpeg. There is already code at line 272 in
render_page() that blends the image into a white background.
Fixed.
Also fixed a compiler warning (unused local variable) and some very
minor memory leaks. Patch attached, please apply on top of original
patchset.
Cheers,
Stefan
From 09c916c7c3b892f42b610adc06cf29bb4efb6a9a Mon Sep 17 00:00:00 2001
From: Stefan Thomas <[email protected]>
Date: Fri, 16 Jul 2010 19:53:22 +0100
Subject: [PATCH] Minor fixes based on reviewer feedback (memory leaks, removed
TextPage instance).
---
utils/pdftocairo.cc | 37 +++++++++++++------------------------
1 files changed, 13 insertions(+), 24 deletions(-)
diff --git a/utils/pdftocairo.cc b/utils/pdftocairo.cc
index 8e5db15..dd9ef41 100644
--- a/utils/pdftocairo.cc
+++ b/utils/pdftocairo.cc
@@ -135,7 +135,7 @@ static const ArgDesc argDesc[] = {
};
-static void format_output_filename(char *outFile, char *outRoot,
+static void format_output_filename(char *outFile, const char *outRoot,
int pg_num_len, int pg)
{
if (!outRoot) outRoot = "cairoout";
@@ -164,7 +164,7 @@ static cairo_surface_t *start_page(char *outFile, int w,
int h,
if (png) {
surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, w*x_res/72.0,
h*y_res/72.0);
} else if (jpg) {
- surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, w*x_res/72.0,
h*y_res/72.0);
+ surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, w*x_res/72.0,
h*y_res/72.0);
} else if (ps) {
surface = cairo_ps_surface_create(outFile, w, h);
} else if (pdf) {
@@ -205,6 +205,9 @@ static void end_page_jpeg(cairo_surface_t *surface, char
*outFile)
p += stride;
}
writer->close();
+
+ delete writer;
+ delete[] row;
}
static void end_page(cairo_surface_t *surface, char *outFile)
@@ -227,7 +230,6 @@ static int render_page(CairoOutputDev *output_dev, PDFDoc
*doc,
{
cairo_t *cr;
cairo_status_t status;
- TextPage *text = NULL;
if (w == 0) w = (int)ceil(pg_w);
if (h == 0) h = (int)ceil(pg_h);
@@ -243,17 +245,6 @@ static int render_page(CairoOutputDev *output_dev, PDFDoc
*doc,
if (!printing)
cairo_scale (cr, x_res/72.0, y_res/72.0);
- // JPEGs are non-transparent, so we need a white background
- if (jpg) {
- cairo_rectangle(cr, 0, 0, w, h);
- cairo_set_source_rgb(cr, 1, 1, 1);
- cairo_fill(cr);
- }
-
- text = new TextPage(gFalse);
- if (!printing)
- output_dev->setTextPage (text);
-
/* NOTE: instead of passing -1 we should/could use cairo_clip_extents()
* to get a bounding box */
cairo_save (cr);
@@ -266,8 +257,8 @@ static int render_page(CairoOutputDev *output_dev, PDFDoc
*doc,
cairo_restore(cr);
output_dev->setCairo(NULL);
- output_dev->setTextPage(NULL);
+ // Add a white background
if (!printing) {
cairo_save(cr);
cairo_set_operator(cr, CAIRO_OPERATOR_DEST_OVER);
@@ -281,9 +272,6 @@ static int render_page(CairoOutputDev *output_dev, PDFDoc
*doc,
fprintf(stderr, "cairo error: %s\n", cairo_status_to_string (status));
cairo_destroy (cr);
- if (text != NULL)
- text->decRefCnt();
-
return 0;
}
@@ -296,7 +284,7 @@ int main(int argc, char *argv[]) {
GBool ok;
int exitCode;
int pg, pg_num_len;
- double pg_w, pg_h, tmp;
+ double pg_w, pg_h;
char *p;
CairoOutputDev *output_dev;
cairo_surface_t *surface;
@@ -420,12 +408,12 @@ int main(int argc, char *argv[]) {
render_page(output_dev, doc, surface, printing, pg,
x, y, w, h, pg_w, pg_h, x_resolution, y_resolution);
end_page(surface, outFile);
+ cairo_surface_finish(surface);
+ status = cairo_surface_status(surface);
+ if (status)
+ fprintf(stderr, "cairo error: %s\n", cairo_status_to_string(status));
+ cairo_surface_destroy(surface);
}
- cairo_surface_finish(surface);
- status = cairo_surface_status(surface);
- if (status)
- fprintf(stderr, "cairo error: %s\n", cairo_status_to_string(status));
- cairo_surface_destroy(surface);
delete output_dev;
exitCode = 0;
@@ -434,6 +422,7 @@ int main(int argc, char *argv[]) {
err1:
delete doc;
delete globalParams;
+ if (outRoot) free(outRoot);
err0:
// check for memory leaks
--
1.7.0.4
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
