MapServer / MapServer-import

3 stars 2 forks source link

Adding support for WKT strings to MapServer... #1466

Open tbonfort opened 12 years ago

tbonfort commented 12 years ago

Reporter: sdlime Date: 2005/09/09 - 23:36

This is the parent bug for the proposal to add WKT (Well Known Text) support to 
MapServer as described in RFC 2.
tbonfort commented 12 years ago

Author: sdlime Date: 2005/09/28 - 05:26

*** Bug 948 has been marked as a duplicate of this bug. ***
tbonfort commented 12 years ago

Author: sdlime Date: 2005/09/28 - 05:26

*** Bug 1115 has been marked as a duplicate of this bug. ***
tbonfort commented 12 years ago

Author: dmorissette Date: 2005/11/24 - 04:39

Steve, is RFC2 completed in 4.8 or will it come only in a later release?
tbonfort commented 12 years ago

Author: sdlime Date: 2005/11/25 - 17:56

The majority is already in 4.8. Both Frank and I have added wrapper for GEOS 
and OGR. There is a new WKT keyword in the feature object of a mapfile so you 
can create features via the mapfile or via URL. The swig MapScript interface 
includes the ability to use WKT via the shapeObj constructor and the shapeObj 
has a toWKT() method.

Todo's:

  - WKT via PHP/MapScript
  - WKT format support for mapshape CGI parameter
  - updating the Python tests

We were rushing to beat the feature freeze and I forgot to update this bug. 
Sorry.

Steve
tbonfort commented 12 years ago

Author: szekerest Date: 2005/12/05 - 21:24


Steve,

You should keep the original constructor in shape.i, beacuse the current 
modification breaks the compatibility with the existing code in c#, because it 
cannot handle default values.

Tamas Szekeres
tbonfort commented 12 years ago

Author: sdlime Date: 2005/12/05 - 21:38

I don't know. Very few people use those bindings and it seems a shame to 
convolute things because of a SWIG problem. There really isn't a good 
alternative other than perhaps using the original constructor for C# and the 
new one for everyother language. Ideas? If we have to break things then this is 
the time to do it.

Steve
tbonfort commented 12 years ago

Author: szekerest Date: 2005/12/08 - 21:20

Steve,

I think using multiple constructors (function overloading) is not a bad 
concept  depending on those parameters that are needed for the initialisation. 
Many of the languages highly play upon this feature. If you intend not to 
provide a default constructor it implies that this object should be 
initialized from a wkt most of the time, but I think it's not the case. The 
shapeObj can also be created by adding lineObj after the initialisation.

So for this case a static createFromWKT() would be more appropriate.

After all if you decide to keep this solution it should be notified as a 
warning for the users.

Tamas
tbonfort commented 12 years ago

Author: dmorissette Date: 2005/12/08 - 23:09

I'd like to add the functions to PHP MapScript once you've decided on the final
function names to use. Can you please summarize once you've made a final
decision... and perhaps also update the mapscript/doc/mapscript.txt?

So far what I see in shape.i is:

... the constructor:
    shapeObj(int type=MS_SHAPE_NULL, char *wkt=NULL) 

... and 1 method:
    char *toWKT()
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2005/12/09 - 06:32

Steve, see http://www.swig.org/Doc1.3/Java.html#overloaded_functions about
function overloading to have multiple constructor forms for Java and C#.
tbonfort commented 12 years ago

Author: sdlime Date: 2005/12/09 - 16:57

Thanks for the link Sean.

Steve
tbonfort commented 12 years ago

Author: sdlime Date: 2005/12/20 - 19:50

Looks like swig 1.3 fixes things with respect to overloading. Who are the best 
candidates to test java and C# with respect to 4.8? I need to update the CVS 
machine to the newest version of SWIG as well.

Steve
tbonfort commented 12 years ago

Author: szekerest Date: 2005/12/21 - 00:49


For C#, I have tried both of the constructors and worked without any problems. 
If you commit the proposed change i will try it.

Tamas
tbonfort commented 12 years ago

Author: sdlime Date: 2005/12/27 - 21:36

