Created attachment 94075 [details] [review]
proposed patch

Make _gtk_printer_get_details public.

Information about hard margins are usefull when someone wants to precisely print on a paper.
It's a functionnality that i greatly approve, as i'm actually getting this information on win32 backend with a patch like yours.
However, exposing the information on GtkPrinter , is not portable accross other operating systems, as GtkPrinter is unix backend only.
I suggest to expose this information on GtkPrintContext. i can come with a patch for win32 backend if you can propose an interface for GtkPrintContext and if someone of the core team validate the solution.

The situation in which I need this is one where I have no print context, just the printer object itself. So your proposal wouldn't help me with that. It's of course useful to have this info also in a cross-platform way, but that should not block this patch.

ok, could you anyway attempt to create a patch for a cross-platform implementation of the unix backend? it could be usefull for the core team to have a reference implementation for this, no?

next attachement is a patch for a reference win32 implementation.

Created attachment 116383 [details] [review]
patch for win32 hard margins

the patch for a possible win32 implementation of hard printer context margins

Created attachment 116387 [details] [review]
correct patch

fix a typo in the previous patch

Seems like we may need both of these patches, and then a third one to make hard margins available on the print context on unix.

All of this needs to wait unit 2.15 at this point, though.

Can you come up with a unified patch, now that 2.15 has opened ?

Are you asking it to me or to christian?
Don't you prefer an incremental patch? as it should be more platform independent?

- one to bring _gtk_printer_get_hard_margins public (the first one)
- one to bring GtkPrintContext aware of hardmargins (the first part of my initial win32 patch)
- one that call _gtk_print_context_set_hard_margins on win32 backend (the second part of my initial win32 patch)
- one that call _gtk_print_context_set_hard_margins on unix backend

Do you have a tip where the call to _gtk_print_context_set_hard_margins should be done on the unix backend?

Created attachment 119334 [details] [review]
patch to bring GtkPrintContext hard margin aware

platform independent patch to bring GtkPrintContext hard margin aware

Created attachment 119336 [details] [review]
patch for win32 backend

patch that depends on the 119334 one to bring hardmargins on win32 backend

as you can see, i'm going for an incremental patch version to make reviewing this easier.
it's missing the unix backend patch , which should be as short as the win32 solution.

Hey folks :)
Matthias, could you review patch 119334 and patch 119336 which still apply cleanly on master? Also, I'd like to know whether it worth the hassle to make patch 94075 apply cleanly, i.e. is it still "accepted-commit_after_freeze"?

Created attachment 134644 [details] [review]
patch

Rediff'd against master

(In reply to comment #11)
> Created an attachment (id=119336) [edit]
> patch for win32 backend
> 
> patch that depends on the 119334 one to bring hardmargins on win32 backend
> 

The margins don't look to be correct in that patch. It should be:

  hard_top    = (double)GetDeviceCaps(hDC,PHYSICALOFFSETY);
  hard_bottom = (double)GetDeviceCaps(hDC,PHYSICALHEIGHT)
                  -(double)GetDeviceCaps(hDC,VERTRES)-hard_top;
  hard_left   = (double)GetDeviceCaps(hDC,PHYSICALOFFSETX);
  hard_right  = (double)GetDeviceCaps(hDC,PHYSICALWIDTH)
                  -(double)GetDeviceCaps(hDC,HORZTRES)-hard_left;

In the cups backend the hard margins (in points) can be obtained with:

  ppd_attr_res = ppdFindAttr (ppd_file, "HWMargins", NULL);
  sscanf (ppd_attr_res->value, "%f %f %f %f", &left, &bottom, &right, &top);

thanks for the review, Adrian

Matthias: the review in comment 14 was for attachment 119336 [details] [review], not attachment 134644 [details] [review]. Resetting patch status on my patch.

Hi, i finally have time to make a complete patch for win32 and linux.
It seems to me that the patch from christian adds hard margings aware into printer class itself witch is only available to unix printing systems(correct me christian if i'm wrong).
My solution adds it into printer context class because it seems to me that it's the only way to be architecture independant, thus portable.
please take a look at my patch #119334 witch is the portable one and tell me if it's ok with you.
I'll apply the correction of the review from Adrian to my patch #119336 (that was a silly bug, sorry)
Then we have 2 choices, i can let christian make a new patch to bring margins to printer context with an incremental patch for unix or let me do it for him.
But i have a question to christian though, do you want to keep printer_get_hard_margins public? or an interface to a printer context is sufficiant? i can keep with the two solutions if Matthias is ok.

Created attachment 148212 [details] [review]
Set unix printer hard margins

Here is the complete set of patches for unix and win32.

The first patch is patch 119334 from comment 10 (make print context hard margin aware). It still applies cleanly to master.

The second patch is patch 134644 from comment 14 (Allow getting the printer's hard margins). I have rebased it to master and attach it here.

Created attachment 148213 [details] [review]
Set unix printer hard margins

Patch to set hard margins on print context for unix printers.

Created attachment 148214 [details] [review]
default to hard margins not available

Default to hard margins not available unless the backend provides the margins.

Created attachment 148216 [details] [review]
set hard margins to 0 when printing n-up

When printing n-up, set the hard margins to 0 as the the entire area of each page on the sheet is printable.

Created attachment 148217 [details] [review]
win32 hard margins

Set hard margins on printer context for win32.

Review of attachment 148212 [details] [review]:

Apart from the Since: line, looks fine to me.

::: gtk/gtkprinter.c
@@ +986,3 @@
+ * Return value: %TRUE iff the hard margins were retrieved
+ *
+ * Since: 2.18

Time marches on...this needs to be Since: 2.20 by now.

Comment on attachment 148216 [details] [review]
set hard margins to 0 when printing n-up

I'm not sure I entirely understand this, but I have given up long ago trying to untangle the interactions of n-up, paper-size, margins, and their implementations at various levels...

I'm just going to trust you that this is the right thing to do - but it would be great to add a comment explaining why it is.

Comment on attachment 148217 [details] [review]
win32 hard margins

I'm not qualified to review this, so I'm again going to trust you on this one.

It's ok with me... it's working the same as my local patches on win32.
I can't tell for linux backend, but it seems ok.

(In reply to comment #25)
> (From update of attachment 148216 [details] [review])
> I'm not sure I entirely understand this, but I have given up long ago trying to
> untangle the interactions of n-up, paper-size, margins, and their
> implementations at various levels...
> 
> I'm just going to trust you that this is the right thing to do - but it would
> be great to add a comment explaining why it is.

The following comment can included in the commit message:

Pages do not have any unprintable area when printing n-up as each page on the sheet has been scaled down and translated to a position within the printable area of the sheet.

All patches pushed to master.

Reopening; backed out.

The build fails with ./.libs/libgtk-x11-2.0.so: undefined reference to `_gtk_print_context_set_hard_margins'.

Adrian: is there a patch missing in the series?

(In reply to comment #30)
> Reopening; backed out.
> 
> The build fails with ./.libs/libgtk-x11-2.0.so: undefined reference to
> `_gtk_print_context_set_hard_margins'.
> 
> Adrian: is there a patch missing in the series?

Comment 19 explains that the first patch is the existing patch in comment 10 that still applies cleanly to master.

I see. That patch however isn't accepted-commit-now yet.

If it isn't, thats an oversight on my part. I meant to ok the entire series.

ok, I've just pushed the complete set of patches, it builds now.