MapServer / MapServer-import

3 stars 2 forks source link

setSize method needed for mapObj? #836

Open tbonfort opened 12 years ago

tbonfort commented 12 years ago

Reporter: sgillies@frii.com Date: 2004/08/30 - 23:11

Was looking in mapobject.c and see a msMapSetSize function that calls
msMapComputeGeoTransform after setting map.width and map.height.

Does this imply that users can no longer depend on setting map height and
width directly in their scripts?
tbonfort commented 12 years ago

Author: fwarmerdam Date: 2004/08/30 - 23:30

that is correct.  We need a SetSize method.  These values shouldn't (ideally)
be set directly.

Now it is currently only an issue for rotated maps, but we should be migrating
people to setting this through a method. 
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2004/08/30 - 23:48

OK, I'll start to spread the word.  I think that most people are setting
the size attributes directly.

Furthermore, I don't know how well understood it is that setExtent is the
only reliable way to change map extents.

Maybe we need to make width, height, and extent immutable before a 4.4
release?
tbonfort commented 12 years ago

Author: fwarmerdam Date: 2004/08/30 - 23:58

Well, I would suggest we deprecate direct access in 4.4 and make them 
immutable for 4.6 so we don't break stuff too soon. 
But that is just my opinion.
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2004/09/10 - 15:59

Finished.  New usage noted in HISTORY.TXT and in mapscript/doc/mapscript.txt
in the CVS HEAD.
tbonfort commented 12 years ago

Author: hobu Date: 2004/09/18 - 01:00

eek... Can I also request that mapObj.height and mapObj.width are implemented as
a properties in python so that existing code won't break?  We can't have "half"
properties that read on one side but can't be set on the other.  As it stands
now, we wouldn't be able to implement a Python property that mimic'ed the old
behavior.

Here's what I'm thinking.  We also need an analogous getSize() that returns
(width,height)

class mapObj:
    def get_height(self):
        return self.getSize()[1] # <-- second member is the height
    def get_width(self):
        return self.getSize()[0] # <-- first member is the width
    def set_height(self, value):
        return self.setSize(self.getSize()[0], value)
    def set_width(self, value):
        return self.setSize(value, self.getSize()[1])
    width = property(get_width, set_width)
    height = property(get_height, set_height)

This will preserve the old way of doing business in Python.  I have A LOT of
code that depends on setting height/width directly.  I'm sure there are many others.
tbonfort commented 12 years ago

Author: sgillies@frii.com Date: 2004/09/18 - 02:54


map height/width can't be set directly because there needs to be some
adjustment afterwards.

map extent (rectObj) can't be set directly because some adjustment is
needed afterwards.

To me, this says that these are no longer real properties and that we
should try to figure out what are the real properties.

In the near term, hobu, your work-around is great, there's some
examples in mapscript/python/pyextend.i of using pythoncode to extend
shadow classes.
tbonfort commented 12 years ago

Author: hobu Date: 2004/09/24 - 17:51

implemented getSize() for Python and height and width properties in pyextend.i
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/11/16 - 17:05

Guys, was this setSize() function really necessary? AFAIK all mapscript
functions that use the width/height do call msPrepareImage or another function
that indirectly calls msMapComputeGeoTransform().

I am wondering if this was really needed before I enter a new bug about adding
this function to PHP MapScript.
tbonfort commented 12 years ago

Author: dmorissette Date: 2004/11/16 - 17:47

After discussion with Frank and Sean on IRC it turns out that setSize() isn't
required since there is already an assertion that all rendering or other code
that uses width/height already has to call msAdjustExtent() for the scale and
extent members. So the same way this assertion applies to the geotransform.

So it's not illegal to set width/height directly, but we agree that it's cleaner
to use a setSize() to set the width/height, extent and other similar members.