Geometry Boost List: Re: [geometry] Changes in WKB (read) behaviour proposal

Subject: Re: [geometry] Changes in WKB (read) behaviour proposal
From: Mateusz Loskot (mateusz_at_[hidden])
Date: 2013-11-26 06:50:27

On 16 November 2013 20:26, Mats Taraldsvik <mats.taraldsvik_at_[hidden]> wrote:
> On 11/07/2013 11:35 AM, Mateusz Loskot wrote:
>> On 4 November 2013 20:31, Mats Taraldsvik <mats.taraldsvik_at_[hidden]> wrote:
>>>
>>> However, as this might be easy to mix up, I'd vote for the length check I
>>> have implemented.
>>
>> IMHO, we should try to keep the WKB parser conforming to the OGC spec.
>> Later, based on WKB work, we can make a separate parser for EWKB format
>> which would be similar but different.
>>
>> So, also based on what I explained earlier in thsi post,
>> my advise would be to try to make use of the metadata in WKB.
>>
>> But please don't feel like I'm trying to reject your efforts, which
>> n.b. I greatly appreciate. Please, treat my words as technical
>> discussion/advise only.
>> If your current implementation does the job for common cases, let's
>> accept it into Boost.Geometry. Then, we can (I can) make another
>> iteration improving it to rely on WKB metadata and to ensure it's OGC
>> WKB compliant.
>
>
> As stated earlier in my reply, I have removed the proposed checks, to rely
> on static information instead. I guess it will be more efficient, and we
> want an efficient WKB parser :)

Yes, thanks :)

> I have implemented tests and support for MultiLinestring and MultiPolygon as
> well, and updated https://svn.boost.org/trac/boost/ticket/9066 with a patch
> formatted according to your instructions.

Your patch looks very good, all compiles and tests for me.

I have polished a few minor things, added Jamfiles and re-generated fresh
complete patch as complete-extensions-gis-io-wkb-with-multi.patch

I will request others to vote for inclusion into the extensions.

We may need to wait a bit until Boost migration to Git/GitHub is completed,
as the SVN repository is now being closed for commits.

> The next step might be to handle Z, M and ZM geometries.

Yes.

> We need to add the new types in ogc.hpp. The geometry_type_parser needs to
> change. I already removed the multipolygon-check, but the current bitwise
> AND trick does not work -- as far as I can tell -- with pointz (1000) etc.
>
> Also, all the geometry_type-checks in *_parser needs to handle multiple
> types (point, pointz, pointm, pointzm).

Here I'd have a comment about the bitmask used in
geometry_type_parser to filter out non-OGC bits:

boost::uint32_t id = value & 0xff;

(I added TODO comment to the parser.hpp file as well).

As I was indicating earlier, if we want to move deeper to support both OGC WKB
and PostGIS EWKB, I'd vote for splitting the WKB parser into two:
WKB - for pure OGC WKB
EWKB25D - for so called WKB2.5 which is extended version of WKB that was
submitted before OGC added 3D support

The 2.5D variation is quite common indeed, in PostGIS, GDAL/OGR, and many other
software packages, so it's worth to have it.

For pure OGC WKB, we don't need any bits flipping but just simple arithmetic for
combinations of
OGC 18 basic types (codes from 0 to 17) and
coordinate dimensionality variants: Z, M, ZM,
The dimensionality only affects how we parse tuples of coordinates.
It does not affect parsing of structure of geometries.

What do you (and others who are interested) think about the idea of
splitting parsers?

> Perhaps, after this, we add the feature to gracefully handle e.g. a 2d
> linestring read from a 3d linestring WKB.

Sure, good idea.

Best regards,
-- 
Mateusz  Loskot, http://mateusz.loskot.net