Opened 7 years ago

Closed 7 years ago

#3244 closed task (fixed)

Vect_get_map_box() and level1

Reported by: martinl Owned by: grass-dev@…
Priority: normal Milestone: 7.4.0
Component: Vector Version: unspecified
Keywords: vlib, map bbox, level1 Cc:
CPU: Unspecified Platform: Unspecified

Description

Recently I discovered that g.region vector does not work with vectors with no topology (level 1). I fixed this issue similarly as v.info do (level1.c) in r70302. Then I found similar issue when trying to colorize points based on z-coord (d.vect zcolor=elevation). It uses Vect_map_get_box() source:grass/trunk/display/d.vect/shape.c#L180 which requires topology (1).

My suggestion would be to modify Vect_get_map_box() to work also on level 1 (fill bbox by reading features). I am not sure about consequences, any opinion?

(1) https://grass.osgeo.org/programming7/vector_2Vlib_2box_8c.html#a418ba3ac55762f3b36c183d2d5113d4d

Attachments (2)

get_map_box.diff (3.5 KB ) - added by martinl 7 years ago.
get_map_box_1.diff (2.6 KB ) - added by martinl 7 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 by martinl, 7 years ago

Patch attached attachment:get_map_box.diff. Unfortunately it requires change in function definition (const struct Map_info * -> struct Map_into *) since Vect_rewind() is called.

Last edited 7 years ago by martinl (previous) (diff)

by martinl, 7 years ago

Attachment: get_map_box.diff added

comment:2 by martinl, 7 years ago

See also discussion in related ticket #3249

in reply to:  1 ; comment:3 by martinl, 7 years ago

Replying to martinl:

Patch attached attachment:get_map_box.diff. Unfortunately it requires change in function definition (const struct Map_info * -> struct Map_into *) since Vect_rewind() is called.

Touching plus_head structure on level1 is not good idea since this structure is designed for topological level 2. I modified patch by introducing Vect_get_map_box1() which works on level1. See attachment:get_map_box_1.diff. On level 1 Vect_get_map_box() returns error code. Example of usage (which fixes d.vect zcolor=elevation for maps open on level 1):

--- display/d.vect/shape.c	(revision 70370)
+++ display/d.vect/shape.c	(working copy)
@@ -188,7 +188,10 @@
 	    z_style = NULL;
         }
 	else {
-	    Vect_get_map_box(Map, &box);
+            if (Vect_level(Map) > 1)
+                Vect_get_map_box(Map, &box);
+            else
+                Vect_get_map_box1(Map, &box);
 	    Rast_make_fp_colors(&zcolors, z_style, box.B, box.T);
 	}
     }

This approach could be used by other modules which works on level 1 like g.region with vector option. Anyway the map features will be read mostly twice(*) (by module itself and by Vect_get_map_box1()), so probably it make better sense to modify each module which works on level1 and requires map bbox to fill bound_box structure locally when reading features by the module.

(*) Not the case of g.region, so at least for this module Vect_get_map_box1() make sense.

Last edited 7 years ago by martinl (previous) (diff)

by martinl, 7 years ago

Attachment: get_map_box_1.diff added

in reply to:  3 comment:4 by martinl, 7 years ago

Replying to martinl:

Touching plus_head structure on level1 is not good idea since this structure is designed for topological level 2. I modified patch by introducing Vect_get_map_box1() which works on level1. See attachment:get_map_box_1.diff. On level 1 Vect_get_map_box() returns error code.

Implemented in r70521.

in reply to:  3 comment:5 by martinl, 7 years ago

Replying to martinl:

(*) Not the case of g.region, so at least for this module Vect_get_map_box1() make sense.

g.region updated in r70522

comment:6 by martinl, 7 years ago

Milestone: 7.2.17.4.0
Resolution: fixed
Status: newclosed

d.vect updated accordingly in r70524. Since API changes no backport is planned. Changing milestone and closing this ticket as solved.

Note: See TracTickets for help on using tickets.