[Geoserver-devel] Improved GetFeatureInfo committed on trunk

Hi,
the rendering based GetFeatureInfo implementation I was talking about some days ago.

Advantages:

  • it handles point symbols with an irregular shape, you get a hit only if you click on the actually
    painted portion of the symbol (or close by), but not if you click in a empty part of it
    Works nicely also for those that simulated offsets by adding extra space in the symbol image
  • if handles properly styles with different symbol sizes, you get a hit only if you click on the
    painted symbol, instead of getting a hit considering the largest possible symbol in the style
  • conversely, if a size is fully dynamic (defined as 10 times the value of an attribute for example),
    you get a hit if you actually click on a painted symbol, instead of having to go very close to
    the center (the old code would give up and assume the search radius is the minimal one, 5px)
  • does not get issues with dash arrays or hatches, will give you a hit even if you’re clicking
    in the white space (which required some extra effort given it’s paint based)

Disadvantage:

  • it’s somewhat slower as it’s painting instead of just querying,
    and significantly slower if you have sizes that are attribute dependent,
    since it will scan the features first (without painting them, just evaluating expressions)
    to know how large the search area must be, but this can be overcome by setting the buffer search area in the feature type.
    But… I guess better slower than giving back a response that is completely inaccurate/wrong?

The code is on trunk and enabled by default, it can be turned off by setting the following system variable:
-Dorg.geoserver.wms.featureinfo.render.enabled=false

I’ll backport to 2.4.x once 2.4.3 is out, in such a way that none of the existing code is touched
(the refactor of GetFeatureInfo won’t be backported) and off by default in this case

Please try it out and let me know how it works for you, it will be in master nightly builds starting
tomorrow.

Cheers
Andrea

== GeoSolutions will be closed for seasonal holidays from 23/12/2013 to 06/01/2014 ==

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


The code is on trunk and enabled by default, it can be turned off by setting the following system variable:
-Dorg.geoserver.wms.featureinfo.render.enabled=false

Can this be added to the WMS page (I dislike customising with system variables :smiley: ). Forgive me if it already has been :slight_smile:

Jody

On Tue, Dec 17, 2013 at 10:54 PM, Jody Garnett <jody.garnett@anonymised.com>wrote:

The code is on trunk and enabled by default, it can be turned off by
setting the following system variable:
-Dorg.geoserver.wms.featureinfo.render.enabled=false

Can this be added to the WMS page (I dislike customising with system
variables :smiley: ). Forgive me if it already has been :slight_smile:

It could, but it's not really meant to be user customizable?
The current code should become the one and only choice, unless there is
something really wrong with it (the system
variable is available as a last resort fallback, just in case)

Can we kick its tires, if we see there are significant misbehaviors I'll
add the config in the UI?

Cheers
Andrea

--
*== GeoSolutions will be closed for seasonal holidays from 23/12/2013 to
06/01/2014 ==*

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

We can certainly kick its tires. If it really is not user configurable it would be good to remove the previous implementation and simplify the codebase.

···

Can this be added to the WMS page (I dislike customising with system variables :smiley: ). Forgive me if it already has been :slight_smile:

It could, but it’s not really meant to be user customizable?
The current code should become the one and only choice, unless there is something really wrong with it (the system
variable is available as a last resort fallback, just in case)

Can we kick its tires, if we see there are significant misbehaviors I’ll add the config in the UI?

On Wed, Dec 18, 2013 at 9:52 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

Can this be added to the WMS page (I dislike customising with system

variables :smiley: ). Forgive me if it already has been :slight_smile:

It could, but it's not really meant to be user customizable?
The current code should become the one and only choice, unless there is
something really wrong with it (the system
variable is available as a last resort fallback, just in case)

Can we kick its tires, if we see there are significant misbehaviors I'll
add the config in the UI?

We can certainly kick its tires. If it really is not user configurable it
would be good to remove the previous implementation and simplify the
codebase.

Eventually, yes. I'd keep it for a couple of reasons:
* benchmarks, if the accuracy is not desired, and this might indeed be a
case for making it configurable to the users, although not sure
  if any real world user would want fast but often inaccurate answers we
have today...
* something is horribly wrong with the new code, and nobody notices before
2.5.0

Using a system variable leaves us with a escape route without putting the
choice in user face. If we do add a configuration instead,
we're pretty much locked with it for the long term.
Not sure which is better.

Anyways, let's hear other opinions? Adding a config option to the GUI is
quick anyways, not a big deal, more worried that we're
going to have to live with it long term.

Cheers
Andrea

--
*== GeoSolutions will be closed for seasonal holidays from 23/12/2013 to
06/01/2014 ==*

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------