Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#937 closed defect (fixed)

cairo driver: d.legend colors all broken

Reported by: hamish Owned by: grass-dev@…
Priority: major Milestone: 7.0.0
Component: Display Version: svn-trunk
Keywords: cairo, d.legend Cc:
CPU: x86-64 Platform: Linux

Description

Hi,

in grass7 d.legend is pretty broken if you use the Cairo driver. The color table seems out of sync or corrupted.

If you use the PNG driver it's fine.

Hamish

ps- with the PNG driver I notice the bottom right corner pixel of the legend is wrong. Line-end wants to be closed?

Attachments (1)

map.png (25.0 KB) - added by hamish 9 years ago.
cairo driver d.legend grass7

Download all attachments as: .zip

Change History (10)

comment:1 in reply to:  description Changed 9 years ago by glynn

Replying to hamish:

in grass7 d.legend is pretty broken if you use the Cairo driver. The color table seems out of sync or corrupted.

If you use the PNG driver it's fine.

I can't reproduce this.

ps- with the PNG driver I notice the bottom right corner pixel of the legend is wrong. Line-end wants to be closed?

The path is closed, but the PNG driver's line-drawing code doesn't draw the last pixel. It's a boundary condition caused by trying to draw lines exactly along the boundaries between pixels, rather than through their centres. d.legend really needs either a 0.5 pixel offset for the lines, or drawing successively smaller filled rectangles rather than stroking outlines and hoping that they align exactly with the contents.

comment:2 Changed 9 years ago by hamish

I can't reproduce this.

trunk/

export GRASS_PNGFILE=map.bmp
export GRASS_WIDTH=640
export GRASS_HEIGHT=480
export GRASS_RENDER_IMMEDIATE=cairo
export GRASS_PNG_MAPPED=TRUE
export GRASS_PNG_READ=TRUE

#spearfish
d.erase
wximgview $GRASS_PNGFILE percent=50 &
d.legend roads

(smooth legends are just as bad)

bmptopnm complains:

bmptopnm: Unrecognized bits per pixel in BMP file header: 32

so I had to use xwd+wximgview for the attached screenshot.

Hamish

Changed 9 years ago by hamish

Attachment: map.png added

cairo driver d.legend grass7

comment:3 Changed 9 years ago by hamish

G7> r.colors.out roads
1 175 98 252
2 227 173 57
3 100 2 115
4 81 219 128
5 57 207 247
nv 255 255 255
default 255 255 255

comment:4 in reply to:  2 ; Changed 9 years ago by glynn

Replying to hamish:

I can't reproduce this.

trunk/

Works for me.

But looking at the image, I'm guessing that it's an alignment problem. The header is 54 bytes, which isn't a multiple of 4. The documentation for cairo_image_surface_create_for_data() says:

This pointer must be suitably aligned for any kind of variable, (for example, a pointer returned by malloc).

It wouldn't surprise me if an unaligned frame buffer behaves incorrectly on x86-64.

Try r41029; this pads the header to 64 bytes in everything which reads/writes these files (PNG driver, cairo driver, ximgview, wximgview, wxpyimgview). The offset to the data is stored in the header (to allow for variable-length fields and the addition of new fields), so the files should still be legal (not that most software can handle them anyhow).

comment:5 in reply to:  4 ; Changed 9 years ago by hamish

Resolution: fixed
Status: newclosed

Replying to glynn:

But looking at the image, I'm guessing that it's an alignment problem. The header is 54 bytes, which isn't a multiple of 4. The documentation for cairo_image_surface_create_for_data() says:

This pointer must be suitably aligned for any kind of variable, (for example, a pointer returned by malloc).

It wouldn't surprise me if an unaligned frame buffer behaves incorrectly on x86-64.

Try r41029; this pads the header to 64 bytes in everything which reads/writes these files (PNG driver, cairo driver, ximgview, wximgview, wxpyimgview). The offset to the data is stored in the header (to allow for variable-length fields and the addition of new fields), so the files should still be legal (not that most software can handle them anyhow).

bingo. that fixed it.

qiv still opens the .bmp as a scrambled 640x64 mess, but I guess that's due to it not liking the 32bitness of it.

just out of curiosity, how do the .bmp, .ppm, and SGi's .rgb differ, in that IIUC they are all just raw binary dumps of width height r1g1b1 r2g2b2 r3g3b3 ... ?

Hamish

comment:6 in reply to:  5 Changed 9 years ago by glynn

Replying to hamish:

just out of curiosity, how do the .bmp, .ppm, and SGi's .rgb differ, in that IIUC they are all just raw binary dumps of width height r1g1b1 r2g2b2 r3g3b3 ...

PPM has a variable length header (where the header length is determined by parsing), followed by 24-bpp RGB data. BMP has a variable-length header (where the length is stored in the header) followed by data in a variety of possible formats. RGB has a fixed-size (512 byte) header followed by data in a variety of possible formats.

PPM was ruled out because it doesn't support 32-bpp data. BMP was the next candidate due to having the documentation immediately to hand. Any format which supports raw 32-bpp RGBA data (with suitable alignment) would work.

comment:7 Changed 9 years ago by hamish

ok one last question for the png,cairo driver help pages- what's the role of PGM? I just lumped it in with PPM. Is it there to cover the alpha channel, is it an automatic change over if only greyscale data is found, or is it a force-greyscale option?

thanks, Hamish

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

Replying to hamish:

ok one last question for the png,cairo driver help pages- what's the role of PGM? I just lumped it in with PPM. Is it there to cover the alpha channel, is it an automatic change over if only greyscale data is found, or is it a force-greyscale option?

PGM is used for the alpha channel.

comment:9 Changed 9 years ago by hamish

64-bit compatibility backported to devbr65 r41068.

Note: See TracTickets for help on using tickets.