[GRASS5] d.photo

GRASS GRASS developer
1

Hi all,

I am currently digitizing vectors using many large orthophotos
on background. I had been a bit annoyed by speed of d.rgb
so I wrote d.photo (slightly modified d.rgb) which can display
orthophoto raster map created from 3 (RGB) raster maps (1 byte/cell) by
r.mapcalc pok2="043130.r + 043130.g * 256 + 043130.b * 65536"
(3 bytes/cell in result)

d.photo is significantly faster than d.rgb:

time d.rgb r=043130.r g=043130.g b=043130.b
real 0m6.810s
user 0m6.000s
sys 0m0.393s

time d.photo map=pok2
real 0m3.199s
user 0m2.760s
sys 0m0.211s

Does it make sense to add d.photo to GRASS? As it is so simple
and d.photo take less than 50% of time of d.rgb I think that
I am missing something. Maybe we already have such module?

Radim

2

Radim Blazek wrote:

I am currently digitizing vectors using many large orthophotos
on background. I had been a bit annoyed by speed of d.rgb
so I wrote d.photo (slightly modified d.rgb) which can display
orthophoto raster map created from 3 (RGB) raster maps (1 byte/cell) by
r.mapcalc pok2="043130.r + 043130.g * 256 + 043130.b * 65536"
(3 bytes/cell in result)

d.photo is significantly faster than d.rgb:

time d.rgb r=043130.r g=043130.g b=043130.b
real 0m6.810s
user 0m6.000s
sys 0m0.393s

time d.photo map=pok2
real 0m3.199s
user 0m2.760s
sys 0m0.211s

Does it make sense to add d.photo to GRASS?

Probably not; see below.

As it is so simple
and d.photo take less than 50% of time of d.rgb I think that
I am missing something. Maybe we already have such module?

Some issues to consider:

1. d.rgb uses the maps' colour tables to obtain the intensity values,
rather than assuming a direct translation. This could be expensive,
particularly if the colour tables have 256 individual entries rather
than a single rule.

2. r.composite does the equivalent of the above r.mapcalc command, but
again it uses the colour tables (and it isn't limited to powers of 2).

3. You are presumably creating a very large colour table for the
composite raster (either ~16 million discrete entries, or 65536
rules).

4. d.rast quantises images to at most 32768 colours (5 bits per
component, 15 bits total). d.rgb doesn't do any quantisation; on a
24-bpp display, you should actually get 24-bpp on-screen.

If this type of raster is worth treating as a special case (and it
quite possibly is), d.rgb (and possibly r.composite) should have a
flag to ignore the colour tables (assuming that's where the
performance hit actually lies).

Basically, it doesn't make sense (to me) to display an RGB image by
generating a composite layer and a colour table, which are passed to
the display driver only to be converted back to RGB.

If you try comparing like-for-like, and find that a composite raster
seems to be *inherently* faster than using the RGB raster functions,
something's wrong with the RGB raster functions.

Try replacing the G_get_raster_color() calls in D_raster_of_type_RGB()
in src/libes/display/raster_rgb.c with simple CELL -> byte truncation.

The fact that d.rgb is using proportionally more "user" time suggests
that the issue is with the client (i.e. d.rgb or libdisplay), not
XDRIVER or I/O overhead.

--
Glynn Clements <glynn.clements@virgin.net>

3

On Friday 18 April 2003 01:54 am, Glynn Clements wrote:

Radim Blazek wrote:
> r.mapcalc pok2="043130.r + 043130.g * 256 + 043130.b * 65536"
>
> time d.rgb r=043130.r g=043130.g b=043130.b
> real 0m6.810s
> user 0m6.000s
> sys 0m0.393s
>
> time d.photo map=pok2
> real 0m3.199s
> user 0m2.760s
> sys 0m0.211s
>
> Does it make sense to add d.photo to GRASS?

Probably not; see below.

> As it is so simple
> and d.photo take less than 50% of time of d.rgb I think that
> I am missing something. Maybe we already have such module?

Some issues to consider:

1. d.rgb uses the maps' colour tables to obtain the intensity values,
rather than assuming a direct translation. This could be expensive,
particularly if the colour tables have 256 individual entries rather
than a single rule.

No, all original maps have only 1 rule in color tables.

2. r.composite does the equivalent of the above r.mapcalc command, but
again it uses the colour tables (and it isn't limited to powers of 2).

3. You are presumably creating a very large colour table for the
composite raster (either ~16 million discrete entries, or 65536
rules).

r.composite r_map=043130.r g_map=043130.g b_map=043130.b output=pok1 levels=32
time d.rast map=pok1
real 0m2.066s
user 0m1.760s
sys 0m0.162s

Yes, this is even faster, but output (even if usually acceptable) is not
of the same quality as that done by d.rgb/d.photo.

r.composite r_map=043130.r g_map=043130.g b_map=043130.b output=pok1 levels=64
time d.rast map=pok1
real 0m2.796s
user 0m2.492s
sys 0m0.156s

r.composite r_map=043130.r g_map=043130.g b_map=043130.b output=pok1 levels=256
time d.rast map=pok1
-> killed after 8 minutes still on 0% progress

4. d.rast quantises images to at most 32768 colours (5 bits per
component, 15 bits total). d.rgb doesn't do any quantisation; on a
24-bpp display, you should actually get 24-bpp on-screen.

And so I think, that we can compare comparable only (in sense of
output quality), and comparable are d.rgb and d.photo, I think,
as d.rast can use 15 bits only.

If this type of raster is worth treating as a special case (and it
quite possibly is), d.rgb (and possibly r.composite) should have a
flag to ignore the colour tables (assuming that's where the
performance hit actually lies).

As I mentioned above, input maps have only 1 rule in color tables.
d.photo is also using D_draw_raster_RGB() with color tables
containing 1 rule. So it seems that here is not the problem.

Basically, it doesn't make sense (to me) to display an RGB image by
generating a composite layer and a colour table, which are passed to
the display driver only to be converted back to RGB.

D_draw_raster_RGB() doesn't do that, right?

If you try comparing like-for-like, and find that a composite raster
seems to be *inherently* faster than using the RGB raster functions,
something's wrong with the RGB raster functions.

As both d.rgb and d.photo are using D_draw_raster_RGB(), the difference
seems to be between reading 1 (3 bytes/cell) and 3 (1 byte/cell) rasters.

Try replacing the G_get_raster_color() calls in D_raster_of_type_RGB()
in src/libes/display/raster_rgb.c with simple CELL -> byte truncation.

I am not quite sure how, this works:
r_buf[i] = (unsigned char) *r_raster;
g_buf[i] = (unsigned char) *g_raster;
b_buf[i] = (unsigned char) *b_raster;

but the time is the same (as we could expect for one rule in color table)
time d.rgb r=043130.r g=043130.g b=043130.b
real 0m6.837s
user 0m5.959s
sys 0m0.420s

Radim

4

On Fri, Apr 18, 2003 at 12:54:58AM +0100, Glynn Clements wrote:
[...]

> d.photo is significantly faster than d.rgb:

[...]

Some issues to consider:

1. d.rgb uses the maps' colour tables to obtain the intensity values,
rather than assuming a direct translation.

what about a flag to support direct translation? Then most people
should be happy.
I don't know if this required many changes or basically some condition
for G_read_colors()?

Markus

5

Radim Blazek wrote:

> If this type of raster is worth treating as a special case (and it
> quite possibly is), d.rgb (and possibly r.composite) should have a
> flag to ignore the colour tables (assuming that's where the
> performance hit actually lies).

As I mentioned above, input maps have only 1 rule in color tables.
d.photo is also using D_draw_raster_RGB() with color tables
containing 1 rule. So it seems that here is not the problem.

OK; because you were creating a composite map, I had assumed that you
were using the paletted raster functions. So, ignore most of what I
said earlier.

Given that you aren't, what's the point in creating the composite map
rather than just reading the three separate maps directly?

> Basically, it doesn't make sense (to me) to display an RGB image by
> generating a composite layer and a colour table, which are passed to
> the display driver only to be converted back to RGB.

D_draw_raster_RGB() doesn't do that, right?

Right.

> If you try comparing like-for-like, and find that a composite raster
> seems to be *inherently* faster than using the RGB raster functions,
> something's wrong with the RGB raster functions.

As both d.rgb and d.photo are using D_draw_raster_RGB(), the difference
seems to be between reading 1 (3 bytes/cell) and 3 (1 byte/cell) rasters.

Except that the r.mapcalc command is having to read 3 rasters. Is the
time taken by the r.mapcalc command included in the overall times?

Or were you effectively just pointing out that, if you only time part
of the process, you get a lower number?

Bear in mind that the composite map isn't much use for anything, other
than displaying it, and then only with d.photo. Displaying it with
d.rast would require it to have a valid colour table (65536 rules).
Ditto for use as a colour layer in NVIZ.

AFAICT, the problems which d.photo gets around (for one very specific
application are):

1. The underlying libgis functionality for reading raster maps is
slow.

2. GRASS' colour handling sucks (both the colour table handling in
libgis and the display issues in libraster/libdisplay and the
drivers).

Solving the underlying problems would be worthwhile (but not trivial);
creating special-case work-arounds isn't, IMHO.

Note that solving either of the two problems listed above would be
sufficient in this particular case.

Solving point 1 would reduce the performance gap between d.rgb and
d.photo (both use the same drawing functions, but differ in how they
obtain the data). It would also improve most of the raster modules.

Solving point 2 (e.g. by supporting RGB colour tables directly in
libgis, libraster, libdisplay and the drivers) would make it practical
to use r.composite and d.rast with 256 levels.

AFAICT, d.photo is an attempt to solve point 2 for a single specific
case, limiting the RGB support into a single program.

Maps which work with d.photo either won't work with d.rast, ps.map,
r.out.{png,ppm,tga,tiff}, NVIZ etc, or (if you provide a suitable
colour table, which d.photo will ignore) will be impractically slow.

With three separate R/G/B maps, the individual channels are themselves
valid raster maps, and suitable commands exist for processing the
group as a whole. The composite maps generated by r.composite are also
valid maps; you just have to trade quality for performance.

--
Glynn Clements <glynn.clements@virgin.net>

6

On Friday 18 April 2003 06:04 pm, Glynn Clements wrote:

Solving point 2 (e.g. by supporting RGB colour tables directly in
libgis, libraster, libdisplay and the drivers) would make it practical
to use r.composite and d.rast with 256 levels.

That would be nice. Speed of d.rast is critical for interactive use.

How RGB CT will look like? Just tells how many bytes are assigned
to each chanel?

Radim

7

Radim Blazek wrote:

> Solving point 2 (e.g. by supporting RGB colour tables directly in
> libgis, libraster, libdisplay and the drivers) would make it practical
> to use r.composite and d.rast with 256 levels.

That would be nice.

It's also the most complex option.

Given that the speed difference between d.rgb and d.photo appears to
be almost entirely due to the additional time spent reading three maps
instead of one, it would definitely be worth looking into whether the
reading speed can be improved. If it could, that would benefit
everything.

BTW, the issue definitely isn't the speed of the underlying read()
operations, as that wouldn't be counted as "user" time (which is
definitely the dominant factor).

Speed of d.rast is critical for interactive use.

Then buy a faster computer :wink:

Actually, that's not entirely a joke. There is a legitimate argument
for sacrificing performance in favour of other properties. In this
case, simplicity. You can almost always get performance improvements
by replacing generic functions with multiple specific-case versions,
but doing so creates maintainence problems, and maintenance is GRASS'
biggest problem.

How RGB CT will look like? Just tells how many bytes are assigned
to each chanel?

The most general option would be to specify a modulus and divisor for
each channel, i.e. not restricted to powers of two. The next most
general would be the number of bits and the ordering (e.g. R8G8B8).
The least general would be to fix the format as either R8G8B8 or
B8G8R8.

In all cases, the colour "table" would have to begin with a specific
token which identifies the mapping as a direct RGB mapping rather than
a table.

Some new functions would need to be added for use by RGB-aware
programs, and some existing functions would need to be extended to
provide compatibility with non-RGB-aware programs (i.e. generate the
equivalent colour table on the fly).

At a minimum, there would need to be new functions to check whether a
colour table is a direct RGB mapping, and to obtain the parameters
(unless they're fixed).

Also, some key functions (e.g. G__lookup_colors()) would be extended
to use the RGB mapping directly. That should make programs which are
just going to convert to RGB (e.g. r.out.*, ps.map) feasible.

Making R_raster_* feasible would involve extending the monitor
protocol and the drivers to provide an RGB alternative to
R_reset_colors (so that the monitors don't create a 64Mb translation
table). It would probably be easier to extend programs such as d.rast
to use the RGB drawing functions if the raster has a direct RGB
mapping.

--
Glynn Clements <glynn.clements@virgin.net>

8

On Tuesday 22 April 2003 05:48 pm, Glynn Clements wrote:

Actually, that's not entirely a joke. There is a legitimate argument
for sacrificing performance in favour of other properties. In this
case, simplicity. You can almost always get performance improvements
by replacing generic functions with multiple specific-case versions,
but doing so creates maintainence problems, and maintenance is GRASS'
biggest problem.

Can I conclude?: GRASS needs RGB CT but we (you) don't feel to have
enough human sources for GRASS maintenance and prefer not to increase
complexity for now.

So back to

Given that the speed difference between d.rgb and d.photo appears to
be almost entirely due to the additional time spent reading three maps
instead of one, it would definitely be worth looking into whether the
reading speed can be improved. If it could, that would benefit
everything.

I commented
   stat = embed_nulls_nomask(fd, (void *) cell, row, CELL_TYPE, 0);
in G_get_c_raster_row_nomask() and time fall down to 1.469s from
original 6.930s. So all evil comes from NULLs.

embed_nulls()
  -> G_get_null_value_row()
    -> G_get_null_value_row_nomask()
and G_get_null_value_row_nomask() spends most of the time
(2.161s if commented)

It means that G_get_null_value_row() takes more than 230%
of the time required for G_get_c_raster_row() (if NULLs not read).
It must be possible to improve performance of G_get_null_value_row()
but rasters are not my hobby.

BTW: It seems that raster values are masked by NULLs in current region
resolution instead of original raster map resolution. I am not sure if it is good.
That is probably more general problem of conversion of rasters to current region.

Radim

9

On Wed, Apr 23, 2003 at 11:55:38AM +0200, Radim Blazek wrote:
[...]

It means that G_get_null_value_row() takes more than 230%
of the time required for G_get_c_raster_row() (if NULLs not read).
It must be possible to improve performance of G_get_null_value_row()
but rasters are not my hobby.

BTW: It seems that raster values are masked by NULLs in current region
resolution instead of original raster map resolution. I am not sure if it is good.
That is probably more general problem of conversion of rasters to current region.

There was some time ago a discussion on NULLs

http://grass.itc.it/pipermail/grass5/2001-June/000307.html

Maybe interesting in this context.

Markus

10

On Wednesday 23 April 2003 12:16 pm, Markus Neteler wrote:

There was some time ago a discussion on NULLs

http://grass.itc.it/pipermail/grass5/2001-June/000307.html

Maybe interesting in this context.

I like this reply to that question in user list:
http://op.gfz-potsdam.de/GRASS-List/Archive/msg04896.html

Yes, why we need separate file for NULL values? How easy
(code maintenance) and fast (faster PC not needed) would it be
just to have one value defined as NULL.

Radim

11

On Wed, 23 Apr 2003, Radim Blazek wrote:

On Wednesday 23 April 2003 12:16 pm, Markus Neteler wrote:
> There was some time ago a discussion on NULLs
>
> http://grass.itc.it/pipermail/grass5/2001-June/000307.html
>
> Maybe interesting in this context.

I like this reply to that question in user list:
http://op.gfz-potsdam.de/GRASS-List/Archive/msg04896.html

Yes, why we need separate file for NULL values? How easy
(code maintenance) and fast (faster PC not needed) would it be
just to have one value defined as NULL.

We _have_ been here before: legacy GRASS rasters do not know about NULL,
so an additional structure is needed to implement NULL as an extra layer,
so that they can still be used in current GRASS. There are - as we know -
lots of modules, and many have typically not used the current API, so they
would break (this may no longer be correct, but was correct), if we change
the low-level representation. Of course, it would be logical, but it looks
to me like a 5.3 thing, not a 5.1 thing especially since both machines and
disks seem to get faster (but user patience gets shorter?).

Roger

--
Roger Bivand
Economic Geography Section, Department of Economics, Norwegian School of
Economics and Business Administration, Breiviksveien 40, N-5045 Bergen,
Norway. voice: +47 55 95 93 55; fax +47 55 95 93 93
e-mail: Roger.Bivand@nhh.no

12

Radim Blazek wrote:

> Actually, that's not entirely a joke. There is a legitimate argument
> for sacrificing performance in favour of other properties. In this
> case, simplicity. You can almost always get performance improvements
> by replacing generic functions with multiple specific-case versions,
> but doing so creates maintainence problems, and maintenance is GRASS'
> biggest problem.

Can I conclude?: GRASS needs RGB CT but we (you) don't feel to have
enough human sources for GRASS maintenance and prefer not to increase
complexity for now.

Whether or not the utility of such a feature would justify the
additional complexity is open to debate; I don't have a strong opinion
either way. I'm just saying that there are tradeoffs involved; IOW, it
isn't automatically true that anything which increases performance is
good; the other factors also have to be considered.

So back to

> Given that the speed difference between d.rgb and d.photo appears to
> be almost entirely due to the additional time spent reading three maps
> instead of one, it would definitely be worth looking into whether the
> reading speed can be improved. If it could, that would benefit
> everything.

I commented
   stat = embed_nulls_nomask(fd, (void *) cell, row, CELL_TYPE, 0);
in G_get_c_raster_row_nomask() and time fall down to 1.469s from
original 6.930s. So all evil comes from NULLs.

embed_nulls()
  -> G_get_null_value_row()
    -> G_get_null_value_row_nomask()
and G_get_null_value_row_nomask() spends most of the time
(2.161s if commented)

It means that G_get_null_value_row() takes more than 230%
of the time required for G_get_c_raster_row() (if NULLs not read).

Riiiight.

230% seems like a high price for backward compatibility.

It must be possible to improve performance of G_get_null_value_row()
but rasters are not my hobby.

get_row.c really needs a complete re-write, but that code is extremely
critical, and possibly quite fragile.

BTW: It seems that raster values are masked by NULLs in current
region resolution instead of original raster map resolution. I am
not sure if it is good. That is probably more general problem of
conversion of rasters to current region.

Right. My guess is that's a result of the null value support being
"tacked on". The nulls should be embedded before the map is resampled.

--
Glynn Clements <glynn.clements@virgin.net>