XYZM

[mbredif]

This PR is based on the PR pgpointcloud#104 and further addresses the issues pgpointcloud#86 and #32:

• it extends PCBOUNDS with zmin/zmax and mmin/mmax double attributes (ie 32 extra header bytes for pcpatches) for optimized postgis conversions
• Z and M values are effciently exported to wkb points and envelopes when present
• Z and M dimensions are looked-up using "Z/H/Height" and "M/T/Time" names. These possible xyzm dimension names are hard coded (similarly to XY dimensions).

@pblottiere, could this improve the performance of lopocs, as I see you have to treat separately xy andz dimensions in https://github.com/LI3DS/lopocs/blob/master/lopocs/database.py#L46-L82 ?
Is that compensating for the pcpatch header size increase ?

[pblottiere]

@mbredif

could this improve the performance of lopocs, as I see you have to treat separately xy andz dimensions in https://github.com/LI3DS/lopocs/blob/master/lopocs/database.py#L46-L82 ?
Is that compensating for the pcpatch header size increase ?

Indeed, I think it will improve performance! This filtering step was clearly slowing down LOPoCS. Thank you!

[elemoine]

@mbredif I haven't looked at your changes yet, but shouldn't this target the dev branch instead of master? master is just a copy of pgpointcloud/pointcloud:master right now…

[mbredif]

@elemoine : I will rebase a xyzm_li3ds branch for internal testing/reviewing and keep xyzm targeting upstream.
@pblottiere , I am looking forward benchmarking results on the li3ds/lopocs use case !
@pblottiere @vpicavet , is the header change a big issue? (Eg does it require db updating routines?)

[elemoine]

@elemoine : I will rebase a xyzm_li3ds branch for internal testing/reviewing and keep xyzm targeting upstream.

Could you please provide more explanations? LI3DS/pointcloud:master is just a clone of pgpointcloud/pointcloud:master right now. Do you plan to change that?

Up to now I've been thinking around the following lines:

• our goal is to merge code upstream (pgpointcloud/pointcloud:master) as much as possile
• quick patches/bug fixes go to pgpointcloud/pointcloud:master directly
• features go into pgpointcloud/pointcloud:master if possible, they can also go into LI3DS/pointcloud:dev as a staging area, for us to make progress with the project
• we regularly sync LI3DS/pointcloud:master from pgpointcloud:pointcloud:master
• we also regularly update LI3DS/pointcloud:dev from pgpointcloud:pointcloud:master

Do you agree with the above? Am I missing something?

[mbredif]

I agree with these guidelines. I just wanted to keep my xyzm branch free from LI3DS stuff to ease the reviewing from people outside the LI3DS project.

[elemoine]

I agree with all the changes of this patch. I think we should create separate, single-purpose, PRs in upstream. For example, the handling of Z/H/Height and M/T/Time should be proposed with one PR, the extension of PCBOUNDS with another one, etc.

[mbredif]

Ok, looking at the code quickly, I can identify the following break up :

1. caching of z/m_position in schemas, looking up for Z/H/Height and M/T/Time dimension names, pc_point_get/set_z/m, pc_schema_check_xyzm, pc_point_to_geometry_wkb
2. Box3D(pcpatch), PCBOX3D, pc_bounds_to_box3d, pcpatch_box3d
3. PCBOUNDS with zmin/zmax and mmin/mmax, expected output PC_MemSize change -> breaking serialization change
4. pgsql/pc_pgsql.c#pc_patch_to_geometry_wkb_envelope moved to lib/pc_util.c#pc_bounds_to_geometry_wkb (should be pc_bounds_to_geometry_wkb_envelope), memcpy size seems erroneous for u_int32_t : 8 -> 4 pgpointcloud/pointcloud#96 fix, expected output geometry change to handle degenerate cases (point/linestring/polygon), test in cu_pc_patch.c
5. pointcloud_postgis configure files (version was hardcoded to 1.0)
6. PC_Envelope(p pcpatch) renamed to PC_Envelope_AsBinary(p pcpatch), such that PC_Envelope(pcpatch) returns geometry () -> breaking API change
7. ( indentation changes )

Is that the level of separation you were thinking of ?

[elemoine]

Is that the level of separation you were thinking of ?

Yep, sounds good to me. We need this level of splitting to ease reviews.

[mbredif]

Updated and Rebased in #59