Opened 12 years ago

Last modified 4 years ago

#72 new defect

PNG driver: boundary rendering is off by one pixel

Reported by: hamish Owned by: grass-dev@…
Priority: major Milestone: 6.4.6
Component: Display Version: svn-develbranch6
Keywords: d.vect, rendering, PNG_DRIVER Cc:
CPU: Unspecified Platform: Unspecified

Description

Hi,

when rendering an area with gis.m's display manager the boundary seems to be one pixel off to the right. (or fill is one pixel off to the left depending on your point of view) In the xmon things are fine, using all d.vect render methods.

see the screenshot attached to this bug report.

similar things were seen before,

http://bambi.otago.ac.nz/hamish/grass/bugs/d.vect/

but Glynn has now AFAICT fixed that for all d.vect + Xdriver render modes.

?

thanks, Hamish

Attachments (11)

offby1_rend.png (33.4 KB) - added by hamish 12 years ago.
gis.m boundary off-by-one screenshot
png_offby1_zoom.png (759 bytes) - added by hamish 11 years ago.
zoom of boundary/fill with "x" at node
png_offby1_zoom2.png (564 bytes) - added by hamish 11 years ago.
another xmag shot, this time with "+" as symbol
geo_symb_fill.png (324 bytes) - added by hamish 11 years ago.
new geology symbol with bad triangle fill
pngdriver-polygon.patch (904 bytes) - added by glynn 11 years ago.
patch for PNG driver polygon fill
map1.png (150 bytes) - added by glynn 11 years ago.
test case before fix (PNG)
map1.svg.bz2 (2.1 KB) - added by glynn 11 years ago.
test case before fix (SVG)
map2.png (150 bytes) - added by glynn 11 years ago.
test case after fix (PNG)
map2.svg.bz2 (2.1 KB) - added by glynn 11 years ago.
test case after fix (SVG)
map2_xmag.png (3.2 KB) - added by hamish 11 years ago.
Glynn's "after" png zoomed
dvect_gis.m.png (986 bytes) - added by hamish 10 years ago.
off-by-one in GIS.m 6.5svn aug 2009

Download all attachments as: .zip

Change History (39)

Changed 12 years ago by hamish

Attachment: offby1_rend.png added

gis.m boundary off-by-one screenshot

comment:1 Changed 12 years ago by martinl

Component: defaultTcl

comment:2 Changed 11 years ago by hamish

Keywords: PNG_DRIVER added
Priority: minorblocker
Summary: gis.m: boundary rendering is off by one pixelPNG driver: boundary rendering is off by one pixel

Hi,

This seems to be a generic problem with the PNG driver and 'd.vect rend=c'.

#spearfish
g.region n=4928610 s=4919190 w=596700 e=598860 res=30
d.mon x0
d.vect fields type=area
d.out.file offby1

qiv offby1.png

It was there last time I rand the rendering tests, but I guess I missed that the error was still there. Using xmag on the tiny peninsula in the rend=c image shows the problem too:

http://bambi.otago.ac.nz/hamish/grass/bugs/d.vect/dec2008/dvect_render_fill_PNGdriver.png

As noted here:

http://article.gmane.org/gmane.comp.gis.grass.devel/23833

"PNG driver: only 'g' lines up correctly. 'c' isn't so bad, it looks ok in the y, but off by one in the x (boundary line 1px to the right vs fill). [exploring with xmag]"

thanks, Hamish

Changed 11 years ago by hamish

Attachment: png_offby1_zoom.png added

zoom of boundary/fill with "x" at node

comment:3 Changed 11 years ago by hamish

Hi, I've just added a new xmag screenshot showing that it is the area fill which is displaced, not the boundary line. (or both the boundary line and symbols travel together)

#spearfish
v.out.ascii fields format=standard | sed -e '1,10d' | \
   grep -v '^C\|^B' | grep -v '^ 1 ' > field_corners.dat


v.in.ascii field_corners.dat fs=space x=1 y=2 out=field_corners

# then display them both in gis.m

Hamish

Changed 11 years ago by hamish

Attachment: png_offby1_zoom2.png added

another xmag shot, this time with "+" as symbol

comment:4 Changed 11 years ago by neteler

Milestone: 6.3.06.4.0

comment:5 Changed 11 years ago by cmbarton

It's easy enough to put in the rendering mode for d.* commands. But there are a couple of questions. 1) Is it ONLY d.vect that has a problem? 2) I've seen discussion go back and forth over which of these switches to use. Has that been settled in this case?

Michael

comment:6 Changed 11 years ago by hamish

Version: svn-trunksvn-develbranch6

Michael:

It's easy enough to put in the rendering mode for d.* commands.

There is no need for the GUI to expose the render mode to the user, it's a debug switch to aid development.

