Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#2403 closed defect (fixed)

cairo driver fails to create surface

Reported by: martinl Owned by: martinl
Priority: major Milestone: 7.0.0
Component: Display Version: unspecified
Keywords: cairo, resolution, surface Cc: grass-dev@…
CPU: x86-64 Platform: Linux

Description

Steps to reproduce:

g.region rast=elevation res=0.1 -p
projection: 99 (Lambert Conformal Conic)
zone:       0
datum:      nad83
ellipsoid:  a=6378137 es=0.006694380022900787
north:      228500
south:      215000
west:       630000
east:       645000
nsres:      0.1
ewres:      0.1
rows:       135000
cols:       150000
cells:      20250000000

Rendering via Cairo driver fails:

d.mon cairo
d.rast elevation
ERROR: Cairo_begin_raster(): Failed to create surface (invalid value (typically too big) for the size of the input (surface, pattern, etc.)). Using rows: 135000, cols: 150000

While PNG driver works:

d.mon -r
d.mon png
d.rast elevation
0..3..6..9..12..15..18..21..24..27..30..33..36..39..42..45..48..51..54..57..60..63..66..69..72..75..78..81..84..87..90..93..96..99..100

wxGUI also works since it uses it's own display resolution.

Attachments (1)

cairo_down_sampling.diff (5.2 KB) - added by martinl 5 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 6 years ago by martinl

Cc: grass-dev@… added
Owner: changed from grass-dev@… to martinl
Status: newassigned

comment:2 in reply to:  description ; Changed 6 years ago by glynn

Replying to martinl:

ERROR: Cairo_begin_raster(): Failed to create surface (invalid value (typically too big) for the size of the input (surface, pattern, etc.)). Using rows: 135000, cols: 150000

It would be possible to change the cairo driver (lib/cairodriver/Raster.c) to render each row separately.

However, this would effectively preclude adding certain features (e.g. anti-aliasing or rotation). It would also make a mess of the file structure for formats such as SVG.

For those reasons, I'd suggest making row-by-row rendering an alternative to the existing method rather than simply replacing it.

comment:3 in reply to:  2 ; Changed 6 years ago by martinl

Replying to glynn:

It would be possible to change the cairo driver (lib/cairodriver/Raster.c) to render each row separately.

for record https://bugs.freedesktop.org/show_bug.cgi?id=85561

comment:4 in reply to:  3 ; Changed 5 years ago by martinl

Replying to martinl:

Replying to glynn:

It would be possible to change the cairo driver (lib/cairodriver/Raster.c) to render each row separately.

for record https://bugs.freedesktop.org/show_bug.cgi?id=85561

the problem is that cairo limits number of rows and cols to 32767 (1). Any idea how this bug could be resolved in GRASS?

(1) http://cgit.freedesktop.org/cairo/tree/src/cairo-image-surface.c#n61

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

Replying to martinl:

the problem is that cairo limits number of rows and cols to 32767 (1). Any idea how this bug could be resolved in GRASS?

One option is to require clients (d.rast, d.his, d.rgb) to limit the region resolution. Another is to modify the driver(s) to decimate the data if it exceeds certain limits. Yet another is to modify the cairo driver to down-sample the data itself (this avoids the issue that a source region of 32767x32767 will require 4 GiB of memory to store the surface regardless of the output resolution).

All of these have the advantage that the client won't waste time reading rows which will be discarded by the driver (when down-sampling, the driver reports the next row which will be used).

comment:6 in reply to:  5 ; Changed 5 years ago by martinl

Replying to glynn:

another is to modify the cairo driver to down-sample the data itself (this avoids the issue that a source region of 32767x32767 will require 4 GiB of memory to store the surface regardless of the output resolution).

I was thinking about something similar, btw, is there any display driver in GRASS which already doing down-sampling? Thanks for any hint.

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

Replying to martinl:

I was thinking about something similar, btw, is there any display driver in GRASS which already doing down-sampling? Thanks for any hint.

The PNG driver resamples each row and stores it directly in the image.

The PS driver writes the data to the PostScript? file as-is (after conversion to hex). This can potentially result in huge files, but it doesn't affect memory requirements (PostScript? "streams" image data into the frame-buffer from the input file; it doesn't store it in memory prior to rendering).

comment:8 in reply to:  7 ; Changed 5 years ago by martinl

Replying to glynn:

The PNG driver resamples each row and stores it directly in the image.

Thanks for the hint. Could you please review attachment:cairo_down_sampling.diff which do down-sampling for Cairo in similar as already does PNG driver?

Down-sampling has of course a significant speed effect when rendering larger rasters using their original region, eg.

g.region rast=dmt -p
...
rows:       11682
cols:       18913
cells:      220941666
...

# cairo: no down-sampling
time GRASS_RENDER_IMMEDIATE=cairo d.rast dmt
real    0m40.797s

