Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework handling of skipped features in replay #1990

Merged
merged 1 commit into from Apr 18, 2014
Merged

Rework handling of skipped features in replay #1990

merged 1 commit into from Apr 18, 2014

Conversation

elemoine
Copy link
Member

There is a bug in the handling of skipped features in the canvas replay when replaying the hit detection instructions. This PR aims at fixing the bug.

Description of the issue:

Currently the canvas replay uses the instructionIndices_ object to store instruction index/feature uid pairs and be able to get the feature uid for an (begin geometry) instruction and know when a feature/geometry should be skipped. See https://github.com/openlayers/ol3/blob/master/src/ol/render/canvas/canvasreplay.js#L256-L261.

But this doesn't work when replaying the batch for feature detection. For feature detection the replay indeed uses a specific instruction arrays (hitDetectionInstructions), so the instruction indices stored in instructionIndices_ are not valid for hit detection. So with the current code, when replaying the instructions for hit detection, features that should not be skipped may be skipped, and features that should be skipped may not be skipped.

The problem can easily be reproduced with just three point features in a vector layer and a select interaction. It's much more difficult to reproduce when the vector layer has many features (because you must be unlucky enough to find a problematic pair of features).

This PR fixes the bug by removing the instructionIndices_ object entirely and storing the references to features in the begin geometry instructions. This simplifies the code and makes "skip feature" work with hit detection as well.

@tschaub
Copy link
Member

tschaub commented Apr 16, 2014

A description of the bug would be nice (when you get a chance).

@elemoine
Copy link
Member Author

Yes, I just wanted to quickly push my work and open this PR. I will write a proper description of the issue and how my patch addresses it.

@elemoine elemoine changed the title [WIP] Rework handling of skipped features in replay Rework handling of skipped features in replay Apr 16, 2014
@elemoine
Copy link
Member Author

Description updated. Ready for review.

@elemoine
Copy link
Member Author

Please review. This is a major bug that we need fixed in master.

@tschaub
Copy link
Member

tschaub commented Apr 18, 2014

This looks like a nice fix to me. Please merge.

@elemoine
Copy link
Member Author

Thanks for the review.

elemoine pushed a commit that referenced this pull request Apr 18, 2014
Rework handling of skipped features in replay
@elemoine elemoine merged commit 0521749 into openlayers:master Apr 18, 2014
@elemoine elemoine deleted the select-bug branch April 18, 2014 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants