Select interaction ignores layer filter for feature overlays #2940

Closed
ssaarela opened this Issue Nov 12, 2014 · 9 comments

Comments

Projects
None yet
3 participants
@ssaarela

Map.forEachFeatureAtPixel is called for all features of a feature overlay regardless of layer filter. I have a couple of layers containing different kind of editable information and use Draw (polygons) for one and Select/Modify (points) for another. I try to target Select to the layer containing the relevant points with layers option, but as Draw uses feature overlay, Select grabs those polygons immediately.

Would it be possible to use layerFilter also in the function(feature, layer) -argument of the map.forEachFeatureAtPixel? One can always supply a filter that accepts null-layer, but without it I see no way to skip feature overlays.


Want to back this issue? Place a bounty on it! We accept bounties via Bountysource.

@elemoine

This comment has been minimized.

Show comment
Hide comment
@elemoine

elemoine Nov 13, 2014

Member

The draw interaction uses a FeatureOverlay when actually drawing. If you use a specific layer/source as where the draw interaction persists the drawn features you will be filter out that layer/source from the selection.

Member

elemoine commented Nov 13, 2014

The draw interaction uses a FeatureOverlay when actually drawing. If you use a specific layer/source as where the draw interaction persists the drawn features you will be filter out that layer/source from the selection.

@ssaarela

This comment has been minimized.

Show comment
Hide comment
@ssaarela

ssaarela Nov 13, 2014

Currently if you have Select and Draw interactions on a map, Select grabs/selects unfinished features that are just being drawn... there might be a use case for that, but as a non-overridable default... I'm tempted to call it a bug.

I'm not suggesting that you modify the Map.forEachFeatureAtPixel - although that might also make sense as you can always make a filter that accepts "null layer" - but the callback function given to it by Select interaction. That callback function(feature, layer) uses array.indexOf to check if given feature is already selected or not, i.e. if it is on Select's own FeatureOverlay. As the layer parameter is null for features from a FeatureOverlay, you could optimise that check to happen only for overlay features. For not-yet-selected features, I think, it would make sense to use given layerFilter_ so that it (Select) would not unintentionally grab features from other interactions' internal FeatureOverlays.

Currently if you have Select and Draw interactions on a map, Select grabs/selects unfinished features that are just being drawn... there might be a use case for that, but as a non-overridable default... I'm tempted to call it a bug.

I'm not suggesting that you modify the Map.forEachFeatureAtPixel - although that might also make sense as you can always make a filter that accepts "null layer" - but the callback function given to it by Select interaction. That callback function(feature, layer) uses array.indexOf to check if given feature is already selected or not, i.e. if it is on Select's own FeatureOverlay. As the layer parameter is null for features from a FeatureOverlay, you could optimise that check to happen only for overlay features. For not-yet-selected features, I think, it would make sense to use given layerFilter_ so that it (Select) would not unintentionally grab features from other interactions' internal FeatureOverlays.

@elemoine

This comment has been minimized.

Show comment
Hide comment
@elemoine

elemoine Dec 28, 2014

Member

A PR would be ideal to understand what you need/suggest. Thanks.

Member

elemoine commented Dec 28, 2014

A PR would be ideal to understand what you need/suggest. Thanks.

@pgiraud

This comment has been minimized.

Show comment
Hide comment
@pgiraud

pgiraud Sep 14, 2015

Member

I've written an example that may illustrate the issue.
http://jsfiddle.net/pgiraud/axmbruf5/

In this example, we have two layers that are "unmanaged". The select interaction is configured with a layers option which should limit the selection to only one layers. However, one is able to select features from both layers.
The layers filter seems to be correctly taken into account when the layers are added to the map (ie. managed).

Member

pgiraud commented Sep 14, 2015

I've written an example that may illustrate the issue.
http://jsfiddle.net/pgiraud/axmbruf5/

In this example, we have two layers that are "unmanaged". The select interaction is configured with a layers option which should limit the selection to only one layers. However, one is able to select features from both layers.
The layers filter seems to be correctly taken into account when the layers are added to the map (ie. managed).

@pgiraud

This comment has been minimized.

Show comment
Hide comment
@pgiraud

pgiraud Sep 16, 2015

Member

In the tests (https://github.com/openlayers/ol3/blob/master/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js#L49), it's clearly written that forEachFeatureAtPixel will always return features from the unmanaged layers.

1 - It should be documented in the API doc.
2 - Can someone explain this choice?

Member

pgiraud commented Sep 16, 2015

In the tests (https://github.com/openlayers/ol3/blob/master/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js#L49), it's clearly written that forEachFeatureAtPixel will always return features from the unmanaged layers.

1 - It should be documented in the API doc.
2 - Can someone explain this choice?

@pgiraud pgiraud referenced this issue in Geoportail-Luxembourg/geoportailv3 Sep 21, 2015

Merged

[WIP] Adding support for drawing tools #864

@elemoine

This comment has been minimized.

Show comment
Hide comment
@elemoine

elemoine Sep 21, 2015

Member

#3883 and #3878 are relevant.

And ol.layer.Layer#setMap includes docs about this already. See https://github.com/openlayers/ol3/blob/master/src/ol/layer/layer.js#L150-L161.

Member

elemoine commented Sep 21, 2015

#3883 and #3878 are relevant.

And ol.layer.Layer#setMap includes docs about this already. See https://github.com/openlayers/ol3/blob/master/src/ol/layer/layer.js#L150-L161.

@pgiraud

This comment has been minimized.

Show comment
Hide comment
@pgiraud

pgiraud Sep 21, 2015

Member

OK. Thanks.

Member

pgiraud commented Sep 21, 2015

OK. Thanks.

@pgiraud pgiraud closed this Sep 21, 2015

@elemoine

This comment has been minimized.

Show comment
Hide comment
@elemoine

elemoine Sep 21, 2015

Member

@pgiraud So I guess the problem is related to the Select interaction itself using an unmanaged layer for drawing selected features. If that unmanaged layer is filtered out then the selected features won't be returned by forEachFeatureAtPixel, making it impossible to unselect them. But there may be a better way to handle this. Instead of ignoring the layer filter for unmanaged layers we could make the Select interaction combine the user-provided layer filter with its own layer filter, in an OR operation.

Member

elemoine commented Sep 21, 2015

@pgiraud So I guess the problem is related to the Select interaction itself using an unmanaged layer for drawing selected features. If that unmanaged layer is filtered out then the selected features won't be returned by forEachFeatureAtPixel, making it impossible to unselect them. But there may be a better way to handle this. Instead of ignoring the layer filter for unmanaged layers we could make the Select interaction combine the user-provided layer filter with its own layer filter, in an OR operation.

@elemoine

This comment has been minimized.

Show comment
Hide comment
Member

elemoine commented Sep 21, 2015

See #4143.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment