MapServer / MapServer-import

3 stars 2 forks source link

Layer tolerance default value #1103

Open tbonfort opened 12 years ago

tbonfort commented 12 years ago

Reporter: jlacroix Date: 2004/12/03 - 17:03

The tolerance default value in the layer is 0. This works well with polygon
layers since this value will return query result for a click inside the polygon.
However for line and point layers a tolerance of 0 seems always return nothing
(It's the like the layer is not queryable). In the documentation the default
tolerance is marked as 3, but in the code it's 0.

Should we update the documentation or update the tolerance value?
Steve?
tbonfort commented 12 years ago

Author: sdlime Date: 2004/12/03 - 18:27

The problem is that there is no perfect default value. Either 3 or 0 is 
problematic in certain cases.

I think we need a way to set a default based on layer type. That would be easy 
via straight CGI or simple mapfile loading via MapScript. It's harder 
(impossible?) when creating layers dynamically, unless we require layer type in 
the constructor.

In lieu of a fix as described above I would vote to change the documentation 
since people have probably already reacted to the behavior of the software. 
Changing documentation doesn't break applications...

Steve
tbonfort commented 12 years ago

Author: jlacroix Date: 2004/12/03 - 19:35

I agree with you on this one. Since we don't know the layer type at the
beginning we can't set the default tolerance. I will add Jeff in CC to update
the Doc.

Jeff: The Layer TOLERANCE default value is 0, not 3 like stated in the doc.

Steve: Maybe we can reuse the same trick for layer tolerance default value that
we use for the symbol default size. Set it to -1 by default. When doing a query,
use the tolerance value or a default value based on the layer type if it's -1.
Just want to have a thought on this.
tbonfort commented 12 years ago

Author: sdlime Date: 2004/12/04 - 00:05

Julien: Good idea on setting tolerance in the query functions. Let's pursue 
that for 4.6, but update the documentation now.

Steve
tbonfort commented 12 years ago

Author: jlacroix Date: 2005/04/13 - 17:53

Aiming 4.6 for the dynamic tolerance default value.
I may take care of this one if I have time.
tbonfort commented 12 years ago

Author: jlacroix Date: 2005/04/15 - 17:13

Reassign to me, I will start working on this today.
tbonfort commented 12 years ago

Author: jlacroix Date: 2005/04/15 - 21:40

I implemented this. The default is now -1. When doing a query, it check for the
layer type. If it's a POINT or a LINE, the default is 3. For all other layer
types, the default is 0.

Marking as FIXED.

Reopen if you see any other issue.
tbonfort commented 12 years ago

Author: sdlime Date: 2005/04/15 - 21:54

Where specifically is the default being set now? Queries are notoriously 
complex and you would've had to modify a number of functions. Just want to make 
sure nothing was missed...

Steve
tbonfort commented 12 years ago

Author: jlacroix Date: 2005/04/15 - 22:19

It seems that the query stuff was only using the tolerance value from the layer
object only at 4 places. you can grep on "layer_tolerance" to see where this is.

If I forgot something, please reopen and I will try to fix it.
tbonfort commented 12 years ago

Author: jmckenna@dmsolutions.ca Date: 2005/04/15 - 23:04

julien did u update the docs for this, or does that still remain?
tbonfort commented 12 years ago

Author: jlacroix Date: 2005/04/19 - 22:39

Sorry for the late answer jeff. I opened bug 1323 for updating the documentation
about this.

Just to know, where are the files to update for the doc?
tbonfort commented 12 years ago

Author: jmckenna@dmsolutions.ca Date: 2005/04/20 - 17:07

no problem

:pserver:msdoc@cvs.gis.umn.edu:/data2/cvsroot 
module: mapserver_docs
file: mapfile-reference.xml

i'm on it
tbonfort commented 12 years ago

Author: jmckenna@dmsolutions.ca Date: 2005/04/20 - 19:45

tested.  nice improvement.
tbonfort commented 12 years ago

Author: sdlime Date: 2005/10/26 - 06:01

I think we may need to re-examine this. I just ran into a situation where these
mods didn't make sense. Specifically with the query by shape functions. I'm not
sure anything but a default tolerance of 0 for all layer types makes sense for
these functions. If you're not careful it's easy to over select points with
polygons by default. To me if I'm using a polygon to select points the default
should be only the points in the polygon. That's not the case now.

I'm of the opinion now that that default of 3 for points and lines makes sense
with point based queries only but in all other cases the default should be 0.

Thoughts?

Steve
tbonfort commented 12 years ago

Author: sdlime Date: 2005/11/05 - 07:13

Any thoughts on this guys? It's really a problem because in some cases because
you can end up with more results than you should have. Depending on the
situation that's a very subtle error. I prefer less subtle errors like no points
found. I think we can do better. A default tolerance of 0 should be used for
points and lines when using doing box or polygon queries. A tolerance of 3 makes
sense with point or line-based queries.

Steve
tbonfort commented 12 years ago

Author: jmckenna@dmsolutions.ca Date: 2006/01/10 - 21:16

i was just trying to test comment#13.  I tested with cgi (itasca demo,
rubberband box query) and phpmapscript (gmap demo, box query) and could not
over-select points (and i removed tolerance values set in mapfile)  I tested
with 4.8.0-rc2.

sorry.
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/10 - 21:40

The place I ran into this with was when querying by shape within MapScript and 
it wreaked havoc trying to debug because points outside the query shape were 
being selected and the number selected varied by image resolution (where the 
tolerance is computed). The persons doing the development were expecting strict 
intersection when using non-point based selection and that's not that case. 

The software is working correctly, just not in the way the user expects.

On one hand this is simply a RTFM problem. On the otherhand a tolerance other 
than 0 only makes sense for point-based queries of points and lines.

Steve
tbonfort commented 12 years ago

Author: jmckenna@dmsolutions.ca Date: 2006/01/10 - 21:59

k.  in that case I agree with your recommendations in comment#13.
tbonfort commented 12 years ago

Author: jlacroix Date: 2006/01/10 - 22:38

Steve do you think it's possible to use different default values only in case of
point and line queries?

I think only in case of point query is feasible since it got its own function.
tbonfort commented 12 years ago

Author: sdlime Date: 2006/01/10 - 23:59

Yup, I think it's possible. The only place right now we should set a default 
tolerance other than zero would be in msQueryByPoint(). We could also do 
something in msQueryByFeature and msQueryByShape based on feature/shape type 
but those functions only support polygon selection shapes. 

Mind you, this is nothing that needs to be fixed ASAP so I'm marking as 
future...

Steve
tbonfort commented 12 years ago

Author: sdlime Date: 2009/04/17 - 05:59 This just bit a user from MapScript. The msQueryByShape function

tbonfort commented 12 years ago

Author: sdlime Date: 2009/04/17 - 06:02 Whoops, forgot to finish typing. This just bit a user from MapScript again. The msQueryByShape() function sets a default of 3 for points/lines and you end up with more than you expect. We really need to fix the code (default = 0 for polygon queries in msQueryByShape/msQueryByFeatures) and clarify things in the docs for 6.0.

Steve

tbonfort commented 12 years ago

Author: sdlime Date: 2009/05/02 - 15:53 See ticket #2998 for an implementation error with the fix put in place for this one. Wrong layer parameter was being checked. Probably need to retest...

Steve

tbonfort commented 12 years ago

Author: jlacroix Date: 2009/05/04 - 16:00 Shame on me for ticket #2928! :$

We still use a layer tolerance different than 0 in the QueryByShape and QueryByFeature function though. I'll make some tests taking your previous comment into account and update the necessary code.