[Geoserver-devel] SRS mess, a proposal

GeoServer GeoServer Developer
1

Hi all,
I just had an interesting talk with Alessio about the SRS
handling we have now in Geoserver, in particular how
to handle the (possible) difference between the declared
SRS and the actual one.

When I say data in the following, I mean both vector and
grid coverage data.
We have three possible cases:
1) data contains a CRS and a correspondent EPSG number is found
    automatically (this happens, thought not in the same way, in
    both vector and raster data)
2) data does not contain a CRS indication, the user declares one
3) data contains a CRS indication, but no matching EPSG number
    can be found because the CRS is not in the EPSG database.

Case 1) is the happy one, no extra work is needed.

Case 2) is badly handled now. The user could add SRS information
in his data to solve it, but we know few users are good enough
to do that. Besides, he already declared an SRS, no?
Unforunately the declared SRS is used for computing the lat/lon envelope, and for declaring the SRS in GML2 responses,
but the web map rendering subsystem just ignores it,
and ignores any kind of reprojection. So, if the original data
did not have an SRS, it's not possible to reproject data.
To solve this, we need to put inside the map context a feature
source that forces the crs into the feature type (the renderer
uses the CRS attached to the GeometryAttributeType).
This is not hard, I'll handle it soon (GEOS-894)

Case 3) is a festival of approaches.
Envelope computation:
FeatureType envelope computation just ignores the original
CRS. Coverage instead assumes the original CRS is the right
one, thus computes the envelope in that CRS, and then
back projects it into 4326. Since coverages do declare an
envelope in the declared SRS, the envelope is then projected
in the declared system. (original -> 4326 -> declared).
I think the coverage one is the right approach.
On the rendering side, WMS will always use the declared CRS
anyways. On the WFS 1.0 side, we're in trouble, since we're
outputting coordinates in the original CRS, but declaring
a different one. We should be reprojecting instead IMHO,
something that is again easy to do, since we already have a
ReprojectFeatureReader.

Of course it would be nice to have a way to tell the user
about lacks and mismatches, and automatically register new CRSs
in the database, but I guess this is a bit beyond our scope
for 1.5.0. The changes proposed above are not big, and I would like to include them in both 1.5.0 and 1.4.1.

Opinions?
Cheers
Andrea

PS: btw Justin, how does WFS 1.1 deal with cases 2 and 3?

2

Andrea Aime ha scritto:

Hi all,
I just had an interesting talk with Alessio about the SRS
handling we have now in Geoserver, in particular how
to handle the (possible) difference between the declared
SRS and the actual one.

Btw, I just discovered that the current Geoserver code tried
to force the declared SRS into the feature source, but failed
to do so effectively, because returned readers and feature results
and their generated features still report the original feature type.

I'll have to tap these holes, but would also prefer to keep using
the original SRS and have the code handle the fact that the
SRS declared in the feature type may not be then one you're
getting from readers (readers gives you the original one, if there
is one, the declard CRS is forced only if data is missing one).

Cheers
Andrea

3

PS: btw Justin, how does WFS 1.1 deal with cases 2 and 3?

The same as before. Since the only dataset its really been tested with is the cite dataset, for which falls into case 1.

-Justin

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:1004,45c8963c41331194215290!

--
Justin Deoliveira
jdeolive@anonymised.com
The Open Planning Project
http://topp.openplans.org

4

Andrea Aime wrote:

Andrea Aime ha scritto:

Hi all,
I just had an interesting talk with Alessio about the SRS
handling we have now in Geoserver, in particular how
to handle the (possible) difference between the declared
SRS and the actual one.

Btw, I just discovered that the current Geoserver code tried
to force the declared SRS into the feature source, but failed
to do so effectively, because returned readers and feature results
and their generated features still report the original feature type.

I'll have to tap these holes, but would also prefer to keep using
the original SRS and have the code handle the fact that the
SRS declared in the feature type may not be then one you're
getting from readers (readers gives you the original one, if there
is one, the declard CRS is forced only if data is missing one).

Your approach sounds good. It would be very good to fix the reprojection issues, I hadn't realized that we weren't successfully reprojecting WMS images. It makes sense though, GeoServer started with no innate concept of SRS, and it was up to the user to supply the right meta-information for their dataset. WFS 1.0 allowed no reprojection. GeoTools has advanced, to be able to handle lots of it, but it looks like GeoServer never really caught up.

So go for it, it'd be great to have that working well.

Chris

Cheers
Andrea

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:1003,45c8a82854981527717022!

--
Chris Holmes
The Open Planning Project
http://topp.openplans.org

