[GRASS-dev] Principle G3D library question

Dear developer,
i have a principal question regarding the coordinate system in the g3d library.

I currently review the g3d code and related modules to get a clear
picture of the order of the underlying coordinate system. It looks
like different approaches have been developed in the modules and the
g3d library. This is very confusing. I will try to harmonize this, so
here is my question:

What kind of coordinate system should be used in the g3d library?

From my point of view: the original authors implemented the idea of a

cube coordinate system starting in the lower left (SW) corner. This is
the common cube coordinate system used in a wide range of software.
BUT this coordinate system uses a different row order than the raster
coordinate system which start in the upper left corner. Later
development of the g3d library tried to establish the upper left
corner as origin, but this messed up the code base and result in
confusion about the correct coordinate transformation and row
ordering.

I currently try to reestablish the lower left (SW) corner approach
using the common x, y, z cube coordinate system. The code works fine
and many tests have been implemented to assure correct behavior. BUT
the result is that volume maps created with several modules before
these changes have now flipped rows. AND the g3d cube coordinate
system is not row compatible with the raster coordinate system. For
example:

g.region b=0 t=1 s=0 w=0 e=120 n=80 res=10 res3=10 tbres=1

r.mapcalc --o expr="raster_map = row()"
r3.mapcalc --o expr="volume_map = row()"

This code will result in raster and volume maps with different row
counting. The raster lib counts rows from north to south but the g3d
lib counts from south to north.

#: r.out.ascii raster_map
north: 80
south: 0
east: 120
west: 0
rows: 8
cols: 12
1 1 1 1 1 1 1 1 1 1 1 1
2 2 2 2 2 2 2 2 2 2 2 2
3 3 3 3 3 3 3 3 3 3 3 3
4 4 4 4 4 4 4 4 4 4 4 4
5 5 5 5 5 5 5 5 5 5 5 5
6 6 6 6 6 6 6 6 6 6 6 6
7 7 7 7 7 7 7 7 7 7 7 7
8 8 8 8 8 8 8 8 8 8 8 8

#: r3.out.ascii volume_map dp=0
version: grass7
order: nsbt
north: 80.000000
south: 0.000000
east: 120.000000
west: 0.000000
top: 1.000000
bottom: 0.000000
rows: 8
cols: 12
levels: 1
8 8 8 8 8 8 8 8 8 8 8 8
7 7 7 7 7 7 7 7 7 7 7 7
6 6 6 6 6 6 6 6 6 6 6 6
5 5 5 5 5 5 5 5 5 5 5 5
4 4 4 4 4 4 4 4 4 4 4 4
3 3 3 3 3 3 3 3 3 3 3 3
2 2 2 2 2 2 2 2 2 2 2 2
1 1 1 1 1 1 1 1 1 1 1 1

Booth modules export the data in the raster coordinate system printing
the rows from north to south.

What i don't know:
Is the row ordering irrelevant for g3d internal value storage, tile
storage and tile cache coordinates? So only coordinate transformation
functions should take care of correct counting? So all g3d modules can
use the raster coordinate system using x, y, and z coordinates and
interact with raster modules in the same coordinate system: "row ==
y":

         Top (z)
            |
            | North
West | _ _ _ _ _ _ East (x)
           / Bottom
         /
       /
  South (y)

Or does the row ordering affect the internal g3d storage and caching
too and should always count from south to north??? So any module which
interacts with raster rows need to translate the y coordinate into the
correct raster row: "row = rows - y - 1"?

I prefer the last solution (which i currently implement) to avoid
potential conflicts with the (by myself not fully understood) storage
and tile caching in the g3d library and to have a well understood and
consistent behavior:

   Top (z) North (y)
          | /
          | /
          | /
West | / _ _ _ _ _ _ East (x)
    Bottom South

Maybe i am to blind to see the obvious?

Any hints or suggestions are very welcome.

Best regards
Soeren

Soeren Gebbert wrote:

What kind of coordinate system should be used in the g3d library?

Make it match the 2d raster library. Row zero should be north; in the
library, in the modules, in the file format, and in terms of
externally-visible behaviour. E.g. this:

r3.mapcalc --o expr="volume_map = row()"

