[Poppler-bugs] [Bug 79936] Add support for printing to a Windows printer from pdftocairo

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=79936

--- Comment #4 from rodrigo <rodrigorivascosta at gmail.com> ---
(In reply to comment #2)
> You only need to add what you need. Other options can be added later.
> 
> Where there is overlap with existing options, the existing options
> should be used to set the printing options. Users expect these options
> to work across all output formats.
>
> If an existing option does not support all the functionality we can
> make the -printopt version override the old one (maybe with a warning
> about duplicated options).
> 

Ok. Actually, I only need the 'source' option. The 'duplex' was just for show,
precisely because it overlapped. The override with warning is a nice idea.

> I'm not sure how easy it is to make the build system compile
> pdftocairowin32.cc only when CAIRO_HAS_WIN32_SURFACE is
> defined. Probably the easiest thing is to just wrap pdftocairowin32.cc
> in a #ifdef CAIRO_HAS_WIN32_SURFACE. This is what we do in the cairo
> source.

I think you would have to write a autoconf test. It is not difficult but
probably not worth it. What we could do is to add "AC_SUBST(WIN32)" to
configure.ac so that the -lwinspool and the pdftocairowin32.cc are only
considered in Windows, even if the code is guarded by CAIRO_HAS_WIN32_SURFACE.

> I don't like the use of an integer in the source option. Why would we
> need to support a number that is not one the #defines for DEVMODE?

The docs say:
 dmDefaultSource: [...] This member can be one of the following values, or it
can be a device-specific value greater than or equal to DMBIN_USER.

I think that it means that a printer driver is free to define its own values >=
DMBIN_USER (256). I don't know how frequent they do that, but an advanced user
may want to use source 258, and that does not have a name. Hey, maybe even
printer manufacturers will use that, some day...

Also, there are a few other fields with *_USER values.

> It would be nicer if parseSource used a table instead of multiple if
> statements for matching the source. eg see standardMedia in
> PSOutputDev.cc.

Yeah, no problem.

> When setting printer options in devmode you are setting the
> corresponding flag in dmFields. My interpretation of the DEVMODE
> documentation is that the devmode structure for a printer only supports
> setting the options indicated in the dmFields member. ie you should
> not be changing dmFields but instead check to see if an option is
> supported in dmFields before setting the option.

I'm not sure. In windows it is frequent to have a bitfield in a structure for
the fields you set, so that the unset ones get default values. See for example
the STARTUPINFO struct. The issue with DEVMODE is that I'm initializing it with
the default values from the printer driver, so if the flag is not set in the
first place it is probably because the driver does not support it. Except for
mutual incompatible flags, such as DM_PAPERSIZE and
DM_PAPERLENGTH/DM_PAPERWIDTH.

Anyway, we only add options explicitly set in the command line, so I don't see
any reason to check. If the user wants it, just do it. The proper function for
checking those would be 'DeviceCapabilities()'.

> The paper size in devmode [... a lot of text and code ...]

Ok, it looks fine. I'll check that and comment later.

> EndDoc and DeleteDC should be called after cairo_surface_finish and
> cairo_surface_destroy.

That was my first idea, but the first time I did it, a few months ago, it
crashed. Maybe now it is corrected, I will try again.

> The pdftocairo.1 man page needs to be updated [...]

Oh, the docs. Sure.

> In DOCINFO put the actual pdf filename if available. If reading from
> standard input I suggest using the document name "pdftocairo <stdin>".

Nice.

> Win32 calls that have both ansi and unicode versions should have an
> "A" or "W" to specify which one we are using. eg DeviceCapabilitiesA.

Well, the default one is the "A" unless UNICODE is defined. If it were a
library, then of course, but it is a program: it is up to us to define UNICODE
or not... Anyway, if you prefer, I'll add the A or W everywhere.
> 
> The "Error: one of the output format ..." error message should only
> include "-print" if this option has been compiled in.

Yes, but the ifdefs would be ugly. Maybe it can be done as:

    fprintf(stderr, "Error: one of the output format options (-png, -jpeg, -ps,
-eps, -pdf,"
#ifdef CAIRO_HAS_WIN32_SURFACE
    " -print,"
#endif
    " -svg) must be used.\n");

If you want, I'll prepare a more complete patch in a few days.

Thank you!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20140729/976c540d/attachment.html>