This is the 4.8 tree now (has been for awhile).
tbonfort commented 12 years ago

Author: dmorissette Date: 2006/01/03 - 18:18

Steve, can you please confirm that the new methods that need to be added to PHP
MapScript are:

... the constructor:
    shapeObj(int type=MS_SHAPE_NULL, char *wkt=NULL) 

... and 1 method:
    char *toWKT()

The PHP methods may have to wait until after the 4.8.0 release though.
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/03 - 20:30

That's the ticket...

Steve
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/13 - 17:41

I've been thinking about this more. I wonder if the overloaded constructor 
really makes sense or should we do a fromWKT... So, processing would look like:

# in perl
$shape = new $mapscript::shapeObj($mapscript::MS_SHAPE_NULL);
$shape->fromWKT($wkt_string);

It's not too late to make this change since this is a new feature. Any opinions 
or should I quit worrying about it.

Steve
tbonfort commented 12 years ago

Author: szekerest Date: 2006/01/13 - 18:09


It would be desirable to mark this function as "static" in order to eliminate
the need of creating a new object previously.

Tamas
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2006/01/13 - 18:55

I mentioned it in an email thread, but I should have commented here before. I
think a fromWKT is much better.
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/13 - 20:37

I agree with you and Tamas, just took me awhile to catch on. I will make the 
change to shape.i this evening.

Steve
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/14 - 05:26

Guys: Any idea how to code this? I've tried a couple of things and either:

  - it works great if there's no error, with a segfault if there is an error

or 

  - it works great with a error, but segfaults with solid input

The interface code for the last case is below. The former happens if you blow
away the old shapeObj (self) and then use self = msShapeFromWKT()...

Mapscript code looks like:

$shape = new mapscript::shapeObj() or die("Unable to create shape.");
if(($status = $shape->fromWKT('LINESTRING(0 0,1 1,1 2)')) != 0) {
  die(mapscript::msGetErrorString("\n")); 
}

Steve

    int fromWKT(char *string)
    {
        shapeObj *shape;
        int i;

        if(!string) return(MS_SUCCESS); /* ok, I guess */

        shape = msShapeFromWKT(string);
        if(!shape) return(MS_FAILURE);

        /* Allocate memory for 4 values */
        if ((shape->values = (char **)malloc(sizeof(char *)*4)) == NULL) {
            msSetError(MS_MEMERR, "Failed to allocate memory for values",
"fromWKT()");
            return(MS_FAILURE);
        } else {
            for (i=0; i<4; i++) shape->values[i] = strdup("");
        }
        shape->numvalues = 4;

        /* free anything previously allocated */
        msFreeShape(self);
        free(self);
        self = shape;

        return(MS_SUCCESS);
    }
tbonfort commented 12 years ago

Author: szekerest Date: 2006/01/14 - 22:39


I prefer the following solution:

%newobject fromWKT;
    static shapeObj *fromWKT(char *wkt) 
    {
        shapeObj *shape;
        int i;

        shape = msShapeFromWKT(wkt);
        if (!shape)
            return NULL;

        /* Allocate memory for 4 values */
        if ((shape->values = (char **)malloc(sizeof(char *)*4)) == NULL)
        {
            msSetError(MS_MEMERR, "Failed to allocate memory for values",
                       "fromWKT()");
            return NULL;
        }
        else
        {
            for (i=0; i<4; i++) shape->values[i] = strdup("");
        }
        shape->numvalues = 4;

        return shape;
    }

To create the shape in c# one could use:

shapeObj myNewShape = shapeObj.fromWKT("LINESTRING(0 0,1 1,1 2)");

I suggest to keep from using keywords such as "string" as a parameter name. This
may result in uncompilable code.

Tamas
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/14 - 22:53

Why static? I'm not familiar with that with regards to SWIG... If I get what
your suggesting then the perl code would be:

$shape = mapscript::shapeObj.fromWKT($wk); 

or something like that. Correct? Sort of an alternate constructor. Will SWIG
still  destroy it properly if the class constructor is not used?

(I agree on the string name and will change) 

Steve
tbonfort commented 12 years ago

Author: szekerest Date: 2006/01/15 - 23:08


Steve

Static functions can be called without the need of creating an object instance.
This approach suits best for your aim mentioned previously, because you would
not like to use multiple constructors. The object construction takes place
inside the fromWKT function which behaves like a constructor, but constructs an
other object not the self.

After creating the object it behaves like any other objects, the destructor -if
any - will be called automatically.

Nevertheless I wonder if this function works correctly because of a confusion in  
msOGRShapeFromWKT in mapogr.cpp.

See
http://mapserver.gis.umn.edu/bugs/show_bug.cgi?id=1614
for more details

Tamas Szekeres
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/17 - 02:28

I figured that's what was up. Sounds like a good solution if it works. I can't
get a perl version working. I may just be having a syntax problem and will test
more. I've only used the GEOS WKT support so no idea on the OGR problems you're
seeing.

Steve
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/17 - 05:27

Ok, I've verified things work in Perl. You do something like:

  $shape = mapscript::shapeObj::fromWKT('LINESTRING(0 0,1 1,1 2)');

I'll commit with just a bit more feedback about other languages, especially Dan
on PHP...

Steve
tbonfort commented 12 years ago

Author: szekerest Date: 2006/01/17 - 23:35


I have tested some wkt-s with C# applying Frank's fix to msOGRShapeFromWKT and
worked.

Tamas
tbonfort commented 12 years ago

Author: dmorissette Date: 2006/01/17 - 23:52

In response to Steve's question from comment #25, we do not use object-oriented
constructors in PHP MapScript, we use regular functions instead (e.g.
ms_newLayerObj())... this is legacy from the PHP3 days since I think we should
be able to use "new layerobj();" in PHP4 and PHP5 (I didn't try though).

Anyway, we can't change that now, so to be consistent with what's already in
place, we could create a ms_shapeObjFromWkt() function for PHP MapScript.
tbonfort commented 12 years ago

Author: dmorissette Date: 2006/01/18 - 01:04

I'll add the PHP methods now:

    shapeObj ms_shapeObjFromWkt(char *wkt) 
    char *shapeObj::toWKT()
tbonfort commented 12 years ago

Author: dmorissette Date: 2006/01/18 - 01:49

I've added shapeObj::toWkt() and ms_shapeObjFromWkt() to PHP MapScript in both
the 4.8 branch and 4.9-dev. Jeff: the PHP MapScript readme has been updated with
the two new functions, you may need to update the docs on the website.

Steve: it seems that the msOGRShapeToWkt() implementation is not finished and it
always returns NULL, is this possible?

Also, can you please confirm that the return value (string) from msShapeToWkt()
is a newly allocated string that should be freed by the caller? I could not get
confirmation of that from looking at the code.
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/18 - 02:36

I updated shape.i to use the method favored by Tamas and Sean in both 4.8 and
4.9. I've also added Frank in the CC to comment on OGR stuff. The GEOS WKT
writer does not create free'able output. It could depending on what Frank's
stuff is doing.

Steve
tbonfort commented 12 years ago

Author: dmorissette Date: 2006/01/18 - 03:36

My current implementation calls msFree() on the string returned by toWkt(). It's
easy to change, just let me know.
tbonfort commented 12 years ago

Author: fwarmerdam Date: 2006/01/18 - 05:49

I have completed implementation of the OGR shape2wkt function (which 
I had apparently not finished before) and I have ensured the returned string
is suitable to free with msFree(), even in odd ogr-is-in-a-dll situations.
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/18 - 06:38

I'll go ahead then and update the geos code to produce a free'able string, and
the SWIG code to create a newobject...

Steve
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/18 - 08:25

Made the last of the changes. This one should be good to go in 4.8 and 4.9...

Steve
tbonfort commented 12 years ago

Author: jmckenna@dmsolutions.ca Date: 2006/01/18 - 19:53

i've updated the phpmapscript doc on the plone site.
tbonfort commented 12 years ago

Author: hobu Date: 2007/08/08 - 00:31 Reading through, I don't see any reason why this bug can't be closed.

tbonfort commented 12 years ago

Author: sdlime Date: 2007/08/08 - 01:24 You are correct sir...