Hi,

> I'm not saying that you have to do everything at once, can you answer 
> the simple question above though?
> Restating it: "How sure you are that you're going to implement proper 
> encoding of 3D bboxes at the very least for JDBC stores (and possibly 
> for shapefiles as well?)"

I am sorry, I cannot give you a guarantee about that at this moment.

> I have looked at the patch and I don't see where in the code you are 
> handling the parsing of a BBOX filter
> coming from a OGC filter in a POST request, or a BBOX coming from a 
> CQL filter.
> If you are not planning to support them please state so, if you are 
> planning to do please clarify how
> and when

The current patch and proposal does not include these features. However, 
I'd understand that WFS post requests need to at least support the same 
features as wfs get requests. Please let me know the minimum 
requirements with regards to this to get the patch approved.

> The proposal is effectively adding a new type of filter, normally that 
> would require a change
> in the filter visitor API to handle such new filter explicitly, 
> however this case is peculiar
> in that the new filter extends an existing one.
>
> This means the API might not be in need of change, but it has 
> consequences on the API.
> In particular, all filter visitors dedicated to the manipulation of 
> filters (including the
> simplifiying filter visitor) will create a copy that's a plain 2D 
> BBOX, and the CQL encoder
> will encode the bbox as a 2D one.
>
> I see there is in the code:
> ...
>
> I'm sure this works with most/all of the existing code, but is this 
> the right way to go?
> Given that most filters manipulating filters are extending from the 
> duplicating filter visitor,
> wouldn't it be better to fix that one instead?
> Also, I believe the ReprojectingFilterVisitor won't work properly with 
> the above hack,
> and that it might need some logic changes (a way consistent with 
> geometry reprojection
> would be to reproject the 2d component of the bbox and leave the 3d 
> part move
> unaltered to the result)
>
> Also, given this is an API change there should be some documentation on a
> "upgrade to 9.x" page telling people to expect this new kind of filter 
> in their custom
> visitors, and probably some changes on the FilterVisitor javadocs are 
> in order as well.
>
> This is one problematic area of the proposal, it would have been 
> better to point
> it out in the proposal document instead of having people have to 
> review the code
> in order to see it: a good proposal is also showing what is _not_ 
> covered/done and
> what the potentially problematic areas are (and how the problems have 
> been addressed).

Yes, you have a point here, this is a weak part. I stole the code from 
your fastbbox, but it is indeed an ugly hack. I would be happy to change 
the filter visitors instead, but I was trying to minimise changes to the 
core.
I am sorry I didn't point this out in the proposal, I will do something 
about it.

> This went a bit off the tangent, but here is the part that affects the 
> filtering code.
> Say I have a layer in postgis that is declared as EPSG:4326 with 
> dimension=3
> (perfectly legit in postgis), how am I going to filter it using BBOX3D?
> It seems whatever I do I might be in some issue from some point of view:
> * if I state the crs is EPSG:4326 then the dimensionality check in
> ReferencedEnvelope3D.checkCoordinateReferenceSystemDimension()
>   will make it impossible to build the filter
> * as a user I could know that I have to use EPSG:4327 instead, but this
>   will make the CRS inconsistent with the one on the DBMS.. besides that,
>   I don't see an immediate way for automated code to build the "3d 
> equivalent"
>   of a 2d system, and the layer will be declared in the feature type 
> as having
>   EPSG:4326 (since that's what we reflect out of the database)

The method checkCoordinateReferenceSystemDimension is identical to the 
one in its superclass, ReferencedEnvelope. (In fact, I should remove 
this copy-pasted method and instead make it protected instead of private 
in its superclass, this is left-over from changing the class structure).
So the existing implementation already checks the srs's dimensionality 
against this. This means that is now forbidden to apply a 2D bbox 
against a 3D SRS. It only seemed logical for me to extend the same logic 
to situation the other way around.
I would personally recommend to use a 3D SRS if you have 3D geometries, 
even in postgis, this seems to me like the best way to work. If this is 
not desired behaviour, I think it is not just my changes, but the 
existing code of the ReferencedEnvelope class that must be reviewed.
Perhaps I could change the check to test if the dimension of SRS < than 
the dimension of the bbox? Does that make any sense at all?

> I find it suprising that 3D reprojection works indeed. Afaik to do it 
> one needs proper support
> for vertical CRSs, which is contained in referencing3D. I though that 
> module was not complete,
> have you looked into it and found otherwise?

As far as I know that module isn't functional.
I admit I am not 100% knowledgeable about everything in this field, I 
have only been learning this SRS stuff the past few months, so there is 
still a lot I don't know.
I am only needing to reproject point and linestring geometries with 3d 
coordinates. Perhaps that explains it.
The third coordinate seems to be transformed adequately in my tests.

Regards
Niels Charlier



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to