MapServer / MapServer-import

3 stars 2 forks source link

[PHP MapScript] Add $layer->getExtent() #826

Open tbonfort opened 12 years ago

tbonfort commented 12 years ago

Reporter: dmorissette Date: 2004/08/24 - 21:32

I see that SWIG MapScript has a $layer->getExtent() but PHP MapScript doesn't
have it. This method needs to be exposed in PHP MapScript as well.

The SWIG implementation takes care of checking the layer->extent member, and
also opens and closes the layer. If I copy that code to PHP MapScript, then
there are chances of the two versions going out of sync later on. Is there any
objection to moving all that logic down to the msLayerGetExtent() in maplayer.c.
I would also modify the logic to only open/close the layer if it was not
previously opened.

Here is the code from SWIG MapScript that I am referring to:

    %newobject getExtent;
    rectObj *getExtent() 
    {
        rectObj *extent;
        if (&(self->extent))
          if (MS_VALID_EXTENT(self->extent)) 
            return (&(self->extent));
        extent = (rectObj *) malloc(sizeof(rectObj));
        msLayerOpen(self);
        msLayerGetExtent(self, extent);
        msLayerClose(self);
        return extent;
    }
tbonfort commented 12 years ago

Author: hobu Date: 2004/08/24 - 22:13

Daniel,

When I was adding the ability for a layer to have its extent set through the
mapfile (bug 786), I added the bit of logic to the SWIG MapScript module to
check the layer's extent member.

I am +1 on moving that logic down into msLayerGetExtent.  Over IRC, Sean, Frank,
and I chatted about doing so.  I wasn't confident about doing it myself because
I didn't know if the opening/closing semantics would cause trouble for all of
the different layer types.
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/08/25 - 18:35

I have ported (copied) the function from SWIG to PHP MapScript in the 4.2.2
branch. This brings both MapScripts in synch and will make it in 4.2.3.

For V4.3, I will evaluate the impacts of moving the logic out of the SWIG and
PHP wrappers to msLayerGetExtent(). This is a cleaner solution, but there is a
little risk so I didn't want to take that risk for the 4.2 branch.
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2004/08/25 - 18:40

Daniel, there is no SWIG layerObj.getExtent in the 4.2 branch.  It is in CVS HEAD
only.
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2004/08/25 - 18:50

Oops, just checked 4.2 and there it is.  I was going from Hobu's note in the
other thread where he said CVS HEAD.

Isn't the 4.2 branch supposed to be somewhat frozen?  Bug fixes only and no new
features (new bugs)?
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/08/25 - 19:34

Howard's comment were about what he added in the CVS HEAD... but the function
was there already in 4.2 as you saw.

As for 4.2 being frozen, you are exactly right. I'll take a 20 points hit for
doing this. :(  I've been debating whether the function should be ported or not
in 4.2, but it's been needed for a long time, it was already in the SWIG
version, and my change was very localized so I thought this would pass smoothly
as a bug fix and nobody would notice. Nice to see that others are watching.
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2004/08/25 - 19:43

Sorry, Daniel, I was *totally* confused earlier ... I thought Howard's
layerObj.extent work had been merged into 4.2, and that this was not
quite appropriate.

I think that synching up the mapscript modules within a revision is
the right thing to do.  No problems with this.

I'd better start working on my reading comprehension!
tbonfort commented 12 years ago

Author: hobu Date: 2004/08/25 - 20:54

Daniel,

I saw that you checked in stuff for layer->getExtent() into the 4.2 branch.  Are
you also going to move the open/close logic and the getExtent() logic into
msLayerGetExtent on CVS head?
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/08/25 - 21:10

Yes, I will do that in 4.3 (see comment#2). However before I do it I need to
take time to evaluate the potential impacts of moving the open/close calls down
to msLayerGetExtent(), and to figure a way to prevent side-effects on code that
already takes care of opening and closing the layers
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/11/10 - 22:05

Doh! I really need to port $layer->getExtent() to 4.4 now!
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/11/11 - 02:27

Implementing getExtent() in 4.4 now.
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/11/11 - 23:42

Um... I did this yesterday and thought I marked it fixed... seems that I didn't
hit submit.

Anyway, $layer->getExtent() is implemented in 4.4. the same way as in SWIG,
including the open/close and the use of layer->extent if set. 

The issue of moving all that logic down to msLayerGetExtent() remains, I have
filed bug 1051 about that.