5

Andrea Aime wrote:

Andrea Aime ha scritto:
  

Hi all,
I just had an interesting talk with Alessio about the SRS
handling we have now in Geoserver, in particular how
to handle the (possible) difference between the declared
SRS and the actual one.
    

Btw, I just discovered that the current Geoserver code tried
to force the declared SRS into the feature source, but failed
to do so effectively, because returned readers and feature results
and their generated features still report the original feature type.
  

I saw that this issue (geotools code not respecting the Query) has been patched after the fact
for the rendering code. Can we move this patch (ie wrap the FeatureReader) into the troubled
data stores please?
Can we get a list of offenders and make sure bug reports exist?

I'll have to tap these holes, but would also prefer to keep using
the original SRS and have the code handle the fact that the
SRS declared in the feature type may not be then one you're
getting from readers (readers gives you the original one, if there
is one, the declard CRS is forced only if data is missing one).

Cheers
Andrea
  

6

Jody Garnett ha scritto:

Andrea Aime wrote:

Andrea Aime ha scritto:

Hi all,
I just had an interesting talk with Alessio about the SRS
handling we have now in Geoserver, in particular how
to handle the (possible) difference between the declared
SRS and the actual one.
    

Btw, I just discovered that the current Geoserver code tried
to force the declared SRS into the feature source, but failed
to do so effectively, because returned readers and feature results
and their generated features still report the original feature type.
  

I saw that this issue (geotools code not respecting the Query) has been patched after the fact
for the rendering code. Can we move this patch (ie wrap the FeatureReader) into the troubled
data stores please?

_Every_ data store ignores the forced CRS and the reprojected one
when asked to do things in read/write mode.
The only place where forced and reproject CRS are used is in the
DataStore.getView(Query) method, that works read only.
Handling CRS forcing in write mode should not be that hard, and requires modifying all getFeatureWriter(xxx) methods, doing the same for reprojection is harder and requires modifying all feature writer
classes to do back-reprojection before writing down the result.
Oh, I forgot one important detail: getFeatureWriter does not accept query, so how I tell it that I want to change CRS?
Hum, this could be a real exercise for the API change handling procedures? It would be a way to break the DataStore API, since we
would need new methods in it.

Can we get a list of offenders and make sure bug reports exist?

There is no bug report, all data stores are offenders. Too much
work for me given my time frames, that's why I'm looking for a
Geoserver side solution, where I can fix at least CRS forcing
in a single place.
To do things quick, I could do this only for the read only case using getView(xxx).
Too bad the datastore API is designed is such a way that I cannot
distinguish read only access from read/write one.
I'll be forced to do some kind of temporary hack, until the gt2
story gets fixed (which I can provide some time to help fixing,
only I cannot do that under a release deadline).

Cheers
Andrea

7

Too bad the datastore API is designed is such a way that I cannot
distinguish read only access from read/write one.

I believe the way it was designed to distinguish read vs. readwrite was to do instanceof FeatureStore. The datastore returns a FeatureSource if it's read only, and FeatureStore if it's read/write (and FeatureLocking if it can handle locking). I concede that it may not be the best design, but I think it should still work...

Chris

--
Chris Holmes
The Open Planning Project
http://topp.openplans.org

8

Chris Holmes ha scritto:

Too bad the datastore API is designed is such a way that I cannot
distinguish read only access from read/write one.

I believe the way it was designed to distinguish read vs. readwrite was to do instanceof FeatureStore. The datastore returns a FeatureSource if it's read only, and FeatureStore if it's read/write (and FeatureLocking if it can handle locking). I concede that it may not be the best design, but I think it should still work...

Sorry, I did not explain myself properly.
What I maean, is that when I'm inside FeatureStore and people ask for
a feature collection, I don't know I they just want to read or also
write on it. The client side can use instanceof to determine whether
they can write, but I don't have a way to tell what the client side
wants to do...

Cheers
Andrea

9

Andrea Aime wrote:

_Every_ data store ignores the forced CRS and the reprojected one when asked to do things in read/write mode.

I see; let me review the API and get back to you on this. I would much rather apply a patch in one spot then force applications to apply wrappers. Currently we have two "applications" doing this work (GeoServer if you proceed and the renderer).

The only place where forced and reproject CRS are used is in the DataStore.getView(Query) method, that works read only.

I am going to go look at this - getView origionally was simply a way to combine a predefined Query with with one for later; if it has also started applying wrapping code that would be the magic *three* hacks around one DataStore QA problem.

