Opened 20 years ago

Closed 20 years ago

Last modified 19 years ago

#836 closed defect (fixed)

setSize method needed for mapObj?

Reported by: sgillies@… Owned by: sgillies@…
Priority: high Milestone:
Component: MapScript-SWIG Version: 4.3
Severity: minor Keywords:
Cc:

Description

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?

Change History (11)

comment:1 by fwarmerdam, 20 years ago

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. 

comment:2 by sgillies@…, 20 years ago

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?

comment:3 by fwarmerdam, 20 years ago

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.

comment:4 by sgillies@…, 20 years ago

Resolution: fixed
Status: newclosed
Finished.  New usage noted in HISTORY.TXT and in mapscript/doc/mapscript.txt
in the CVS HEAD.

comment:5 by hobu, 20 years ago

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.

comment:6 by hobu, 20 years ago

Cc: hobu@… added

comment:7 by sgillies@…, 20 years ago

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.
 

comment:8 by hobu, 20 years ago

implemented getSize() for Python and height and width properties in pyextend.i

comment:9 by dmorissette, 19 years ago

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.

comment:10 by dmorissette, 19 years ago

blocked: 1066

comment:11 by dmorissette, 19 years ago

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. 
Note: See TracTickets for help on using tickets.