#: r3.out.ascii volume_map dp=0

8 8 8 8 8 8 8 8 8 8 8 8
7 7 7 7 7 7 7 7 7 7 7 7
6 6 6 6 6 6 6 6 6 6 6 6

shouldn't happen.

Modules should scan from north to south, i.e. increasing row numbers.

Don't worry too much about compatibility; for 7.0, sanity comes first.

However: if the existing file format has row zero at the southern
edge, then we should to be able to distinguish the new format from the
old, and the library should be able to (transparently) read either (it
doesn't necessarily need to be able to write the old format).

--
Glynn Clements <glynn@gclements.plus.com>

Hello Glynn,

2011/6/17 Glynn Clements <glynn@gclements.plus.com>:

Soeren Gebbert wrote:

What kind of coordinate system should be used in the g3d library?

Make it match the 2d raster library. Row zero should be north; in the
library, in the modules, in the file format, and in terms of
externally-visible behaviour. E.g. this:

Well you just destroyed my dreams to finish the g3d library and
related modules next week. :slight_smile:
I will try to implement the addressing to match 2d raster library ...
many test have to be rewritten and validated.

Don't worry too much about compatibility; for 7.0, sanity comes first.

Ok.

However: if the existing file format has row zero at the southern
edge, then we should to be able to distinguish the new format from the
old, and the library should be able to (transparently) read either (it
doesn't necessarily need to be able to write the old format).

The problem is that i currently not completely understand the internal
tile caching algorithm. I will need more time to understand it in more
detail.

Best regards
Soeren

--
Glynn Clements <glynn@gclements.plus.com>

[r3.in.ascii]

readHeaderString()
   while (fgetc(fp) != '\n');

probably it should use G_getl2(), which also accepts DOS and MacOS9 newlines?

version:
order:

once formally decided, we should modify the 6.x versions to deal with the
extra header instructions, and continue on (with a message?) if it matches
grass6's format, otherwise fatal_error out with a reasonable error message.

ASCII is an acronym of a named entity, and should remain uppercased.

Note that unit tests for r3.in.ascii are implemented in the
<em>test.r3.out.ascii.sh</em> script located in the r3.out.ascii directory.

if many/most modules will eventually get test scripts, is it worth
mentioning that in the help page? maybe <!-- commented out --> so only
developers see it?

I think is it enough to describe in the grass7 help page how
to make a grass6-compatible G3D output file, probably with an
example. A flag for that could be overkill.

btw, I believe the "AV" responsible for parts of the G3D code is Alfonso Vitti.

Hamish

Hello Hamish,
many thanks for code reviewing.

2011/6/18 Hamish <hamish_b@yahoo.com>:

[r3.in.ascii]

readHeaderString()
while (fgetc(fp) != '\n');

probably it should use G_getl2(), which also accepts DOS and MacOS9 newlines?

Done in svn.

version:
order:

once formally decided, we should modify the 6.x versions to deal with the
extra header instructions, and continue on (with a message?) if it matches
grass6's format, otherwise fatal_error out with a reasonable error message.

I don't think it is necessary to backport the header changes to 6.x.
The grass6 version works fine, maybe a manual update is meaningful?
For backward compatibility a flag was added to r3.out.ascii which is
IMHO not overkill. :slight_smile:

ASCII is an acronym of a named entity, and should remain uppercased.

Fixed in svn.

Note that unit tests for r3.in.ascii are implemented in the
<em>test.r3.out.ascii.sh</em> script located in the r3.out.ascii directory.

if many/most modules will eventually get test scripts, is it worth
mentioning that in the help page? maybe <!-- commented out --> so only
developers see it?

Good test scripts provide valuable information about the module
capabilities, data handling and possible bugs. Test scripts should be
designed so that grass user can easily read and write them ... at
least this is the goal we have. My hope is that user will write test
scripts in case they found a bug or in case test scripts are still
missing.
Therefor in my humble opinion the test scripts should be added
automatically to the end of the modules HTML manual pages. In case
they are missing in the module directory (r3.in.ascii) the manual
should explain why.

I think is it enough to describe in the grass7 help page how
to make a grass6-compatible G3D output file, probably with an
example. A flag for that could be overkill.

btw, I believe the "AV" responsible for parts of the G3D code is Alfonso Vitti.

Thanks, i was now able to find the g3d related mails in the grass5
developer mail archive. Very valuable information.

Best regards
Soeren

Hello,
It's not an backporting of header it's about GRASS6 being able to
distinguish GRASS7 format g3d files. I have not peaked into code if
there such feature exists. If not, it should be implemented to prevent
GRASS6 failures (or worse - data corruption) on GRASS7 files.

Maris.

2011/6/18 Soeren Gebbert <soerengebbert@googlemail.com>:

I don't think it is necessary to backport the header changes to 6.x.
The grass6 version works fine, maybe a manual update is meaningful?
For backward compatibility a flag was added to r3.out.ascii which is
IMHO not overkill. :slight_smile:

Hi Maris,

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

Hello,
It's not an backporting of header it's about GRASS6 being able to
distinguish GRASS7 format g3d files. I have not peaked into code if
there such feature exists. If not, it should be implemented to prevent
GRASS6 failures (or worse - data corruption) on GRASS7 files.

I see, good point!

The grass6 implementation checks for valid header:

static void readHeaderString(FILE * fp, char *valueString, double *value)
{
    static char format[100];

    /* to avoid buffer overflows we use snprintf */
    G_snprintf(format, 100, "%s %%lf", valueString);
    if (fscanf(fp, format, value) != 1) {
        G_debug(0, "bad value for [%s]", valueString);
        fatalError("readHeaderString: header value invalid");
    }
    while (fgetc(fp) != '\n') ;
}

I think this should prevent the import of new grass7 volume ascii
files in grass6. The grass6 import will break in when the new lines of
the grass7 header are parsed.

Best regards
Soeren

(Still haven't looked at code) - I hope GRASS7 files will have a
version number in header to be future-proof.

Maris.

2011/6/21 Soeren Gebbert <soerengebbert@googlemail.com>:

Hi Maris,

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

Hello,
It's not an backporting of header it's about GRASS6 being able to
distinguish GRASS7 format g3d files. I have not peaked into code if
there such feature exists. If not, it should be implemented to prevent
GRASS6 failures (or worse - data corruption) on GRASS7 files.

I see, good point!

The grass6 implementation checks for valid header:

static void readHeaderString(FILE * fp, char *valueString, double *value)
{
static char format[100];

/* to avoid buffer overflows we use snprintf */
G_snprintf(format, 100, "%s %%lf", valueString);
if (fscanf(fp, format, value) != 1) {
G_debug(0, "bad value for [%s]", valueString);
fatalError("readHeaderString: header value invalid");
}
while (fgetc(fp) != '\n') ;
}

I think this should prevent the import of new grass7 volume ascii
files in grass6. The grass6 import will break in when the new lines of
the grass7 header are parsed.

Best regards
Soeren

Hi Maris,
the new rater3d ascii header looks like this now:

version: grass7
order: nsbt
north: 80.000000
south: 0.000000
east: 120.000000
west: 0.000000
top: 50.000000
bottom: 0.000000
rows: 8
cols: 12
levels: 5

The options "version" and "order" are introduced with my changes on
the g3d library in grass7. The manual page of the svn version of
r3.out.ascii in grass7 explains a bit more.

Best regards
Soeren

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

(Still haven't looked at code) - I hope GRASS7 files will have a
version number in header to be future-proof.

Maris.

2011/6/21 Soeren Gebbert <soerengebbert@googlemail.com>:

Hi Maris,

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

Hello,
It's not an backporting of header it's about GRASS6 being able to
distinguish GRASS7 format g3d files. I have not peaked into code if
there such feature exists. If not, it should be implemented to prevent
GRASS6 failures (or worse - data corruption) on GRASS7 files.

I see, good point!

The grass6 implementation checks for valid header:

static void readHeaderString(FILE * fp, char *valueString, double *value)
{
static char format[100];

/* to avoid buffer overflows we use snprintf */
G_snprintf(format, 100, "%s %%lf", valueString);
if (fscanf(fp, format, value) != 1) {
G_debug(0, "bad value for [%s]", valueString);
fatalError("readHeaderString: header value invalid");
}
while (fgetc(fp) != '\n') ;
}

I think this should prevent the import of new grass7 volume ascii
files in grass6. The grass6 import will break in when the new lines of
the grass7 header are parsed.

Best regards
Soeren

I would suggest to change grass7 to something not GRASS release dependent.

just my 0.02,
Maris.

2011/6/21 Soeren Gebbert <soerengebbert@googlemail.com>:

Hi Maris,
the new rater3d ascii header looks like this now:

version: grass7
order: nsbt
north: 80.000000
south: 0.000000
east: 120.000000
west: 0.000000
top: 50.000000
bottom: 0.000000
rows: 8
cols: 12
levels: 5

The options "version" and "order" are introduced with my changes on
the g3d library in grass7. The manual page of the svn version of
r3.out.ascii in grass7 explains a bit more.

Best regards
Soeren

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

(Still haven't looked at code) - I hope GRASS7 files will have a
version number in header to be future-proof.

Maris.

2011/6/21 Soeren Gebbert <soerengebbert@googlemail.com>:

Hi Maris,

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

Hello,
It's not an backporting of header it's about GRASS6 being able to
distinguish GRASS7 format g3d files. I have not peaked into code if
there such feature exists. If not, it should be implemented to prevent
GRASS6 failures (or worse - data corruption) on GRASS7 files.

I see, good point!

The grass6 implementation checks for valid header:

static void readHeaderString(FILE * fp, char *valueString, double *value)
{
static char format[100];

/* to avoid buffer overflows we use snprintf */
G_snprintf(format, 100, "%s %%lf", valueString);
if (fscanf(fp, format, value) != 1) {
G_debug(0, "bad value for [%s]", valueString);
fatalError("readHeaderString: header value invalid");
}
while (fgetc(fp) != '\n') ;
}

I think this should prevent the import of new grass7 volume ascii
files in grass6. The grass6 import will break in when the new lines of
the grass7 header are parsed.

Best regards
Soeren

On Tue, Jun 21, 2011 at 1:58 PM, Maris Nartiss <maris.gis@gmail.com> wrote:

I would suggest to change grass7 to something not GRASS release dependent.

See [0] and following lines for an example of file format versioning
independent of the release

[0] https://trac.osgeo.org/grass/browser/grass/trunk/include/vect/dig_defines.h#L151

Silently following the thread,

Markus M

just my 0.02,
Maris.

2011/6/21 Soeren Gebbert <soerengebbert@googlemail.com>:

Hi Maris,
the new rater3d ascii header looks like this now:

version: grass7
order: nsbt
north: 80.000000
south: 0.000000
east: 120.000000
west: 0.000000
top: 50.000000
bottom: 0.000000
rows: 8
cols: 12
levels: 5

The options "version" and "order" are introduced with my changes on
the g3d library in grass7. The manual page of the svn version of
r3.out.ascii in grass7 explains a bit more.

Best regards
Soeren

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

(Still haven't looked at code) - I hope GRASS7 files will have a
version number in header to be future-proof.

Maris.

2011/6/21 Soeren Gebbert <soerengebbert@googlemail.com>:

Hi Maris,

2011/6/21 Maris Nartiss <maris.gis@gmail.com>:

Hello,
It's not an backporting of header it's about GRASS6 being able to
distinguish GRASS7 format g3d files. I have not peaked into code if
there such feature exists. If not, it should be implemented to prevent
GRASS6 failures (or worse - data corruption) on GRASS7 files.

I see, good point!

The grass6 implementation checks for valid header:

static void readHeaderString(FILE * fp, char *valueString, double *value)
{
static char format[100];

/* to avoid buffer overflows we use snprintf */
G_snprintf(format, 100, "%s %%lf", valueString);
if (fscanf(fp, format, value) != 1) {
G_debug(0, "bad value for [%s]", valueString);
fatalError("readHeaderString: header value invalid");
}
while (fgetc(fp) != '\n') ;
}

I think this should prevent the import of new grass7 volume ascii
files in grass6. The grass6 import will break in when the new lines of
the grass7 header are parsed.

Best regards
Soeren

_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev