PC_Envelope : testing, degenerate cases, PC_Envelope_AsBinary

[mbredif]

Features

• Degenerate envelopes are now constructed as points or linestrings instead of relying on ad-hoc perturbations

Issues

• memcpy size seems erroneous for u_int32_t : 8 -> 4 #96 : wrongly sized memset is now corrected. The envelope construction code is moved to the core library as a new pc_bounds_to_wkb function so that it may be tested, leaving only the serialization relevant code in the pgsql directory. A test has been added (valgrind is not able to catch the wrongly sized memset issue though).
• Confusing pcpatch::geometry cast #30 is partially addressed as the geometry cast is kept for backward compatibility but the following functions are renamed
  • PC_Envelope(pcpatch) : wkb -> PC_Envelope_AsBinary(pcpatch) : wkb
  • geometry(pcpatch) : geometry -> PC_Envelope(pcpatch) : geometry
• pc_envelope(patch) not considering Z values #86 is not addressed by this PR since it relies on PCBOUNDS which only features x/y values.

[mbredif]

@strk, @pramsey, how do you feel about this PR ?

[pramsey]

I feel OK about the implementation. I worry that the concept might cause people problems. The nice thing about "ad hoc perturbations" on polygons is that the result of running PC_Envelope() on a set of patches is a nice uniform set of polygons.

So for GIS applications, which like uniform types, it's easy to write a query that creates a layer that is immediately usable in a visualization tool. I'm not sure I like breaking that concept, even in order to provide more precise results.

My assumption was that people would be using these envelopes mostly for visualization or queries where perfect precision was not required. What use case did you have where that assumption was wrong?

[mbredif]

It is a matter of taste, but I feel that getting the precise result and coercing it to a polygon if need be is much more explicit. It is also more aligned with http://postgis.net/docs/ST_Envelope.html. Is that a bad design choice in PostGIS?

As a side note, the perturbation in https://github.com/mbredif/pointcloud/blob/4aaa8ca0d7cb2569c3bd064a330def3a1dedfce8/pgsql/pc_pgsql.c#L950-L951 was not taken into account as the generated geometry relies on pa->bounds rather than xmax and ymax, so degenerate cases generated degenerate polygons. (Moreover, there might be issues with negative or 0 values of xmax and ymax)

[mbredif]

Closing in favor of upcoming smaller and more focused PRs.