1) Is it ONLY d.vect that has a problem?

no, it's a problem with the rendering library functions. the different render= switches choose which lib fns to render with.

2) I've seen discussion go back and forth over which of these switches to use. Has that been settled in this case?

For d.vect it has been changed to "c" as the best compromise for GRASS 6. (where "best" means it was the last man standing; IIRC some of the otherwise more favoured candidates had the possibility of writing outside of the X buffer etc. and so were disqualified)

see Glynn's reply of 2008-04-16 which covers the guts of this bug,

http://article.gmane.org/gmane.comp.gis.grass.devel/26529

(I can't get on any high-horse about using the trac system right now as the trac server is currently unresponsive, while email continues to flow through)

I had thought to post new screenshot tests responding to that post exploring my observation that the current problem is with near vertical lines. Spearfish's fields vector layer is a good example as it has many near horiz + vertical boundaries and is the basis of the first 2 of 3 screenshots attached to this report. The third is varied coastline data, but demonstrates what happens with a diversity of line angles. In light of those 3 I don't think any more screenshots would provide any more info.

Glynn wrote in the above post: "It wouldn't be particularly hard to make the closer-to-vertical case match, by using the same algorithm in both cases. Making the closer-to-horizontal case match is harder (and messy)."

I'm cautiously optimistic that the problem is with the closer-to-vertical case. If so, and thus it isn't a major developer drain to fix it, then I'd really hope we could do that for 6.4 as for GRASS 6 the PNG driver is our primary d.* hardcopy output method.

My current workaround is to do:

alias xwdtopng='xwd | xwdtopnm | pnmtopng > '
xwdtopng image.png

GRASS 7 is expected to use floating point coordinates for rendering, so the problem goes away (or is replaced by another).

Hamish

comment:7 Changed 11 years ago by cmbarton

Component: Tcldefault
Keywords: d.vect added; gis.m removed

Changed component from TclTk? to default. This is not a TclTk? problem.

Changed 11 years ago by hamish

Attachment: geo_symb_fill.png added

new geology symbol with bad triangle fill

comment:8 Changed 11 years ago by hamish

CPU: Unspecified
Platform: Unspecified

new screenshot added showing 6.4svn PNG driver output (d.out.file) of new strike_triangle symbol with bad fill/outline alignment.

Hamish

comment:9 in reply to:  8 ; Changed 11 years ago by glynn

Replying to hamish:

new screenshot added showing 6.4svn PNG driver output (d.out.file) of new strike_triangle symbol with bad fill/outline alignment.

Please provide the exact sequence of commands used to create the image.

comment:10 in reply to:  9 ; Changed 11 years ago by hamish

Replying to hamish:

new screenshot added showing 6.4svn PNG driver output (d.out.file) of new strike_triangle symbol with bad fill/outline alignment.

Replying to glynn:

Please provide the exact sequence of commands used to create the image.

# grass 6.4svn from yesterday
echo "symbol geology/strike_triangle 22 50 50 black black" > draw_triangle.dgr
d.graph draw_triangle.dgr
d.out.file strike_triangle_PNG_driver

seems dependent on the symbol size, I tried 125 but it looked ok, so went back to 22 as per the screenshot and it misaligns.

Hamish

Changed 11 years ago by glynn

Attachment: pngdriver-polygon.patch added

patch for PNG driver polygon fill

Changed 11 years ago by glynn

Attachment: map1.png added

test case before fix (PNG)

Changed 11 years ago by glynn

Attachment: map1.svg.bz2 added

test case before fix (SVG)

Changed 11 years ago by glynn

Attachment: map2.png added

test case after fix (PNG)

Changed 11 years ago by glynn

Attachment: map2.svg.bz2 added

test case after fix (SVG)

comment:11 in reply to:  10 ; Changed 11 years ago by glynn

Replying to hamish:

echo "symbol geology/strike_triangle 22 50 50 black black" > draw_triangle.dgr
d.graph draw_triangle.dgr

Okay. There were two problems with the existing polygon filler, which should be fixed by attachment:ticket:72:pngdriver-polygon.patch.

The first is that the x coordinates were calculated for the top of the pixel rather than the centre, causing an effective downward shift.

The second is that the x coordinates were truncated rather than rounded, causing a leftward shift.

In the test case, both of these contribute to the gap on the right-hand side, while the latter contributes to the overlap on the left-hand side.

The one issue which this patch doesn't attempt to address is the fact that all lines are effectively shifted half a pixel down and to the right. This is inherent in the use of integer coordinates. While you can draw a polygon with orthogonal edges perfectly aligned to the pixel grid, you can't draw a single-pixel orthogonal stroke with its centre-line so aligned.

I have attached PNG files containing the output before and after the patch, as well as corresponding SVG files which show exactly what is happening in more detail. The actual vertices passed to the rendering functions (both line and polygon) are [(315,240), (320,229), (325,240)]. This corresponds to the black triangle. The red triangle shows the half pixel down-right shift inherent in the line drawing.

Changed 11 years ago by hamish

Attachment: map2_xmag.png added

Glynn's "after" png zoomed

comment:12 in reply to:  11 ; Changed 11 years ago by hamish

Replying to glynn:

Okay. There were two problems with the existing polygon filler, which should be fixed by attachment:ticket:72:pngdriver-polygon.patch.

...

The one issue which this patch doesn't attempt to address is the fact that all lines are effectively shifted half a pixel down and to the right. This is inherent in the use of integer coordinates.

any preprocessing floor(0.5+x) voodoo of the line data to align them with the polygon rendering? [yeah, that's what got us into this mess in the first place]

While you can draw a polygon with orthogonal edges perfectly aligned to the pixel grid, you can't draw a single-pixel orthogonal stroke with its centre-line so aligned.

... as it would have to be zero or two pixels wide.

I have attached PNG files containing the output before and after the patch, as well as corresponding SVG files which show exactly what is happening in more detail.

I've attached a zoom of "after" showing the left over stray pixels shown in the SVG. Even with those, a nice improvement.

thanks a lot for looking into this- to me this issue is a really bad look for us, sloppy output makes the project look sloppy. (and annoying for me to have to use xwd instead of d.out.file in 6.4.

I'll try the patch in the AM.

cheers, Hamish

comment:13 in reply to:  12 ; Changed 11 years ago by glynn

Replying to hamish:

thanks a lot for looking into this- to me this issue is a really bad look for us, sloppy output makes the project look sloppy. (and annoying for me to have to use xwd instead of d.out.file in 6.4.

If you want quality, I would expect both the cairo and PS drivers to produce better results overall than either PNG or XDRIVER. However, you might run into corner cases where code relies upon the half pixel down-right shift, which isn't present in those drivers. You might also run into problems with the distinction between polylines and multiple line segments (caps versus joins) being visible.

comment:14 in reply to:  13 ; Changed 11 years ago by hamish

Replying to glynn:

If you want quality, I would expect both the cairo and PS drivers to produce better results overall than either PNG or XDRIVER.

both are less WYSIWYG. Cairo thickens and antialiases lines, and PS driver in 6.4 is limited by 6.4, and needs to be passed through pstoimg or ghostscript to get the web/presentation image which has its own rasterization quirks.

You might also run into problems with the distinction between polylines and multiple line segments (caps versus joins) being visible.

unrelated, but ps.map's region box command has that problem, obvious once you set the width a bit wider.

I ran some 'd.vect rend=' tests with the latest 6.4svn (w/ Polygon.c commit). screenshots & scripts here:

http://bambi/hamish/grass/bugs/d.vect/dec2008/

results for render mode trials 5 Dec 2008

coastline
=========

6.3.0
-----
xdriver:   R, C perfect; G near perfect; D, L off (x+y)
pngdriver: R, C off (x); G perfect; D off (y); L off(x+y)


6.4.svn 20081205 (with Polygon.c patch)
-------
xdriver:   R, C, G perfect; D, L off (x+y)
pngdriver: R, C off (x); G perfect; D, L off (x)
gis.m: off (x)

inter-version
-------------
xdriver-   results mostly unchanged (fixed 1 pixel in G)
pngdriver- D,L slightly improved



boxes
=====
6.4svn xdriver: D, L off (x+y)
6.4svn png driver: all look good.
6.4svn gis.m: looks good

Hamish

comment:15 in reply to:  14 Changed 11 years ago by hamish

Replying to hamish:

I ran some 'd.vect rend=' tests with the latest 6.4svn (w/ Polygon.c commit). screenshots & scripts here:

http://bambi/hamish/grass/bugs/d.vect/dec2008/

ahem,

http://bambi.otago.ac.nz/hamish/grass/bugs/d.vect/dec2008/

'd.vect rend=g' seems to work ok with the PNG driver, due to thicker lines which cover up any gaps (trade one problem for another).

Hamish

comment:16 Changed 11 years ago by hamish

... another thing I noticed in those screenshots is that 'd.text size=5' (percent) is different between 6.3.0 and 6.4svn for the same frame size. shrug.

and of course width=2 covers up minor sins.

Hamish

comment:17 Changed 11 years ago by hamish

see also trac #232.

Hamish

comment:18 Changed 10 years ago by hamish

Priority: blockercritical

comment:19 Changed 10 years ago by hamish

a similar thing is visible in the tcltk gis.m GUI in 6.5.svn if you do add vector layer; bugsites (@spearfish)*; icon: basic/circle; size column: cat; [Render]. see attached xmag screenshot (dvect_gis.m.png).

[*] can the gui layer manager lists show the mapnames like this? it's a bit easier to read

from a 6.5svn xmon it renders correctly.

Hamish

Changed 10 years ago by hamish

Attachment: dvect_gis.m.png added

off-by-one in GIS.m 6.5svn aug 2009

comment:20 Changed 8 years ago by hamish

see also #138

comment:21 Changed 7 years ago by hamish

Milestone: 6.4.06.4.4

to late in 6.4.3 for deep changes to the PNG driver, bumping down the road to 6.4.4.

comment:22 Changed 6 years ago by hamish

Hi,

there is a similar off-by-one problem in the Cairo driver for 6.4 and 6.5, very easy to reproduce:

export GRASS_WIDTH=100
export GRASS_HEIGHT=100
d.mon start=cairo
d.mon stop=cairo

you will see a single pixel black line along both the right and bottom edges of the resulting map.png file. (trouble if you wanted to use GRASS_TRANSPARENT=TRUE)

crummy work-around is to make GRASS_WIDTH and HEIGHT one larger, then crop away the bad edges with pnmcrop.

Hamish

comment:23 in reply to:  22 ; Changed 6 years ago by mmetz

Replying to hamish:

Hi,

there is a similar off-by-one problem in the Cairo driver for 6.4 and 6.5, very easy to reproduce:

export GRASS_WIDTH=100
export GRASS_HEIGHT=100
d.mon start=cairo
d.mon stop=cairo

you will see a single pixel black line along both the right and bottom edges of the resulting map.png file. (trouble if you wanted to use GRASS_TRANSPARENT=TRUE)

This has been introduced by you with r12527. Reverting r12527 removes the single pixel black line along both the right and bottom edges. Do you remember by any chance the reason for r12527?

Note that the line drawn by D_show_window() on the left and top edges is outside the current frame.

comment:24 in reply to:  23 ; Changed 6 years ago by mlennert

Replying to mmetz:

Replying to hamish:

Hi,

there is a similar off-by-one problem in the Cairo driver for 6.4 and 6.5, very easy to reproduce:

export GRASS_WIDTH=100
export GRASS_HEIGHT=100
d.mon start=cairo
d.mon stop=cairo

you will see a single pixel black line along both the right and bottom edges of the resulting map.png file. (trouble if you wanted to use GRASS_TRANSPARENT=TRUE)

This has been introduced by you with r12527. Reverting r12527 removes the single pixel black line along both the right and bottom edges. Do you remember by any chance the reason for r12527?

It's explained in the mailing list message referenced in the changelog:

http://grass.fbk.eu/pipermail/grass5/2004-February/013625.html

"I was having a look at why d.frame's boxes don't line up properly.."

comment:25 in reply to:  24 Changed 6 years ago by mmetz

Replying to mlennert:

Replying to mmetz:

Replying to hamish:

Hi,

there is a similar off-by-one problem in the Cairo driver for 6.4 and 6.5, very easy to reproduce:

export GRASS_WIDTH=100
export GRASS_HEIGHT=100
d.mon start=cairo
d.mon stop=cairo

you will see a single pixel black line along both the right and bottom edges of the resulting map.png file. (trouble if you wanted to use GRASS_TRANSPARENT=TRUE)

This has been introduced by you with r12527. Reverting r12527 removes the single pixel black line along both the right and bottom edges. Do you remember by any chance the reason for r12527?

It's explained in the mailing list message referenced in the changelog:

http://grass.fbk.eu/pipermail/grass5/2004-February/013625.html

"I was having a look at why d.frame's boxes don't line up properly.."

This seems to be two problems at once. d.frame tries to draw a one pixel wide black border around each frame, but does so outside the frame. That means that each frame is eating away one row / column from each adjacent frame, resulting in 2 pixel wide borders, thus "you can see that the frames are drawn one pixel to wide and tall".

The fact that d.frame draws a border outside the frame is due to D_show_window() which draws outside the current screen window.

The ideal solution would be if D_show_window() would not draw a border outside the current screen window and if d.frame would leave a one pixel wide gap between frames to draw a border between frames in that gap.

comment:26 Changed 5 years ago by hamish

Component: DefaultDisplay
Milestone: 6.4.46.4.5
Priority: criticalmajor

to get back to the main point of this ticket, by "boundary" I mean polygon area boundary not display frame outer edge.

the fill area of a circle does not line up with the black line drawn around its edge.

I'll get the screenshot back up at some point, for now see the image attached to this ticket.

Hamish

comment:27 Changed 4 years ago by martinl

Milestone: 6.4.5

Ticket retargeted after milestone closed

comment:28 Changed 4 years ago by martinl

Milestone: 6.4.6
Note: See TracTickets for help on using tickets.