Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Page Size Issues #127

Closed
michaelrsweet opened this issue May 25, 2003 · 10 comments
Closed

Custom Page Size Issues #127

michaelrsweet opened this issue May 25, 2003 · 10 comments
Milestone

Comments

@michaelrsweet
Copy link
Collaborator

Version: 1.1.19rc5
CUPS.org User: david.gelphman

Michael,
There are a couple of issues that have come up recently regarding custom page size handling.

The first one is that when generating custom page size, cups is treating this as *PageSize Custom and using the order dependency for *PageSize rather than using the NonUIOrderDependency for CustomPageSize. This is causing problem for at least one OEM. I started to look at the code to craft a solution for this but I thought I'd ask if you'd heard about this problem before. As a side note, I've always generated the PS code as
%%BeginFeature *CustomPageSize True
code goes here
%%EndFeature

rather than using the *PageSize Custom as CUPS currently does. I don't think this is a big deal really but I thought I'd mention it. Note that using the order dependency is a real issue that I'll need to resolve at some point.

(As a side note, it is a bit odd that the order dependency for custom paper size is different than that for normal page size for any printer. I've pushed back on our corresponding bug regarding this because I think the straightforward workaround for the manufacturer is to update their PPD so these order dependencies are the same.)

The other problem is that the way the CustomPageSize code is being generated appears to be out of spec with the PPD 4.3 specification (section 5.16 p106 of the PPD spec v4.3). That is not one of the sterling portions of the PPD spec (it is a mess really) but the bottom line is that when generating orientation value of zero, the width must be larger than the height. See figure 3 on page 109 and Table 2 on p118. In the case of orientation 0, if height is > width, this will produce a landscape page. In principle we should be using width > height if we are using orientation zero.

I'm reluctant to simply make this change because it will almost certainly break things for some large number of printers. Defensive PPD code could address this by examining the parameters passed in and adjusting them appropriately.

I'm interested in hearing about your experience and thoughts on this matter. The complaints are coming from a variety of sources and unfortunately because of bugs the implementation in LaserWriter 8 had the same problem. This has lead to a mixture of behavior from incorrect results on properly formed PPDs as well as manufacturers' adjusting their PPDs to non-conformance with the PPD spec but correct results with Macintosh print drivers.

Thanks for your feedback,
David

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

Re: the order dependency and feature comment issues, those should definitely be fixed.

Re: width > height, I find no text on the specified pages that supports that assertation; certainly the spec refers to portrait and landscape pages, however the orientation value is specifically used to rotate the coordinate system (supported by both the text and figures) of the page and therefore is independent of the landscape/portrait appearance of the print. You'll notice that the bottom "roll" of Figure 3 on page 109 shows Height > Width for all four orientation values...

Patch to emit.c to follow shortly...

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

I've attached an initial patch for the page comment and also changes to use the ParamCustomPageSize attributes (noticed that they were not being used...)

I'm not sure how we'll handle the order dependency issue - the sorting code doesn't have access to the PPD file pointer, so it can't lookup the attribute from it and sort based on that value, only based upon the PageSize option...

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: david.gelphman

Regarding the issue of the width, height and orientation parameters: first look at Figure 3 on page 109. What you see in the bottom of the two diagrams, i.e. the one where Height > Width is that the page image that appears on the roll is a landscape page for orientation zero and a portrait page for orientation 1. Since we are using Height > Width and orientation 0, for PPD code and a device that follows the spec, I would expect a landscape page to result. Since we expect a portrait orientation to be in place after we execute the page size code this produces incorrect results.

Now to page 118 and Table 2. If you look at the last bullet item immediately before Table 2 it says: "Unless the print manager lets the user enter Orientation directly, the print manager must deduce the value of Orientation from a combination of user requests, as shown in Table 2."

If you are to consult Table 2 you will see that for the case where Height > Width (as we are currently generating) Orientation 0 will produce a landscape document.

I am concerned that from my reading of this portion of the PPD spec and the feedback we are getting from some OEMs including Adobe I believe that cups is currently producing incorrect output
b) I'm also concerned that changing this will cause new bugs to appear because of people writing their PPDs have not followed the spec either, probably because of driver bugs that produced incorrect output if they did.

I'm not 100% clear what the solution is. On the one hand, if we start to follow the spec we may break various printer/PPDs. On the other hand other printer/PPDs don't work correctly with the current code. The one advantage is that by changing to follow the spec we can fix those that already do and force correct conformance by others in turn.

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

