[GRASS5] Bug: vector maps shifted (d.area / d.vect)

Hi,

just found an inconsistency: d.area draws maps
shifted to north-west.

I have generated a polygon with r.digit, the vectorized
with r.poly. Comparing d.vect and d.rast the overlay
looks perfect, while d.area produces a shifted result.
Also v.area is not consistent (I guess d.area and v.area
have a similar code history).

Is anyone willing to fix these two problems before releasing
5.0.0?

Thanks in advance,

Markus

(attachments)

darea_dvect_bug.gif

Markus Neteler wrote:

just found an inconsistency: d.area draws maps
shifted to north-west.

I have generated a polygon with r.digit, the vectorized
with r.poly. Comparing d.vect and d.rast the overlay
looks perfect, while d.area produces a shifted result.
Also v.area is not consistent (I guess d.area and v.area
have a similar code history).

d.vect uses G_plot_line() (in src/libes/gis/plot.c), which (in
fastline()) adds 0.5 to each coordinate. d.area doesn't use this
function.

If you remove the +0.5 from fastline(), d.vect and d.area coincide.
However, G_plot_line() is used in many places, so changing it to suit
d.vect may break other things.

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

Glynn Clements wrote:

> just found an inconsistency: d.area draws maps
> shifted to north-west.
>
> I have generated a polygon with r.digit, the vectorized
> with r.poly. Comparing d.vect and d.rast the overlay
> looks perfect, while d.area produces a shifted result.
> Also v.area is not consistent (I guess d.area and v.area
> have a similar code history).

d.vect uses G_plot_line() (in src/libes/gis/plot.c), which (in
fastline()) adds 0.5 to each coordinate. d.area doesn't use this
function.

If you remove the +0.5 from fastline(), d.vect and d.area coincide.
However, G_plot_line() is used in many places, so changing it to suit
d.vect may break other things.

OTOH, d.area uses D_u_to_d_{col,row}. Although these functions return
a double, they truncated the value to an integer (they don't any more;
I've changed them). Then, in every place where these functions are
called, the returned double is typecast to an int.

Now that the full precision is returned, the caller has the choice of
whether to truncate or round.

For drawing lines, I suspect that truncation is the correct operation.
The (integer) pixel coordinate <X,Y> covers the area [X,X+1]*[Y,Y+1].
E.g. the point <1.999,2.999> lies within square corresponding to the
pixel with coordinates <1,2>.

So, d.vect (and, by implication, anything which is consistent with
it), is probably wrong.

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

On Sat, Oct 20, 2001 at 12:44:41PM +0100, Glynn Clements wrote:

Markus Neteler wrote:

> just found an inconsistency: d.area draws maps
> shifted to north-west.
>
> I have generated a polygon with r.digit, the vectorized
> with r.poly. Comparing d.vect and d.rast the overlay
> looks perfect, while d.area produces a shifted result.
> Also v.area is not consistent (I guess d.area and v.area
> have a similar code history).

d.vect uses G_plot_line() (in src/libes/gis/plot.c), which (in
fastline()) adds 0.5 to each coordinate. d.area doesn't use this
function.

If you remove the +0.5 from fastline(), d.vect and d.area coincide.
However, G_plot_line() is used in many places, so changing it to suit
d.vect may break other things.

Thanks for looking into it. But I wouldn't call it "breaking things":
For 5.0 the display needs to be consistent in my opinion, we should
not accept map shifts. I am willing to search-and-fix, but need a
hint what route to follow.

Raster and vector overlays should be 100% o.k.

Markus

On Sun, Oct 21, 2001 at 01:55:33PM +0200, Markus Neteler wrote:

On Sat, Oct 20, 2001 at 12:44:41PM +0100, Glynn Clements wrote:

> If you remove the +0.5 from fastline(), d.vect and d.area coincide.
> However, G_plot_line() is used in many places, so changing it to suit
> d.vect may break other things.

Thanks for looking into it. But I wouldn't call it "breaking things":
For 5.0 the display needs to be consistent in my opinion, we should
not accept map shifts. I am willing to search-and-fix, but need a
hint what route to follow.

Raster and vector overlays should be 100% o.k.

Nobody will say no to this.
On the other hand Glynn's comment is correct:
Changing things in a core function which is used at various places
is not a change you want todo between a pre2 and a final release.
If this inconsistency was there for a long time other code might
work around it in places you cannot easily see. Fixing it in the
core fuction is very likely to break things at other places.
Therefore you need to test it intensively.

Not knowing the details of this case I recommend that adding a
workaround to d.vect for the 5.0.0 release should be considered.
  Bernhard
--
Professional Service around Free Software (intevation.net)
The FreeGIS Project (freegis.org)
Association for a Free Informational Infrastructure (ffii.org)
FSF Europe (fsfeurope.org)

On Sun, Oct 21, 2001 at 03:58:03PM +0200, Bernhard Reiter wrote:

On Sun, Oct 21, 2001 at 01:55:33PM +0200, Markus Neteler wrote:
> On Sat, Oct 20, 2001 at 12:44:41PM +0100, Glynn Clements wrote:

> > If you remove the +0.5 from fastline(), d.vect and d.area coincide.
> > However, G_plot_line() is used in many places, so changing it to suit
> > d.vect may break other things.
>
> Thanks for looking into it. But I wouldn't call it "breaking things":
> For 5.0 the display needs to be consistent in my opinion, we should
> not accept map shifts. I am willing to search-and-fix, but need a
> hint what route to follow.
>
> Raster and vector overlays should be 100% o.k.

Nobody will say no to this.
On the other hand Glynn's comment is correct:
Changing things in a core function which is used at various places
is not a change you want todo between a pre2 and a final release.
If this inconsistency was there for a long time other code might
work around it in places you cannot easily see. Fixing it in the
core fuction is very likely to break things at other places.
Therefore you need to test it intensively.

Not knowing the details of this case I recommend that adding a
workaround to d.vect for the 5.0.0 release should be considered.
   Bernhard

... maybe yes - of course we don't want to break the system now.
But:
- d.vect is right
- d.area, v.area are wrong
according to d.rast (and my eyes).
So we have to careful where to apply the work-around.

Markus

d.vect should be left alone for GRASS5.0 release - it is a core program
that is being used all the time and it does not have a problem.
d.area seems to me of less importance (I have just tried it out, it is nice,
but I must admit that I did not even know that it existed) because
what it does can be accomplished to certain extent by v.to.rast, d.rast -o,
or is there anything else that d.area does that is irreplaceable and crucial
for work with GRASS? I fully agree with Bernhard that you don't want to change

something that affects lots of code at this point, especially not for a
command
that is not being used too often (and that is probably why the bug has not
been caught
long time ago).

Helena

Markus Neteler wrote:

On Sun, Oct 21, 2001 at 03:58:03PM +0200, Bernhard Reiter wrote:
> On Sun, Oct 21, 2001 at 01:55:33PM +0200, Markus Neteler wrote:
> > On Sat, Oct 20, 2001 at 12:44:41PM +0100, Glynn Clements wrote:
>
> > > If you remove the +0.5 from fastline(), d.vect and d.area coincide.
> > > However, G_plot_line() is used in many places, so changing it to suit
> > > d.vect may break other things.
> >
> > Thanks for looking into it. But I wouldn't call it "breaking things":
> > For 5.0 the display needs to be consistent in my opinion, we should
> > not accept map shifts. I am willing to search-and-fix, but need a
> > hint what route to follow.
> >
> > Raster and vector overlays should be 100% o.k.
>
> Nobody will say no to this.
> On the other hand Glynn's comment is correct:
> Changing things in a core function which is used at various places
> is not a change you want todo between a pre2 and a final release.
> If this inconsistency was there for a long time other code might
> work around it in places you cannot easily see. Fixing it in the
> core fuction is very likely to break things at other places.
> Therefore you need to test it intensively.
>
> Not knowing the details of this case I recommend that adding a
> workaround to d.vect for the 5.0.0 release should be considered.
> Bernhard

... maybe yes - of course we don't want to break the system now.
But:
- d.vect is right
- d.area, v.area are wrong
according to d.rast (and my eyes).
So we have to careful where to apply the work-around.

Markus
_______________________________________________
grass5 mailing list
grass5@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass5

Markus Neteler wrote:

... maybe yes - of course we don't want to break the system now.
But:
- d.vect is right
- d.area, v.area are wrong
according to d.rast (and my eyes).
So we have to careful where to apply the work-around.

d.vect uses G_plot_line(), and G_plot_line() is wrong. If it happens
to produce the right result, then the 0.5 pixel South-East shift is
cancelling out an equal-but-opposite error elsewhere.

OTOH, if "right" simply means that it matches something else, the
"something else" may be equally wrong.

The fact that removing the shift causes d.vect to match d.area, along
with d.area not having any obvious flaws, strongly suggests that
d.vect is at fault.

BTW, I personally wouldn't consider using r.poly for this kind of test
(i.e. unless it's r.poly that you're testing). The reverse operation
(rasterising) is much more straightforward than feature extraction.

Also, note that "right" is a subjective term when applied to
rasterising geometry. E.g. when filling a polygon, what is the
criterion for setting a pixel to the fill colour? Is it:

a) any part of the pixel lies within the polygon, or
b) all of the pixel lies within the polygon, or
c) the centre of the pixel lies within the polygon, or
d) at least X% of the pixel lies within the polygon, or
e) some other criterion?

Depending upon what you are trying to achieve, any of the above may be
the correct answer.

Also, even when you know what you want, you have to take into account
what you can realistically have. Many primitive graphics APIs
(including X and GD) only allow integer coordinates, so you typically
end up having to modify the geometry (unless you plot individual
pixels).

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