Summary: | low resolution fill adjust is wrong | ||
---|---|---|---|
Product: | Ghostscript | Reporter: | Henry Stiles <henry.stiles> |
Component: | Graphics Library | Assignee: | Robin Watts <robin> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | marcos.woehrmann |
Priority: | P4 | ||
Version: | master | ||
Hardware: | Macintosh | ||
OS: | MacOS X | ||
Customer: | Word Size: | --- | |
Attachments: |
rectfill.ps
rectfill.tiff foo.tiff rectfill_frac_pixel.ps clipbug.ps clipbug.ps patch |
Description
Henry Stiles
2009-07-10 13:35:40 UTC
Created attachment 5207 [details]
rectfill.ps
postscript program to print out sample rectangular fills, the number on the
right is the height of the rectangle. The actual pixel height of the
rectangles should be 1 if the height == 0 and ceil(height) otherwise, like the
adobe output. Our output is "less than" any part of pixel due to only
adjusting fills by .25.
Created attachment 5208 [details]
rectfill.tiff
96 dpi acrobat rendering.
Created attachment 5209 [details]
foo.tiff
ghostscript's 96 dpi output.
The attached tiff files may be opened in Adobe Acrobat (8 and 9), set the resolution to 96 dpi and turn off smoothing options. A magnifier like "pixie" (mac) could be used to look at the pixel width of the lines. Imagemagick's display program has a built in magnifier and shows device coordinates. Created attachment 5211 [details]
rectfill_frac_pixel.ps
Attached is a more comprehensive filling test, this is a 31 page test the first
page tests a range of rectangle fill heights and subsequent pages translate the
test by a small amount in an effort to see how fractional pixel moves affect
the filling. The current gs rectangle fill algorithm does not match adobe
acrobat 8.
The following code change prints the comprehensive test (see the attachment in comment #5) reasonably and the regression fixing several problems in the ps cets. It also is a reasonable interpretation of the mathematical description of the filling algorithm from the postscript and pdf manual (6.5.3 scan conversion rules), contrasting markedly with the code it replaces. I am posting the patch here for feedback and counterexamples. At 72 dpi the patch does give a pixel for pixel match of the rectangles in the comprehensive test (the current code has many discrepancies), with a few exceptional cases where the adobe renderer appears incorrect. NOTE: the code should not be checked in yet since it has not been tested with languages other than postscript and pdf. svn diff Index: gsdps1.c =================================================================== --- gsdps1.c (revision 9859) +++ gsdps1.c (working copy) @@ -243,19 +243,21 @@ } } else { int x, y, w, h; - - draw_rect.p.x -= max(pgs->fill_adjust.x - fixed_epsilon, 0); - draw_rect.p.y -= max(pgs->fill_adjust.y - fixed_epsilon, 0); - draw_rect.q.x += pgs->fill_adjust.x; - draw_rect.q.y += pgs->fill_adjust.y; rect_intersect(draw_rect, clip_rect); - x = fixed2int_pixround(draw_rect.p.x); - y = fixed2int_pixround(draw_rect.p.y); - w = fixed2int_pixround(draw_rect.q.x) - x; - h = fixed2int_pixround(draw_rect.q.y) - y; - if (w > 0 && h > 0) - if (gx_fill_rectangle(x, y, w, h, pdc, pgs) < 0) - goto slow; + draw_rect.p.x = fixed_floor(draw_rect.p.x); + draw_rect.p.y = fixed_floor(draw_rect.p.y); + draw_rect.q.x = fixed_ceiling(draw_rect.q.x); + draw_rect.q.y = fixed_ceiling(draw_rect.q.y); + x = fixed2int(draw_rect.p.x); + y = fixed2int(draw_rect.p.y); + w = fixed2int(draw_rect.q.x - draw_rect.p.x); + h = fixed2int(draw_rect.q.y - draw_rect.p.y); + if (w == 0) + w = 1; + if (h == 0) + h = 1; + if (gx_fill_rectangle(x, y, w, h, pdc, pgs) < 0) + goto slow; } } return 0; The patch in #6 is improved to support center of pixel clients, pass through 0 area rectangles to high level devices, and emulate the peculiarities of Adobe Acrobate 8 0 fills. With the latter change the rectangles in rectfill_frac_pixel.ps match adobe pixel for pixel at 72 dpi. The patch has now been regression tested against all the languages without issues. svn diff Index: gsdps1.c =================================================================== --- gsdps1.c (revision 9862) +++ gsdps1.c (working copy) @@ -196,7 +196,9 @@ bool hl_color = (hl_color_available && dev_proc(pdev, fill_rectangle_hl_color)(pdev, &empty, pis, pdc, NULL) == 0); + bool center_of_pixel = (pgs->fill_adjust.x == 0 && pgs->fill_adjust.y == 0); + /* Processing a fill object operation */ gs_set_object_tag(pgs, GS_PATH_TAG); @@ -234,8 +236,14 @@ draw_rect.q.y = max(p.y, q.y); if (hl_color) { rect_intersect(draw_rect, clip_rect); - if (draw_rect.p.x < draw_rect.q.x && - draw_rect.p.y < draw_rect.q.y) { + /* We do pass on 0 extant rectangles to high level + devices. It isn't clear how a client and an output + device should interact if one uses a center of + pixel algorithm and the other uses any part of + pixel. For now we punt and just pass the high + level rectangle on without adjustment. */ + if (draw_rect.p.x <= draw_rect.q.x && + draw_rect.p.y <= draw_rect.q.y) { code = dev_proc(pdev, fill_rectangle_hl_color)(pdev, &draw_rect, pis, pdc, pcpath); if (code < 0) @@ -243,19 +251,37 @@ } } else { int x, y, w, h; - - draw_rect.p.x -= max(pgs->fill_adjust.x - fixed_epsilon, 0); - draw_rect.p.y -= max(pgs->fill_adjust.y - fixed_epsilon, 0); - draw_rect.q.x += pgs->fill_adjust.x; - draw_rect.q.y += pgs->fill_adjust.y; rect_intersect(draw_rect, clip_rect); - x = fixed2int_pixround(draw_rect.p.x); - y = fixed2int_pixround(draw_rect.p.y); - w = fixed2int_pixround(draw_rect.q.x) - x; - h = fixed2int_pixround(draw_rect.q.y) - y; - if (w > 0 && h > 0) - if (gx_fill_rectangle(x, y, w, h, pdc, pgs) < 0) - goto slow; + if (center_of_pixel) { + draw_rect.p.x = fixed_rounded(draw_rect.p.x); + draw_rect.p.y = fixed_rounded(draw_rect.p.y); + draw_rect.q.x = fixed_rounded(draw_rect.q.x); + draw_rect.q.y = fixed_rounded(draw_rect.q.y); + } else { /* any part of pixel rule - touched */ + draw_rect.p.x = fixed_floor(draw_rect.p.x); + draw_rect.p.y = fixed_floor(draw_rect.p.y); + draw_rect.q.x = fixed_ceiling(draw_rect.q.x); + draw_rect.q.y = fixed_ceiling(draw_rect.q.y); + } + x = fixed2int(draw_rect.p.x); + y = fixed2int(draw_rect.p.y); + w = fixed2int(draw_rect.q.x - draw_rect.p.x); + h = fixed2int(draw_rect.q.y - draw_rect.p.y); + /* clients that use the "any part of pixel" rule also + fill 0 areas. This is true of current graphics + library clients but not a general rule. */ + if (!center_of_pixel) { + if (w == 0) + w = 1; + /* yes Adobe Acrobat 8, seems to back up the y + coordinate when the width is 0, sigh. */ + if (h == 0) { + y--; + h = 1; + } + } + if (gx_fill_rectangle(x, y, w, h, pdc, pgs) < 0) + goto slow; } } return 0; Reassigning path and fill problems to Robin Watts. Henry - is there are reason why the patch in comment #7 can't just be tested/committed? I can't see any outstanding issues... Not that I can think of, but I'd rather you go forward with it, I've shifted my attention elsewhere and it would take me a while to wrap my head around this again. A separate but related problem I do remember is the pxl client has changed on the HP Color Laserjet 4600, rendering the same as postscript using an any-part-of-pixel (not centered) model, but we can deal with that by changing the XL client. This code should be okay. Created attachment 5788 [details]
clipbug.ps
Testing with local regressions/htmldiff shows problems in various of the ps3cet
test files, whereby a single dot appears where it shouldn't (09-01.ps for
example, page 2). I've done some investigation and boiled the problem down to
the attached file.
The problem is that the new code doesn't cope well with a zero size clipping
rectangle.
Created attachment 5789 [details]
clipbug.ps
A more complex version of clipbug.ps that demonstrates rectfilling an area
constrained by 6 different clipping paths. The clipping path in each case is:
1) A moveto
2) A moveto, close
3) A moveto, lineto (same position), close
4) A moteto, lineto (horizontal change), close
5) A moteto, lineto (vertical change), close
6) A moteto, lineto (diagonal change), close
Acrobat only marks the page for 4,5,6. Unpatched ghostscript for 4 and 6.
Patched ghostscript marks the page for all but 5.
Created attachment 5790 [details]
patch
Updated version of henrys patch that returns patched ghostscript to only
getting rendering as wrong as unpatched ghostscript currently does.
Basically, we add an additional few lines to ensure that we don't mark the page
when the clip is truly empty.
@@ -219,6 +220,10 @@
gs_fixed_rect clip_rect;
gx_cpath_inner_box(pcpath, &clip_rect);
+ /* We should never plot anything for an empty clip rectangle */
+ if ((clip_rect.p.x >= clip_rect.q.x) &&
+ (clip_rect.p.y >= clip_rect.q.y))
+ return 0;
for (i = 0; i < count; ++i) {
gs_fixed_point p, q;
gs_fixed_rect draw_rect;
FWIW Jaws marks the page the same as Acrobat; 4, 5 and 6. The patch has has been committed as revision 10513. The clipping bug seen with clipbug.ps has been opened as bug 691008. Therefore I'm closing this bug. |