OK, let's start from the beginning.

From the standpoint of CUPS, all custom pages are portrait orientation, even if width > height (length). If we send a print file with a custom size, the coordinate system originates at the bottom lefthand corner with X increasing to the right and Y increasing to the top.

All of the CUPS raster drivers I know of and many of the PPD files we have for PS printers ignore the orientation value that is passed to the CustomPageSize code, so from the CUPS implementation standpoint the only thing we care about is that the default value we send for Orientation (currently 0) is the correct one according to the spec. We do not want the device doing any rotation of the page image for us, we just want a common, neutral coordinate space to image onto.

That said, after looking at figure 3 again I'm guessing that the correct orientation value is 1, not 0, but it could also be 3 (the figure is not at all clear which way the paper goes into the printer...)

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: david.gelphman

You are correct: if we supply a WIDTH < HEIGHT then we should be using orientation 1. If we supply a WIDTH > HEIGHT we should be suppling orientation 0. The other orientations are for fancier paper handling and presumably would come from a UI (which we don't have). Note that WIDTH is not necessarily the custom paper width and HEIGHT is not necessarily the custom paper length.

A couple of notes:

  1. If orientation 1 is not allowed (the range of orientations is supplied by *ParamCustomPageSize for the Orientation value) then we should be using orientation 0 and swapping supplying width > height.

  2. If the sheet width is indeed greater than the sheet height then we should specify orientation 0. If the sheet width is less than the sheet height we should specify orientation 1 (assuming orientation 1 is supported).

Now this is the way the spec reads and which Adobe is pressing for. Please do note that I'm not claiming first hand experience of correct results with this approach. I had the a bug in the LW8 code I wrote which, while it was trying to do this, turns out it was not :-(

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

Hmm, I'm not sure that sending the width/height swapped with orientation = 0 is the "right" solution, as we'd then need to rotate the page ourselves. Keep in mind that from our standpoint a custom page size is always "portrait" even when width > length, since "portrait" and "landscape" in CUPS/IPP refer to page image rotation and not the actual media orientation (although that might be how a printer implements it in some situations...)

IIRC (and I don't have the PPD spec in front of me right now) the spec specifically states that the printer may or may not actually rotate the page image with different Orientation values.

That said, I can't say that I've ever looked for PPD files that limit the Orientation to 0, so I don't know if this will ever be a problem, but I'm just bringing up the issue because I'm still not clear on how providing the wrong width/length will do anything but result in a print that is cut off on one side...

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

I've commited a change to CVS to set Orientation to 1 or 0 as needed, and to range check against the ParamCustomPageSize limits, however I did not swap the width/length if the orientation is limited to a value of 0. Some basic searches through PPD files reveiled that the CUPS sample drivers limit orientation to 0, and they will definitely break if we do any swapping. Also, some basic tests on a DesignJet 3500 (which does support the orientations) shows that swapping the width and length results in a cut-off print, as expected...

@michaelrsweet
Copy link
Collaborator Author

CUPS.org User: mike

Note, for CUPS 1.1.21 we will only be using an orientation of 1 unless the PPD file specifies that 1 is not a supported value. The previous code we used had bad side-effects on large-format PS printers, and using an orientation value of 1 seems to work in all cases.

@michaelrsweet
Copy link
Collaborator Author

"str127.patch":

Index: emit.c

RCS file: /development/cvs/cups/cups/emit.c,v
retrieving revision 1.31
diff -u -r1.31 emit.c
--- emit.c 2003/02/28 21:06:08 1.31
+++ emit.c 2003/05/25 14:14:27
@@ -196,27 +196,53 @@
* Send DSC comments with option...
*/

  •  if (fprintf(fp, "%%%%BeginFeature: *%s %s\n",
    
  •              ((ppd_option_t *)choices[i]->option)->keyword,
    
  •     choices[i]->choice) < 0)
    
  •  {
    
  •    free(choices);
    
  •    return (-1);
    

- }

   if ((strcasecmp(((ppd_option_t *)choices[i]->option)->keyword, "PageSize") == 0 ||
        strcasecmp(((ppd_option_t *)choices[i]->option)->keyword, "PageRegion") == 0) &&
       strcasecmp(choices[i]->choice, "Custom") == 0)
   {
    /*
  •    \* Variable size; write out standard size options (this should
    
  • * eventually be changed to use the parameter positions defined

  • * in the PPD file...)

  •    \* Variable size; write out standard size options, using the
    
    • parameter positions defined in the PPD file...
      */
  •    ppd_attr_t *attr;      /* PPD attribute */
    
  • int pos, /* Position of custom value */

  •       values[5];  /\* Values for custom command */
    
  •    fputs("%%BeginFeature: *CustomPageSize True\n", fp);
    
    • size = ppdPageSize(ppd, "Custom");
  •    fprintf(fp, "%.0f %.0f 0 0 0\n", size->width, size->length);
    
  •    memset(values, 0, sizeof(values));
    
  • if ((attr = ppdFindAttr(ppd, "ParamCustomPageSize", "Width")) != NULL)

  • {

  • pos = atoi(attr->value) - 1;
    
  •      if (pos < 0 || pos > 4)
    
  •   pos = 0;
    
  • }

  • else

  • pos = 0;
    
  • values[pos] = (int)size->width;

  • if ((attr = ppdFindAttr(ppd, "ParamCustomPageSize", "Height")) != NULL)
  • {
  • pos = atoi(attr->value) - 1;
    
  •      if (pos < 0 || pos > 4)
    
  •   pos = 1;
    
  • }
  • else
  • pos = 1;
    
  • values[pos] = (int)size->length;
  •    fprintf(fp, "%d %d %d %d %d\n", values[0], values[1],
    
  •       values[2], values[3], values[4]);
    

    if (choices[i]->code == NULL)
    {
    /*
    @@ -228,6 +254,13 @@
    fputs(ppd_custom_code, fp);
    }
    }

  •  else if (fprintf(fp, "%%%%BeginFeature: *%s %s\n",
    
  •                   ((ppd_option_t *)choices[i]->option)->keyword,
    
  •          choices[i]->choice) < 0)
    
  •  {
    
  •    free(choices);
    
  •    return (-1);
    
  •  }
    

    if (choices[i]->code != NULL && choices[i]->code[0] != '\0')
    {
    @@ -274,7 +307,8 @@
    ppd_section_t section) /* I - Section to write /
    {
    int i, /
    Looping var */

  •   count;          /\* Number of choices */
    
  •   count,          /\* Number of choices */
    
  •   custom_size;        /\* Non-zero if this option is a custom size _/
    

    ppd_choice_t *_choices; /* Choices /
    ppd_size_t *size; /
    Custom page size /
    char buf[1024]; /
    Output buffer for feature /
    @@ -310,30 +344,72 @@
    /

    • Send DSC comments with option...
      */
      +
  •  if ((strcasecmp(((ppd_option_t *)choices[i]->option)->keyword, "PageSize") == 0 ||
    
  •       strcasecmp(((ppd_option_t *)choices[i]->option)->keyword, "PageRegion") == 0) &&
    
  •      strcasecmp(choices[i]->choice, "Custom") == 0)
    
  •  {
    
  •    custom_size = 1;
    
  •  snprintf(buf, sizeof(buf), "%%%%BeginFeature: *%s %s\n",
    
  •           ((ppd_option_t *)choices[i]->option)->keyword,
    
  •      choices[i]->choice);
    
  • strcpy(buf, "%%BeginFeature: *CustomPageSize True\n");

  •  }
    
  •  else
    
  •  {
    
  •    custom_size = 0;
    
  • snprintf(buf, sizeof(buf), "%%%%BeginFeature: *%s %s\n",

  •        ((ppd_option_t *)choices[i]->option)->keyword,
    
  •    choices[i]->choice);
    
  •  }
    
    • if (write(fd, buf, strlen(buf)) < 1)
      {
      free(choices);
      return (-1);
      }
  •  if ((strcasecmp(((ppd_option_t *)choices[i]->option)->keyword, "PageSize") == 0 ||
    
  •       strcasecmp(((ppd_option_t *)choices[i]->option)->keyword, "PageRegion") == 0) &&
    
  •      strcasecmp(choices[i]->choice, "Custom") == 0)
    
  •  if (custom_size)
    

    {
    /*

  •    \* Variable size; write out standard size options (this should
    
  • * eventually be changed to use the parameter positions defined

  • * in the PPD file...)

  •    \* Variable size; write out standard size options, using the
    
    • parameter positions defined in the PPD file...
      */
  •    ppd_attr_t *attr;      /* PPD attribute */
    
  • int pos, /* Position of custom value */

  •       values[5];  /\* Values for custom command */
    
    • size = ppdPageSize(ppd, "Custom");
  •    snprintf(buf, sizeof(buf), "%.0f %.0f 0 0 0\n", size->width,
    
  •        size->length);
    
  •    memset(values, 0, sizeof(values));
    
  • if ((attr = ppdFindAttr(ppd, "ParamCustomPageSize", "Width")) != NULL)

  • {

  • pos = atoi(attr->value) - 1;
    
  •      if (pos < 0 || pos > 4)
    
  •   pos = 0;
    
  • }

  • else

  • pos = 0;
    
  • values[pos] = (int)size->width;

  • if ((attr = ppdFindAttr(ppd, "ParamCustomPageSize", "Height")) != NULL)
  • {
  • pos = atoi(attr->value) - 1;
    
  •      if (pos < 0 || pos > 4)
    
  •   pos = 1;
    
  • }
  • else
  • pos = 1;
    
  • values[pos] = (int)size->length;
  •    snprintf(buf, sizeof(buf), "%d %d %d %d %d\n", values[0], values[1],
    
  •        values[2], values[3], values[4]);
    

    if (write(fd, buf, strlen(buf)) < 1)
    {

@michaelrsweet
Copy link
Collaborator Author

"str127-part2.patch":

Index: emit.c

RCS file: /development/cvs/cups/cups/emit.c,v
retrieving revision 1.32
diff -u -r1.32 emit.c
--- emit.c 2003/05/25 14:39:17 1.32
+++ emit.c 2003/06/02 19:42:31
@@ -207,7 +207,8 @@

     ppd_attr_t *attr;      /* PPD attribute */
int     pos,        /* Position of custom value */
  •       values[5];  /\* Values for custom command */
    
  •       values[5],  /\* Values for custom command */
    
  •       orientation;    /* Orientation to use */
    
    
     fputs("%%BeginFeature: *CustomPageSize True\n", fp);
    

    @@ -240,6 +241,36 @@

    values[pos] = (int)size->length;

  •    if (size->width < size->length)
    
  • orientation = 1;
    
  • else

  • orientation = 0;
    
  • if ((attr = ppdFindAttr(ppd, "ParamCustomPageSize",

  •                       "Orientation")) != NULL)
    
  • {

  • int min_orient, max_orient;   /\* Minimum and maximum orientations */
    
  •      if (sscanf(attr->value, "%d%*s%d%d", &pos, &min_orient,
    
  •            &max_orient) != 3)
    
  •   pos = 4;
    
  • else
    
  • {
    
  •        if (pos < 0 || pos > 4)
    
  •     pos = 4;
    
  •        if (orientation > max_orient)
    
  •     orientation = max_orient;
    
  •   else if (orientation < min_orient)
    
  •     orientation = min_orient;
    
  • }
    
  • }

  • else

  • pos = 4;
    
  • values[pos] = orientation;

  • fprintf(fp, "%d %d %d %d %d\n", values[0], values[1],
    values[2], values[3], values[4]);

@@ -377,7 +408,8 @@

     ppd_attr_t *attr;      /* PPD attribute */
int     pos,        /* Position of custom value */
  •       values[5];  /\* Values for custom command */
    
  •       values[5],  /\* Values for custom command */
    
  •       orientation;    /* Orientation to use */
    
    
     size = ppdPageSize(ppd, "Custom");
    

    @@ -407,6 +439,36 @@
    pos = 1;

    values[pos] = (int)size->length;
    +

  •    if (size->width < size->length)
    
  • orientation = 1;
    
  • else

  • orientation = 0;
    
  • if ((attr = ppdFindAttr(ppd, "ParamCustomPageSize",

  •                       "Orientation")) != NULL)
    
  • {

  • int min_orient, max_orient;   /\* Minimum and maximum orientations */
    
  •      if (sscanf(attr->value, "%d%*s%d%d", &pos, &min_orient,
    
  •            &max_orient) != 3)
    
  •   pos = 4;
    
  • else
    
  • {
    
  •        if (pos < 0 || pos > 4)
    
  •     pos = 4;
    
  •        if (orientation > max_orient)
    
  •     orientation = max_orient;
    
  •   else if (orientation < min_orient)
    
  •     orientation = min_orient;
    
  • }
    
  • }

  • else

  • pos = 4;
    
  • values[pos] = orientation;

 snprintf(buf, sizeof(buf), "%d %d %d %d %d\n", values[0], values[1],
     values[2], values[3], values[4]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant