you can't render a geometry

[tschaub]

Ok, this will likely end up just being a documentation patch, but I want to eventually get us out of this geometry.feature dependence. Perhaps geometries will always have a reference to feature - I'd like to have this be an undocumented part of the API for now.

So, I'm going to add jsdoc @private stuff where I think it's appropriate. Don't want this to hold up 2.4, but I also don't want it to have to wait for 3.0.

[sderle]

what was the justification for having a geometry.feature property in the first place?

[crschmidt]

We use it in the SelectFeature control:

Control/SelectFeature.js:89: if(OpenLayers.Util.indexOf(this.layer.selectedFeatures, geometry.feature) > -1) { Control/SelectFeature.js:95: if(OpenLayers.Util.indexOf(this.layer.selectedFeatures, geometry.feature) > -1) { Control/SelectFeature.js:120: if(!(OpenLayers.Util.indexOf(this.layer.selectedFeatures, geometry.feature) > -1)) { Control/SelectFeature.js:147: if(geometry.feature.originalStyle == null) { Control/SelectFeature.js:148: geometry.feature.originalStyle = geometry.feature.style; Control/SelectFeature.js:150: this.layer.selectedFeatures.push(geometry.feature); Control/SelectFeature.js:162: if(geometry.feature.originalStyle == null) { Control/SelectFeature.js:163: geometry.feature.originalStyle = geometry.feature.style; Control/SelectFeature.js:165: this.layer.renderer.drawGeometry(geometry, geometry.feature.originalStyle); Control/SelectFeature.js:166: OpenLayers.Util.removeItem(this.layer.selectedFeatures, geometry.feature);

[tschaub]

just to get feedback

[tschaub]

Just want to make sure this sounds like the right direction. For 2.4, renderers will still have drawGeometry and geometry.feature will still exist. Nobody else should know this.

If you want to render a feature, do layer.renderFeature(feature) and give an optional style arg if you want.

Also, vector layers get the default style when instantiated.

If this sounds reasonable, I'll continue with the handlers, a few controls, and other examples that rely on the old way.

[tschaub]

conceal geometry.feature

[tschaub]

Ok, the gist of this is that the supported way to render a feature is

layer.drawFeature(feature); // with an optional style arg

This replaces constructs like

layer.renderer.drawGeometry(feature.geometry, feature.style);

Also, if you find the need to use geometry.feature, you're probably doing something wrong. This sort of referencing is limited to handlers and renderers at this point. If you're writing a control or customizing an application, you should stick with geometry, feature, and feature.geometry.

Other included bits:

1) feature._setGeometryFeatureReference() has been removed in favor of recursive calls to feature.setGeometry()

2) tests have been added to show that feature.setGeometry() works

3) the lock/unlock editing stuff has been removed from Layer.Vector

[tschaub]

Ok, don't think anybody is looking at this right now - so I'm removing the review request. I've got a couple other ideas I'd like to try...

[tschaub]

This one does it. With this patch, geometry.feature is gone. The only reference to drawGeometry is in the renderers, otherwise, it's all drawFeature.

Scrolling through the patch:

1) examples - mostly replacing things like

    feature.layer.renderer.drawGeometry(feature.geometry, feature.style);

with

    feature.layer.drawFeature(feature);

2) Control/SelectFeature.js now deals in features instead of geometries - all references to geometry.feature replaced with feature.

3) Feature/Vector.js had a couple setters that were not used and useless. In particular, the recursive setGeometry was only used to set geometry.feature.

4) Format/GML.js and KML.js now set geometry directly

5) Geometry.js - lose feature property

6) Handler/Feature.js now deals exclusively in features

7) Handler/Point.js, Path.js, and Polygon.js - all deal in features instead of geometries (with the exception of the callbacks to the control - these get geometries because that's what they deserve [no style or attributes in this context])

8) Layer/Vector.js - the editing flags are irrelevant - the layer.renderer.reproject() call did the exact same thing that the loop below does (draw each feature), so this has been removed - a vector layer now has a drawFeature method, this is the appropriate place to do drawing, instead of layer.renderer.drawSomething() - also layer.getFeatureFromEvent() is new, more on this below

9) Renderer.js and subclasses - the "public" method of interest here is drawFeature(), called by the layer - renderer.drawGeometry() is very private - if you are not a renderer and you are calling drawGeometry(), you are doing something wrong - nodes no longer have expando properties like geometry (circular references that lead to memory leaks) - nodes now know about things like _featureId - this is returned to the layer when getFeatureIdFromEvent is called - the layer finds the corresponding feature (since it already has a collection of those)

10) More on renderers - the two places that node.geometry was required were reprojectNode and setStyle for circles (VML) - reprojectNode was not required, and setStyle is now called with the geometry it needs

That's it. 10 small things. And some tests.

Examples and tests can be run from  http://dev.openlayers.org/sandbox/tschaub/feature

[tschaub]

end of geometry.feature

[tschaub]

please review

[crschmidt]

Tim:

Applied, updated to trunk, re-diffed, reuploaded. This code is good, the change is smart, and thank you for your diligence on this. Please apply to trunk and send email to the dev list informing them of methods which are going away so that people know to switch away from them, then mark pullup.

[tschaub]

reolved with r3043

[crschmidt]

Brought up to 2.4 branch for RC2 with r3088