msClipPolygonRect() / msClipPolylineRect()

[Frank Warmerdam]

On Thu, 24 Mar 2005 12:57:43 -0500, Stephen Woodbridge
<woodbri at swoodbridge.com> wrote:
> Frank,
>
> Excellent! Performance has been an issue discussed on the list in resent
> history, any chance of getting this fix back ported to a dot release of
> 4.4.x

Steve,

It is a fairly trivial change and easily backported to 4.4.2.  It is
just an extra little check in the two functions in mapprimitive.c.

However, I myself would prefer to let the change prove itself
in 4.5 for a 4.6 public release rather than backport it.  But others
might want to take a more aggresive view on this.  I just hate
fiddling around in point releases with changes that aren't clearly
bug fixes.

> I think that we should defer to the shapefile spec on bounds being set
> correctly. It is possible to have "bad" shapefiles so so some sanity
> checks should be in the code to trap bogus or out of bounds values and
> then die with some useful message about which file was bad. I would then
> add a bounds check to the verify or check shape utility to report and
> fix things.

I was going to say that I don't think the bounds from the shapefile are
used for the shapeObj, but reviewing mapshape.c, it seems that they
are.  So if they were corrupt in the source file, that could cause problems.
However, I believe the same bounds are used for the spatial querying so
it wouldn't just cause problems for my change.

Looking at this code, I can't help but wonder if we couldn't avoid reading
all the geometry for shapes that are ourside our current rendering window
if we did slightly closer integration between the mapshape.c shape reading
code and the higher level layer logic for shapes.   However, this is outside
the scope of my current optimization efforts focused on S-57 input.

> I would rather assume that my data is good and get the extra rendering
> speed and if I have bogus data then running lots of files through the
> checker is the price I have to pay.

I would tend to agree in an application as performance sensitive as MapServer.
Actually, it would be kind of nice if MapServer did extra validation of stuff
like this, but only when DEBUG is set on.

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent