Geometry Boost List: Re: [geometry] io - WKB

Subject: Re: [geometry] io - WKB
From: Mats Taraldsvik (mats.taraldsvik_at_[hidden])
Date: 2013-08-27 13:52:15

Hi Mateusz,

On 07/29/2013 11:44 AM, Mateusz Loskot wrote:
> On 28 July 2013 11:45, Mats Taraldsvik <mats.taraldsvik_at_[hidden]> wrote:
>> On 07/18/2013 11:51 PM, Mateusz Loskot wrote:
>>> On 18 July 2013 20:04, Mats Taraldsvik <mats.taraldsvik_at_[hidden]> wrote:
>> Thanks for the explanations.
>>
>> - Can I use the boostorg geometry repository on github, or is there another
>> (preferably git) source I should use?
> AFAIU, currently, you can use it as read-only repository, but as the development
> has not yet moved to GitHub or any other Git repository, thus
> you can't submit pull requests or commits there.
>
> The only place where of Boost development is SVN repository
> and Trac instance where contributors can open tickets and submit patches.
>
> If you have any questions about Boost migration to Git, please ask on the
> Boost development list at http://lists.boost.org/mailman/listinfo.cgi/boost
> I doubt anyone here is able to provide you with accurate information
> on the Git/GitHub migration status.
>
>> - Do you have any guidelines on where I should start?
> In order to add multi-geometries support to io/wkb?
> I would follow directory/file structure of wkt, then implement required
> bits (specialisations of parsers, read_wkb, etc.).
>
>> - Should I begin with implementing the multi-geometries as-is, or should the
>> implementation be changed to use streams first? Which stream type?
>> (As far as I can tell, for WKT, read_wkt use a std::string, while write_wkt
>> use basic_ostream)
> IMHO, general rule should be this:
> read_{format} accepts std::basic_istream
> write_{format} accepts std::basic_ostream
>
> On the other hand, I quite like the iterator-based interface of Boost.Spirit
> and I think it would be easier to flip the idea over:
> - provide I/O interfaces based on iterators
> - allow users to pass streams through iterators
> instead of passing complete stream objects.
>
> Iterators are good for generalisation of both containers and streams.
>
> Bruno, what do you think?
>
>
> Mats, in the meantime, I'd suggest you proceed based on the current
> version of interface of I/O for WKB. It is, make your geometries work with:
>
> <unspecified> input(...);
> G geometry;
> read_wkb(input.begin(), input.end(), geometry);
>
> <unspecified byte type>* input = ...;
> std::size_t length = ...;
> read_wkb(input, length, geometry);

Sorry for the late response. I finally had time to look at this, and 
have implemented a working MultiPoint-parser. Currently, the 
implementation modifies 
include/boost/geometry/extensions/gis/io/wkb/parser.hpp and read_wkb.hpp 
(and also an equals implementation for multipoints in 
include/boost/geometry/multi/algorithms/equals.hpp).

Do you want me to move the multipoint implementations to a 
multi-directory, as in the rest of boost geometry? 
include/boost/geometry/extensions/multi/gis/io/wkb/parser.hpp and 
read_wkb.hpp, perhaps?

I could only find tests for Point in 
extensions/test/gis/io/wkb/read_wkb.cpp, should I add some tests for 
LineString, Polygon.. as well?

Should I move my tests for MultiPoint from 
extensions/test/gis/io/wkb/read_wkb.cpp to another multi-folder as well? 
extensions/test/multi/gis/io/wkb/read_wkb.cpp?

>> - I appreciate that in the WKB spec, an enum is used for geometry_type, but
>> doesn't this make it hard to extend (for a user)?
> The WKB is a well-specified format and there is no need for extending it.
>
>> An example is PostGIS' extended WKB type, which could be easier to implement
>> on top of the wkb implementation if e.g. a static inline const unsigned int
>> was used instead, or something with is specializable/overloadable.
> EWKB is a different format, and IMO it should not be seen as a
> specialisation of WKB.
> If OGC changes WKB format and those changes are in conflict with
> requirements of PostGIS
> needs, then PostGIS developers reserve rights to further disconnect
> WKB and EWKB.
>
> Having said that, I would be careful with making EWKB parser "OOP-like
> derived" from WKB parser.
> Instead, I'd suggest to extract some common elements (if possible) and
> re-use them to compose
> final *separate* parsers. This would allow fine-tuned optimisations as well.
>
> For example, both parsers can re-use definitions of
> value_parser, byte_order_parser, geometry_type_parser, etc.
> But, I wouldn't worry about code re-use, the parsers are not large
> enough to over-engineer their design.
>
>> - A write_wkb method is, as far as I can tell, also missing.
> Yes, WKB is read-only parsers. The write support is missing.
>
> Best regards,

Regards,
Mats