Added allowPicking to Entity to allow per-entity picking.

[dwhipps]

This relates to issue #2895.

Please note that this is an initial stab at this and probably needs more work. Also note that while jsHint and test-non-webgl pass, for some reason I can never (even on master branch) get the test-webgl tests to complete, so I suspect that several of my new tests have problems.

At this point I'd just like some comments on my implementation, before I spend a lot of time looking further into this.

[hpinkos]

Thanks @dwhipps! I've marked this for the bug bash so we'll be sure to review it then if we don't get a chance to ahead of time

[pjcozzi]

@dwhipps thanks for this nice contribution and all your past contributions.

Someone will need to do a detailed review at the bug bash, but I took a quick look at this and I think it is the right direction. I suggest:

• adding allowPicking to Label since it was added to Billboard and Point.
• changing Label/Billboard/Point private boolean members to use a bitfield since they are starting to get quite large and memory is often an issue. This isn't strictly required for this PR, but we would deeply appreciate it if you have the time.
• @bagnell can you please look at this https://github.com/AnalyticalGraphicsInc/cesium/pull/4410/files#diff-1764c99f9184c0f5fb91649454b53a99R154. Perhaps there is a more obvious and less coupled approach?

[dwhipps]

1. Added 'allowPicking' to Label (this was just an oversight.)
2. I'd be happy to start on this for one or more of them, but I'd really prefer to make it a separate PR so as not to conflate issues. This is a fairly big one and some of the architecture is new to me.
3. I agree that there is some pretty tight coupling there, but I don't see an easy way to decouple without a (much) larger refactor.

[mramato]

Thanks @dwhipps! I plan on reviewing this during the bug bash, but the one major thought I have is that we were originally planning to have a picking priority number instead of a simple boolean (with 0 being disabled). Think of a billboard inside of an ellipsoid. I want to pick the billboard instead of the ellipsoid, but still pick the ellipsoid if I click anywhere else on it.

That being said, I think that feature will either be trivial to do now that you laid all of the ground work OR has some really complicated aspects I haven't thought of which would lead us to just use the pickEnabled boolean that you've already set up.

I don't expect you to try to update the PR to implement pick priority, I'm just thinking out loud. It might be overkill. When I get a chance to look at the code, I'll provide some more concrete thoughts.

Either way, I'm excited for this PR and all of the work you've been doing on Cesium. Thanks again.

[dwhipps]

Matt,

Happy for this to form the basis for pick priority if that's what you want to use it for. I just wanted the ability to do this in my app, and thought I'd contribute it back.

For the record, this still needs to be QA'd. It's passing all tests (as well as my basic testing here) but needs a good once-over before I'd put it out in the wild.

• Dave

[pjcozzi]

2. I'd be happy to start on this for one or more of them, but I'd really prefer to make it a separate PR so as not to conflate issues. This is a fairly big one and some of the architecture is new to me.

A separate PR would be great, thanks!

[pjcozzi]

the one major thought I have is that we were originally planning to have a picking priority number instead of a simple boolean

I believe we would implement this will drill picking: pick all objects, then select the one with the highest priority. There could be an early exit if we know the highest priority or limit the number of depth layers to explore (which I highly suggest).

I don't think this would need the new boolean.

[mramato]

I believe we would implement this will drill picking: pick all objects, then select the one with the highest priority. There could be an early exit if we know the highest priority or limit the number of depth layers to explore (which I highly suggest).

I don't think this would need the new boolean.

Right, that's exactly what I planned on doing, the new boolean would be replaced with a number defaulting to 1, 0 would mean do not pick.

[pjcozzi]

Right, that's exactly what I planned on doing, the new boolean would be replaced with a number defaulting to 1, 0 would mean do not pick.

OK with me.

So this would keep a fast path for pickPriority = 0 for a single pick and then use pickPriority for the drill pick. I think you already said this, but this PR could be merged before priority picking.

[mramato]

but this PR could be merged before priority picking.

Right, my only concern is churn, I'm going to try and prototype the priority picking locally, but at the first sign of trouble, I'll punt to a future pull.

[dwhipps]

@mramato Anything more you want me to do on this one? I feel like this PR is "complete" but not necessarily the implementation you wanted (i.e. "pick priority").

[mramato]

We will most likely take this PR largely as is and save the priority part for a future PR, I just need to carve out the time to actually review the existing code before merging.

[pjcozzi]

@dwhipps we have another bug bash in February. We'll see if someone can review this then.

[hpinkos]

What's the status of this PR @mramato? This would be helpful for a number of our users so it would be great to get this merged

[crubier]

Any news on this ? Would be useful

[hpinkos]

@crubier sorry, we haven't had a chance to revisit this yet

[smlawrence]

It'd be great to see this feature make it through the pipeline. I have manually merged this into my 1.46 local working for Billboards/etc. "out of the box", but the GeometryUpdaters have changed a lot so had to hunt down the where the primitives get created.

Is there an update on when this feature may next be evaluated/reviewed?

[hpinkos]

Hi @smlawrence, sorry no update. We're not quite sure if the is is the method we want to use for this kind of feature and haven't had a chance to dig into it yet. It is high priority though since this is a feature that's requested frequently from our users.

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

[hpinkos]

@mramato do you think we should close this or park it on a branch or what? This PR has been open for 2 years and we need a plan.

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[hinsonan]

Please someone add this feature

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[jimklo]

This is a greatly needed feature. Please implement...

[cesium-concierge]

Thanks again for your contribution @dwhipps!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[hpinkos]

I'm going to close this due to inactivity. Thanks for the work you put into this @dwhipps! This just isn't quite the approach we want and we haven't had a chance to revisit a different strategy. Hopefully this is something we'll get to before too long.

[srcejon]

I'm going to close this due to inactivity. Thanks for the work you put into this @dwhipps! This just isn't quite the approach we want and we haven't had a chance to revisit a different strategy. Hopefully this is something we'll get to before too long.

Was an alternative for this implemented?