search

sempr-tk / sempr

SEMPR - Semantic Environment Mapping, Processing and Reasoning
BSD 3-Clause "New" or "Revised" License
7 stars 1 forks source link

Store PointCloud as blob not text #51

Closed ctieben closed 6 years ago

ctieben commented 6 years ago

Currently the PointCloud and his channels are stored as text value in the database. The points as wkt and the channels as list of values separated by a white-space.

It would be more efficient to store both as binary blob (e.g. wkb) in the database.

This changes depends on #44

niniemann commented 6 years ago

Is it even stored correctly? The current pointcloud traits (branch: pointcloud_channel_template) are commented out.

Edit: Oh wait, it's within the MultiPoint, right? In that case all we need to do is change the pragma in the corresponding entities from

#pragma db type("TEXT")

to

#pragma db type("BLOB")
ctieben commented 6 years ago

The traits in the entity-pointcloud branch works.

And yes it is in the MultiPoint. If you change it there than also all MultiPoints (not only) will only be stored as blob. And the channel traits have to be updated

ctieben commented 6 years ago

My idea is that the PointCloud will not inheritance from MultiPoint anymore but it will contain a multipoint geos object like MultiPoint Class that they are the same inheritance level.

With the raw geometry pointer the PointCloud could also add and remove points. (But this is a bit tricky)

@niniemann What do you think about this?

niniemann commented 6 years ago

I don't understand what that has to do with this issue.

We talked about this already, and as I said previously: It's all about the interface. At first I thought just using the MultiPoint class and extending it would be a practical way to implement more types of pointclouds which automatically expose the basic cloud functionality to existing/coming processing modules. By now I think it might be better to allow entities to implement multiple interfaces, through inheritance of abstract classes. This is not implemented yet, but would be the nice way of doing it. And if done so we have implementation and interface decoupled and I won't care anymore how you implement anything, just what it implements. :wink:

That being said, isn't that a discussion for the pointcloud-interface issue?

ctieben commented 6 years ago

Yes and no :-) Its is a implementation detail yes and there is an abstract interface for it. But I dont like the idea to change anything in the MultiPoint class because of a derived PointCould class will have something spacial.

niniemann commented 6 years ago

You mean the change from "TEXT" to "BLOB"? I used "TEXT" just for debugging purposes in the first place, are there any other reasons to keep it that way?

ctieben commented 6 years ago

"TEXT" make sense for debugging and to have a human readable format. Its also common supported by foreign GIS tools (but this is currently no use case) and for smaller geometry it is not so much larger.

niniemann commented 6 years ago

So there is no harm in changing it? :wink:

ctieben commented 6 years ago

No harm currently but it could be painful at some point in the future, e.g. if you try to integrate SEMPR anywhere ;-)

For now I have changed it only in the PointCloud class. Its solved in 2d91b5fc75111d89bef33de79f5bd4caf5754cf4 Also the channels are now stored as blob.

As result the *.db file only needs 775KB instead of 1,35MB to store 10000 points with channels for R, G, B and intensity.

There general point about "TEXT" vs. "BLOB" for geometry should be discussed outside of this issue.

niniemann commented 6 years ago

I'd expect that whatever system can work with our WKT-column in the database can as well work with WKB. So we should be fine. :)