<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - Add support for printing to a Windows printer from pdftocairo" href="https://bugs.freedesktop.org/show_bug.cgi?id=79936#c32">Comment # 32</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - Add support for printing to a Windows printer from pdftocairo" href="https://bugs.freedesktop.org/show_bug.cgi?id=79936">bug 79936</a> from <a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> Carlos Garcia Campos</a> <pre>Comment on <a href="attachment.cgi?id=103703" name="attach_103703" title="New patch version with many improvements">attachment 103703</a> <a href="attachment.cgi?id=103703&action=edit" title="New patch version with many improvements">[details]</a> <a href='page.cgi?id=splinter.html&bug=79936&attachment=103703'>[review]</a> New patch version with many improvements Review of <a href="attachment.cgi?id=103703" name="attach_103703" title="New patch version with many improvements">attachment 103703</a> <a href="attachment.cgi?id=103703&action=edit" title="New patch version with many improvements">[details]</a> <a href='page.cgi?id=splinter.html&bug=79936&attachment=103703'>[review]</a>: ----------------------------------------------------------------- ::: utils/pdftocairo-win32.cc @@ +1,3 @@ > +#include <cairo-win32.h> > + > +static void win32GetFitToPageTransform(cairo_matrix_t *m) This is a private method of this file, I don't think we need to use the win32 prefix for this. @@ +135,5 @@ > + } > + } > +} > + > +static void win32BeginDocument(GooString *outputFileName, double w, double h) I would could this something like createPrintingSurface so that we don't need to use the win32 prefix, this is called by beingDocument after all, not instead of beginDocument. And I would make it return the surface instead. @@ +137,5 @@ > +} > + > +static void win32BeginDocument(GooString *outputFileName, double w, double h) > +{ > + if (!print) And this could be checked by the caller @@ +140,5 @@ > +{ > + if (!print) > + return; > + > + if (printer.getCString()[0] == 0) I think if (printer.getLength() == 0) is easier to read. @@ +141,5 @@ > + if (!print) > + return; > + > + if (printer.getCString()[0] == 0) > + { Move this to the previous line, for consistency with the rest of the code. @@ +149,5 @@ > + GetDefaultPrinterA(devname, &szName); > + printer.Set(devname); > + gfree(devname); > + } > + char *cPrinter = printer.getCString(); This should probably be const char * @@ +154,5 @@ > + > + //Query the size of the DEVMODE struct > + LONG szProp = DocumentPropertiesA(NULL, NULL, cPrinter, NULL, NULL, 0); > + if (szProp < 0) > + { Move this to the previous line @@ +164,5 @@ > + devmode->dmSize = sizeof(DEVMODEA); > + devmode->dmSpecVersion = DM_SPECVERSION; > + //Load the current default configuration for the printer into devmode > + if (DocumentPropertiesA(NULL, NULL, cPrinter, devmode, NULL, DM_OUT_BUFFER) < 0) > + { Ditto. @@ +170,5 @@ > + exit(99); > + } > + fillCommonPrinterOptions(devmode, w, h); > + fillPrinterOptions(devmode); > + HDC hdc = CreateDCA(NULL, cPrinter, NULL, devmode); I have no idea what DCA stands for, but maybe all previous code could be moved to a helper function that initialize the printer and returns the HDC @@ +173,5 @@ > + fillPrinterOptions(devmode); > + HDC hdc = CreateDCA(NULL, cPrinter, NULL, devmode); > + gfree(devmode); > + if (!hdc) > + { Ditto. @@ +182,5 @@ > + DOCINFOA docinfo; > + memset(&docinfo, 0, sizeof(docinfo)); > + docinfo.cbSize = sizeof(docinfo); > + if (outputFileName->cmp("fd://0") == 0) > + docinfo.lpszDocName = outputFileName->getCString(); Fixed by Adrian in a follow up patch, it would be better to fix it here before instead of landing a patch that break the build. @@ +194,5 @@ > + > + surface = cairo_win32_printing_surface_create(hdc); > +} > + > +static void win32BeginPage(double w, double h) Ah, I see all are static, but included in the other file. I would rather add a header for the public methods, and include the header from pdtocairo instead. ::: utils/pdftocairo.cc @@ +77,4 @@ > static GBool ps = gFalse; > static GBool eps = gFalse; > static GBool pdf = gFalse; > +static GBool print = gFalse; I find this a bit confusing, because we also have printing variable. I would use something like printingToWinPrinter or something like that @@ +485,5 @@ > // shrink to fit > cairo_matrix_scale (m, scale, scale); > } > +#ifdef CAIRO_HAS_WIN32_SURFACE > + win32GetFitToPageTransform(m); I see now this was actually "public", but again this is not the getFitToPageTransform for win32, no? I would use a better name that express what the function actually does</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>