# cairo: down-sampling (patch applied)
time GRASS_RENDER_IMMEDIATE=cairo d.rast dmt
real    0m1.400s

Changed 5 years ago by martinl

Attachment: cairo_down_sampling.diff added

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

Replying to martinl:

Could you please review attachment:cairo_down_sampling.diff which do down-sampling for Cairo in similar as already does PNG driver?

The main issue was that it neglected the inner Y loop when up-scaling.

r63582 makes the cairo driver behave almost identically to the PNG driver.

For reasons which I've been unable to determine so far, the result is one destination pixel wider than with the PNG driver.

Ultimately, the code should be modified to use an intermediate surface which has the same dimensions as the destination rectangle, to avoid the overhead of copying the (empty) borders.

comment:10 in reply to:  9 Changed 5 years ago by martinl

Resolution: fixed
Status: assignedclosed

Replying to glynn:

r63582 makes the cairo driver behave almost identically to the PNG driver.

I took liberty to backport it in r63595.

For reasons which I've been unable to determine so far, the result is one destination pixel wider than with the PNG driver.

Ultimately, the code should be modified to use an intermediate surface which has the same dimensions as the destination rectangle, to avoid the overhead of copying the (empty) borders.

Since the originally reported issue has been fixed, I took liberty to close this ticket.

comment:11 Changed 5 years ago by neteler

This is excellent! Thanks, Glynn.

This (formerly failing) example was done over NFS mounted /grassdata over a 1Gb/s intranet connection with local GRASS GIS installation and remote network data repository:

GRASS 7.0.0svn (eu_laea):~ > g.region rast=lst_2014_03_minimum -p
projection: 99 (Lambert Azimuthal Equal Area)
zone:       0
datum:      etrs89
ellipsoid:  grs80
north:      5447750
south:      770000
west:       2168000
east:       7716750
nsres:      250
ewres:      250
rows:       18711
cols:       22195
cells:      415290645

GRASS 7.0.0svn (eu_laea):~ > d.mon wx0

GRASS 7.0.0svn (eu_laea):~ > time -p d.rast lst_2014_03_minimum 
 100%
real 34.12
user 33.13
sys 0.67

##### comparison to see how long data reading itself takes:
GRASS 7.0.0svn (eu_laea):~ > time -p r.stats lst_2014_03_minimum 
...
real 12.45
user 12.26
sys 0.12

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

Replying to neteler:

GRASS 7.0.0svn (eu_laea):~ > time -p d.rast lst_2014_03_minimum 

##### comparison to see how long data reading itself takes:
GRASS 7.0.0svn (eu_laea):~ > time -p r.stats lst_2014_03_minimum 

FWIW, the comparison isn't particularly meaningful, as d.rast now only needs to read the rows which are actually rendered, while r.stats needs to read the entire map.

Beyond the need to uncompress entire rows, the rendering time should be dictated by the output width, not the input width. However, d.rast's values= feature adds overhead proportional to the input width.

Can you try commenting out the mask_raster_array() call at line 71 of display/d.rast/display.c? If that has a significant impact, we should try to skip that call altogether when values= isn't being used.

comment:13 in reply to:  12 Changed 5 years ago by neteler

Replying to glynn:

Can you try commenting out the mask_raster_array() call at line 71 of display/d.rast/display.c? If that has a significant impact, we should try to skip that call altogether when values= isn't being used.

Sure. Test setting: connected from home via ssh/tsocks tunnel over ADSL connection. Opening GRASS 7.0.svn (r63585) remotely, tunnelling wx0 over network:

rows:       18711
cols:       22195
cells:      415290645

# original
d.mon wx0
time -p d.rast europe_altitude_map.shade
 100%
real 4.64
user 0.93
sys 0.03

# with commented line of mask_raster_array() call
d.mon wx0 # ... previously opened monitor closed
time -p d.rast europe_altitude_map.shade
 100%
real 0.92
user 0.91
sys 0.01

Probably that's significant.

Note: This is the time when d.rast finishes and returns to the command line. The display in d.mon wx0 comes many seconds later but that's of course a different issue.

comment:14 in reply to:  9 ; Changed 5 years ago by neteler

Replying to glynn:

r63582 makes the cairo driver behave almost identically to the PNG driver.

For reasons which I've been unable to determine so far, the result is one destination pixel wider than with the PNG driver.

Not sure if related but here I remember ticket #72.

Ultimately, the code should be modified to use an intermediate surface which has the same dimensions as the destination rectangle, to avoid the overhead of copying the (empty) borders.

Should the ticket be reopened for this?

comment:15 in reply to:  14 Changed 5 years ago by martinl

Replying to neteler:

Ultimately, the code should be modified to use an intermediate surface which has the same dimensions as the destination rectangle, to avoid the overhead of copying the (empty) borders.

Should the ticket be reopened for this?

If so, please open a new ticket. It's hard to follow tickets with multiple issues...

Note: See TracTickets for help on using tickets.