Discussion:
[Viking-devel] Four more questions about Viking source code
Kamil Ignacak
2016-05-03 08:48:37 UTC
Permalink
Hello,

While working on a copy of Viking I've found few places in code that I
think may be improved. Can you please take a look?



1.
src/viktrwlayer.c:2193

g_array_index(dp->vtl->track_gc, GdkGC *, 11)

I think that the value '11' is invalid, because in my builds the
following message apears in terminal:
"(viking:3814): Gdk-CRITICAL **: IA__gdk_draw_arc: assertion 'GDK_IS_GC
(gc)' failed"

Since this line is supposed to draw a stop, perhaps
VIK_TRW_LAYER_TRACK_GC_STOP should be used instead.



2.
src/vikviewport.c:1641

vik_coord_load_from_utm ( &test, VIK_VIEWPORT_DRAWMODE_UTM, &u );

Strictly speaking the second argument to the function should be
VIK_COORD_UTM. Both names evaluate to zero, so this works.



3.
src/viktrwlayer.c:3324

GHashTable *vik_trw_layer_get_waypoints_iters ( VikTrwLayer *vtl )
{
return vtl->waypoints;
}

Shouldn't this function return vtl->waypoints_iters?



4.
src/viktrwlayer.c:2178
src/viktrwlayer.c:2365

if ( (!dp->one_zone && !dp->lat_lon) ...

Shouldn't these conditions for drawing a trackpoint and a waypoint be
the same? Currently they are not, second term of each condition is:

((!dp->one_zone) || tp->coord.utm_zone == dp->center->utm_zone)
vs.
( dp->lat_lon || wp->coord.utm_zone == dp->center->utm_zone )

From my understanding (!dp->one_zone) != dp->lat_lon.
Am I mistaken?


Best regards,
Kamil
Robert Norris
2016-05-03 19:11:10 UTC
Permalink
----------------------------------------
Date: Tue, 3 May 2016 10:48:37 +0200
Subject: [Viking-devel] Four more questions about Viking source code
Hello,
While working on a copy of Viking I've found few places in code that I
think may be improved. Can you please take a look?
Yes no problem.

Thanks for taking the time to report these issues.
1.
src/viktrwlayer.c:2193
g_array_index(dp->vtl->track_gc, GdkGC *, 11)
I think that the value '11' is invalid, because in my builds the
"(viking:3814): Gdk-CRITICAL **: IA__gdk_draw_arc: assertion 'GDK_IS_GC
(gc)' failed"
Since this line is supposed to draw a stop, perhaps
VIK_TRW_LAYER_TRACK_GC_STOP should be used instead.
Yes definitely - it should be using VIK_TRW_LAYER_TRACK_GC_STOP.
2.
src/vikviewport.c:1641
vik_coord_load_from_utm ( &test, VIK_VIEWPORT_DRAWMODE_UTM, &u );
Strictly speaking the second argument to the function should be
VIK_COORD_UTM. Both names evaluate to zero, so this works.
Yes it should be VIK_COORD_UTM.
3.
src/viktrwlayer.c:3324
GHashTable *vik_trw_layer_get_waypoints_iters ( VikTrwLayer *vtl )
{
return vtl->waypoints;
}
Shouldn't this function return vtl->waypoints_iters?
Yes it should.
4.
src/viktrwlayer.c:2178
src/viktrwlayer.c:2365
if ( (!dp->one_zone && !dp->lat_lon) ...
Shouldn't these conditions for drawing a trackpoint and a waypoint be
((!dp->one_zone) || tp->coord.utm_zone == dp->center->utm_zone)
vs.
( dp->lat_lon || wp->coord.utm_zone == dp->center->utm_zone )
From my understanding (!dp->one_zone) != dp->lat_lon.
Am I mistaken?
I agree that is makes more sense for the conditions to be same for trackpoints and waypoints.

To be honest I don't know what/why/how the use of this 'One Zone' for UTM helps anything.

These lines haven't changed since the first inception of Viking and these extra conditions only effect drawing in UTM mode.
(Which I assume is not used much).

I suspect the condition for the waypoint drawing test should be using !dp->one_zone.

But in my limited testing it seems to make no difference.

Thus for now I've leave this bit of code as is.
Best regards,
Kamil
Greg Troxel
2016-05-03 23:26:18 UTC
Permalink
Post by Robert Norris
To be honest I don't know what/why/how the use of this 'One Zone' for UTM helps anything.
I agree UTM is used less and less as there are fewer maps in UTM (vs
maps that used to be in UTM reprojected into Web Mercator).

When using UTM, you can get awkwardness when the displayed portion
crosses a zone boundary. It makes sense to decide the zone based on
the center, and then convert all the rest of the screen to/from geodetic
coordinates based on that single zone. This will result in some UTM
coordinates with the wrong zone, basically extended from the next one,
instead of the UTM coordinates you'd get from the geodetic coordinates,
but it should still render ok. And if a UTM-style TMS-ish source has
tiles in that beyond-the-edge region, it should work ok.

I could well be misunderstanding.

Loading...