OpenLayers OpenLayers

Ticket #533 (closed feature: fixed)

Opened 1 year ago

Last modified 7 months ago

Add SLD format

Reported by: sderle Assigned to:
Priority: major Milestone: 2.6 Release
Component: Format Version:
Keywords: Cc:
State: Complete

Description

See #143 for more details. This ticket is sort of a stepping stone to that one, in that this needs to be implemented first.

Attachments

sldRenderer.diff (131.5 kB) - added by ahocevar on 11/04/07 17:10:12.
Implementation of an SLD parser and renderer with tests and example (http://dev.openlayers.org/sandbox/ahocevar/sldRenderer/examples/gml-tasmania-sld.html)
sldRenderer.2.diff (151.1 kB) - added by ahocevar on 11/06/07 18:29:10.
new version of the patch, see http://trac.openlayers.org/ticket/533#comment:6 for details.
sld.diff (97.8 kB) - added by ahocevar on 12/15/07 14:12:53.
patch with tests and examples
sld-newapi.diff (98.3 kB) - added by ahocevar on 12/15/07 17:05:43.
Alternative patch using the new API propsed by crschmidt in #1154. All tests pass in FF and IE.
sld.patch (98.4 kB) - added by tschaub on 12/19/07 19:09:50.
different style patch (very minor differences)

Change History

06/17/07 10:14:38 changed by crschmidt

  • milestone changed from 2.5 Release to 2.6 Release.

10/16/07 13:19:51 changed by ahocevar

  • keywords set to review.

(follow-up: ↓ 4 ) 10/16/07 17:05:53 changed by ahocevar

Current status of SLD support in the ahocevar sandbox (sldRenderer)

What is new?

* New class OpenLayers.Format.SLD implements read() for reading sld files.

* New class OpenLayers.Style is a container for rule-based styles. The core method createStyle is used to calculate a style based on the feature, its attributes and the map scale.

* New class OpenLayers.Rule is the base class for rule subclasses: Comparison, FeatureId and Logical implement the supported filters of the OGC Filter specification.

* OpenLayers.Layer.Vector has a new setStyle API method. This can be used to apply new styles, even if the layer is already rendered.

* OpenLayers.Layer.Vector and OpenLayers.Control.SelectFeature are the only two files of the existing codebase that needed to be changed. This is because the OpenLayers style hash can still be used, but before applying a style to a feature we check for the CLASS_NAME of the style property to see if it is OpenLayers.Style. If so, we use the createStyle method of the style to calculate the current style according to the feature attributes and map scale

What of SLD is supported?

* Points: ExternalGraphic (bitmaps only) with Size and Opacity

* Lines: Stroke with stroke, stroke-width, stroke-opacity and stroke-linecap

* Polygons: Stroke with stroke, stroke-width, stroke-opacity and stroke-linecap; Fill with fill and fill-opacity

* MinScaleDenominator and MaxScaleDenominator

What of OGC Filters is supported?

* FeatureId filter

* And, Or and Not filters

* Comparison filters: EqualTo, NotEqualTo, LessThan, GreaterThan, LessThanOrEqualTo, GreaterThanOrEqualTo, !Between, !Like

How to use it

1. Read in a sld from a string or dom document with OpenLayers.Format.SLD.read().

1. We now have a hash of OpenLayers.Style, with the sld:Name of the UserStyle as key.

1. If the sld document matches vector layer names in our OpenLayers application via the sld:Name of a NamedLayer and also has one of the UserStyles for this NamedLayer marked with <IsDefault>1</IsDefault>, the whole result hash from OpenLayers.Format.SLD.read() can directly be passed to the style property of the vector layer.

1. In all other cases, one element of the hash retrieved by OpenLayers.Format.SLD.read() can be passed to the style property of the vector layer.

1. For OpenLayers.Control.SelectFeature, the selectStyle property can also hold one element of the Styles hash from OpenLayers.Format.SLD.read().

(in reply to: ↑ 3 ) 10/16/07 17:54:41 changed by ahocevar

Replying to ahocevar:

What of SLD is supported?
* Points: ExternalGraphic (bitmaps only) with Size and Opacity

Forgot to mention that, of course, sld:Mark points with Fill and Stroke attributes are also supported. But the WellKnownName will be ignored, because OpenLayers currently only implements a circle shape for points.

11/04/07 17:10:12 changed by ahocevar

  • attachment sldRenderer.diff added.

Implementation of an SLD parser and renderer with tests and example (http://dev.openlayers.org/sandbox/ahocevar/sldRenderer/examples/gml-tasmania-sld.html)

(follow-up: ↓ 6 ) 11/06/07 07:50:35 changed by elemoine

Hello!

I've started reviewing Andreas' code. My first comments:

  • (line 234) OpenLayers.Format.SLD's parseFilter() is annotated to return a String (line 234). This isn't correct, a rule is returnd by that method.
  • (line 405) The style property of an OpenLayers.Layer.Vector object may cover multiple layers whereas only the part related to that object (layer) is relevant. Wouldn't it make sense to store only what's necessary? By doing so, we could also avoid the lookup done in getDefaultStyle().
  • OpenLayers.Layer.WFS is part of the patch while it's untouched.
  • (line 21) OpenLayers/Rule/Comparison.js: the type of APIProperty type is wrong, should be {<OpenLayers.Rule.Comparison.Type>}
  • On a more general note, I don't like too much the fact that OpenLayers.Layer.Vector's style property can be a hash or an OpenLayers.Style object (instanceof must be used). Wouldn't be possible to use OpenLayers.Style everywhere - OpenLayers.Style looks pretty generic. Maybe you're concerned with backward compatibility here.

I haven't reviewed the tests yet, but will do.

Anyway, thanks for this excellent work Andreas.

(in reply to: ↑ 5 ) 11/06/07 18:27:41 changed by ahocevar

Replying to elemoine: Eric, thank you for reviewing my code! I fixed most of the problems, and I have additional comments for some of them:

* (line 405) The style property of an OpenLayers.Layer.Vector object may cover multiple layers whereas only the part related to that object (layer) is relevant. Wouldn't it make sense to store only what's necessary? By doing so, we could also avoid the lookup done in getDefaultStyle().

You are absolutely right. This was how it was originally implemented. But the magic to determine the correct style that is in SLD (and also in a hash of styles derived from it), seemed too precious to ignore. Anyway, I think I have found a solution: a hash of OpenLayers.Style can no longer be passed as option with the constructor. But the setStyle method can take a hash and determine the correct style, which will then be stored in the style property of the vector layer.

* On a more general note, I don't like too much the fact that OpenLayers.Layer.Vector's style property can be a hash or an OpenLayers.Style object (instanceof must be used). Wouldn't be possible to use OpenLayers.Style everywhere - OpenLayers.Style looks pretty generic. Maybe you're concerned with backward compatibility here.

Yes, backwards compatibility was exactly my concern. I wanted to leave a footprint as small as possible on the existing code base. But you are right, OpenLayers.Style is pretty generic. And it holds a style hash in its defaultStyle and defaultSelectStyle properties that will be used if no rules are defined.

I created a new patch. This one also contains the changes proposed in http://trac.openlayers.org/ticket/1073. Another new feature: style properties in a style object can now have references to feature attributes, acting as a variable, eg.

feature.attributes['labelText'] = 'myLabelText';
feature.style.defaultStyle.label = 'feature labeled ${labelText}';

This label property would evaluate to "feature labelled myLabelText". In SLD, the same could be defined by saying

<sld:Label>feature labelled <ogc:PropertyName>labelText</ogc:PropertyName></sld:Label>

This fancy trick is somewhat beyond the SLD spec, except for labels. But it is convenient, especially when working with style objects directly instead of SLD. The use case of #1073 can be solved without lookup style rules, by simply saying

style.defaultStyle.externalGraphic = "${category}.png";

It does not only work when set for style properties, but also for value and lower/upper boundary of comparison rules and for min/maxScaleDenominator.

This patch is still lacking some tests for the new features, but I provide it anyway for reviewing. Tests will follow soon.

11/06/07 18:29:10 changed by ahocevar

  • attachment sldRenderer.2.diff added.

new version of the patch, see http://trac.openlayers.org/ticket/533#comment:6 for details.

(follow-up: ↓ 8 ) 11/10/07 19:38:05 changed by ahocevar

  • component changed from Layer to Format.

The current version of the patch does not reflect the latest changes, those are only in the ahocevar/sldRenderer sandbox. I still have to create some tests for the new features, and after that I will split up this patch into three separate tickets, because there is more in it than just SLD. I think this will also be easier to review and move into trunk. So if this is ok, I'll split the patch up into the following three parts:

  • Rules Framework (OpenLayers.Rule and subclasses)
  • Styles Framework (OpenLayers.Style, depends on the Rules Framework)
  • SLD Parser (OpenLayers.Format.SLD, depends on the Styles Framework)

The Styles Framework is the only part that has impact on existing code.

Another question, because this was commented by Eric: what do the other developers think about changing all style properties for features, !Control.Select and layers to be OpenLayers.Style instead of the style hash that is now being used?

If that is desired, I would propose to change the style properties of those classes to type OpenLayers.Style. The constructor of OpenLayers.Style can already take a style hash as argument. So the only thing to change in existing applications would be from e.g.

layer.style = myStyleHash;

to

layer.style = new OpenLayers.Style(myStyleHash);

Another thing I would like to discuss is the caching of style/symbolizer attributes that need string format parsing to replace property names with literals (the "foo ${bar}" stuff), which is currently done in OpenLayers.Style.createStyle, populating the propertyStyles property when it is invoked for the first time.

Should I change rules and defaultStyle from API-properties to private properties, and add a !setStyle and a !setRules method (maybe also !addRule)? These methods would then call a common method that does the caching after setting the style/rules.

(in reply to: ↑ 7 ) 11/11/07 07:09:04 changed by elemoine

Replying to ahocevar:

The current version of the patch does not reflect the latest changes, those are only in the ahocevar/sldRenderer sandbox. I still have to create some tests for the new features, and after that I will split up this patch into three separate tickets, because there is more in it than just SLD. I think this will also be easier to review and move into trunk. So if this is ok, I'll split the patch up into the following three parts: * Rules Framework (OpenLayers.Rule and subclasses) * Styles Framework (OpenLayers.Style, depends on the Rules Framework) * SLD Parser (OpenLayers.Format.SLD, depends on the Styles Framework)

Splitting up the patch into these three patches will certainly ease the review. Thanks.

The Styles Framework is the only part that has impact on existing code. Another question, because this was commented by Eric: what do the other developers think about changing all style properties for features, !Control.Select and layers to be OpenLayers.Style instead of the style hash that is now being used? If that is desired, I would propose to change the style properties of those classes to type OpenLayers.Style. The constructor of OpenLayers.Style can already take a style hash as argument. So the only thing to change in existing applications would be from e.g. {{{ layer.style = myStyleHash; }}} to {{{ layer.style = new OpenLayers.Style(myStyleHash); }}}

I'm not sure breaking compatibility is an option before 3.0.

What could be done though is use OpenLayers.Style everywhere internally, and translate old-style style objects passed by the user to OpenLayers.Style objects. Then, for 3.0, we'll just remove these translations and force the user to use OpenLayers.Style.

My opinion here.

-- Eric

11/16/07 15:18:43 changed by ahocevar

  • keywords deleted.

I removed the "review" keyword because I do not want anyone to waste time looking at code that is work in progress. The patches for this ticket are outdated, and I am not satisfied with the way the styles are applied to features before calling drawFeature(). If anyone is interested and wants to help thinking (or even working on that), I'll gladly explain the existing work.

What I do consider ready though are the following classes:

11/16/07 19:02:43 changed by ahocevar

For limitations of the OpenLayers.Format.* classes, and what would be nice to have in order to improve OpenLayers.Format.SLD, see http://trac.openlayers.org/ticket/1154

12/01/07 07:04:20 changed by ahocevar

  • priority changed from minor to major.

Attached is a patch that provides OpenLayers.Format.SLD to read SLD files and uses the styles framework (#1183) for display.

The patch depends on #1183.

12/01/07 14:28:26 changed by ahocevar

  • keywords set to review.

12/15/07 11:35:48 changed by ahocevar

Maybe a small API change of OpenLayers.Format.SLD is necessary, depending on the result of the discussion in #1154.

12/15/07 14:12:53 changed by ahocevar

  • attachment sld.diff added.

patch with tests and examples

12/15/07 17:05:43 changed by ahocevar

  • attachment sld-newapi.diff added.

Alternative patch using the new API propsed by crschmidt in #1154. All tests pass in FF and IE.

12/19/07 19:09:50 changed by tschaub

  • attachment sld.patch added.

different style patch (very minor differences)

12/19/07 19:20:38 changed by tschaub

  • summary changed from Style vector layers with SLD to Add SLD format.

This is great work! Andreas - thank you for all your work and especially your patience with us finally getting around to this.

I don't have any real substantive comments. I only tweaked the patch for a few reasons:

  • your previous one didn't actually add the files - they were represented in the patch in a different way than I had seen (I just did an svn diff > sld.patch on a modified version of the trunk) - probably some patch option I don't know about to get it to add when patching
  • I added one semicolon at the end of a line
  • I changed your extend to applyDefaults only because it read better to me - nothing functional
  • meaningless whitespace changes that I have the bad habit of making while I read

I did notice one thing that deserves your feedback - but since it all works as is, I'm inclined to get this in. In parseUserStyle you return [] if there are no rules. I assume you mean to simply add no rules to the userStyle that you're putting together. Don't know how this applies in real life, but it looks to me like callers of parseUserStyle expect to get an object back (not an array). I think I'll just open another ticket for this.

I also see we've got some work to do with selection. This was something we knew already. I think with this format in the trunk, we'll have better reason to do selection and feature style right.

Thanks again Andreas.

12/19/07 19:24:21 changed by tschaub

  • status changed from new to closed.
  • state changed from Review to Complete.
  • resolution set to fixed.

(In [5520]) Big thanks to Andreas Hocevar for this SLD format class - this continues to extend his Rule and Style work by giving us the ability to read SLD docs - onward with style r=tschaub (closes #533).

12/20/07 12:44:37 changed by tschaub

Ignore my comment above about the patch adding files. Can't remember what I was thinking - patch was fine.