Handling CRS forcing in write mode should not be that hard, and requires modifying all getFeatureWriter(xxx) methods, doing the same for reprojection is harder and requires modifying all feature writer classes to do back-reprojection before writing down the result.

Oh, I forgot one important detail: getFeatureWriter does not accept query, so how I tell it that I want to change CRS?

My impression is the reprojecting FeatureWriter did this sort of thing; or the ReprojectingFeatureCollection in the work justin and I were working on.

Hum, this could be a real exercise for the API change handling procedures? It would be a way to break the DataStore API, since we
would need new methods in it.

I am not interested in maintaining FeatureWriter as an API, I find this better handled (and almost identically handled) in a FeatureCollection implementation (the only difference is you can use the FeatureCollection more then once).

Can we get a list of offenders and make sure bug reports exist?

There is no bug report, all data stores are offenders. Too much
work for me given my time frames, that's why I'm looking for a
Geoserver side solution, where I can fix at least CRS forcing
in a single place.

Gak.

To do things quick, I could do this only for the read only case using getView(xxx).
Too bad the datastore API is designed is such a way that I cannot distinguish read
only access from read/write one.

The ability to distinguish was one of the suggestions put into the DataAccess proposal.

I'll be forced to do some kind of temporary hack, until the gt2 story gets fixed (which
I can provide some time to help fixing, only I cannot do that under a release deadline).

Okay understood the difference between deadline and fix; for now can we get a bug report
and list this as one of the things we need straight for the GeoTools 2.4 timeframe.

Jody

Cheers
Andrea

10

Jody Garnett ha scritto:

Andrea Aime wrote:

_Every_ data store ignores the forced CRS and the reprojected one when asked to do things in read/write mode.

I see; let me review the API and get back to you on this. I would much rather apply a patch in one spot then force applications to apply wrappers. Currently we have two "applications" doing this work (GeoServer if you proceed and the renderer).

Geosever may be a good case, the renderer should reproject only after decimation. If not, it's plain stupid.
Anyways, I already did the thing with Geoserver, I have a release to
put out of the door.

The only place where forced and reproject CRS are used is in the DataStore.getView(Query) method, that works read only.

I am going to go look at this - getView origionally was simply a way to combine a predefined Query with with one for later; if it has also started applying wrapping code that would be the magic *three* hacks around one DataStore QA problem.

Handling CRS forcing in write mode should not be that hard, and requires modifying all getFeatureWriter(xxx) methods, doing the same for reprojection is harder and requires modifying all feature writer classes to do back-reprojection before writing down the result.

Oh, I forgot one important detail: getFeatureWriter does not accept query, so how I tell it that I want to change CRS?

My impression is the reprojecting FeatureWriter did this sort of thing;

There is no reprojecting feature writer I can see, and how could you
create one if you don't pass the CRS information to getWriter(xxx)?
That information is in a Query object, remember?

Hum, this could be a real exercise for the API change handling procedures? It would be a way to break the DataStore API, since we
would need new methods in it.

I am not interested in maintaining FeatureWriter as an API, I find this better handled (and almost identically handled) in a FeatureCollection implementation (the only difference is you can use the FeatureCollection more then once).

I tried to implement a wrapping FeatureCollection, but stopped right away. I would have had to implement 50 methods!!! This is absurd, no one will implement the custom Feature collections you envisage under these
conditions, an abstract class providing boiler plate code is needed,
something along the lines of AbstractCollection in the collection framework, that leaves you to implement only 2 methods.

The ability to distinguish was one of the suggestions put into the DataAccess proposal.

The DataAccess proposal is a nice view of how the API may become in a 2 years timeframe (since it breaks all code plain and does not allow
for most the optimizations that do make Geotools work (think about optimized data loading in the renderer)).
Give me something that we can realistically implement in 3/6 months
and I'll consider it. I'm more for breaking a little the existing API
in order to fix it than creating another Geotools.

I'll be forced to do some kind of temporary hack, until the gt2 story gets fixed (which
I can provide some time to help fixing, only I cannot do that under a release deadline).

Okay understood the difference between deadline and fix; for now can we get a bug report
and list this as one of the things we need straight for the GeoTools 2.4 timeframe.

There's no way to fix this unless we allow getWriter(xxx) to accept a Query. After that, you "simply" have to add the method to all datastores, link to feature collections and feature results around, or
else you'll end up with another half backed solution like the filters in 2.3.0.
The fact you don't like writers does not match well with the reality that writers are still not deprecated in trunk, and that feature collection is not, imho, in a state that allows it be seen as a replacement.

Cheers
Andrea