Picking refactor

[IanLilleyT]

Fixes #7247

Moved most of the picking functions and their helper functions out of Scene.js and into a new file called Picking.js, a transfer of about 800 lines of code. Since Scene.js exposes the public api for picking, Scene.js is simply a passthrough for all picking functions. Tests, linter, and sandcastle demos are all working. Hopefully nothing slipped through the cracks. No new tests or public facing changes were made.

Some thoughts:

• Scene.js documentation lists the kinds of errors each function can trigger, but the actual handling happens in Picking.js. This could be good if other areas of code besides Scene.js call picking functions, but it makes the Scene.js documentation a bit confusing.
• updateMostDetailedRayPick and getPickDepth are the only two functions in Picking.js that are called from Scene.js but are not public api functions. Should these kinds of functions stay in Picking.js? Also, should updateMostDetailedRayPicks go in Scene.js or Picking.js?
• I was tempted to put all scratch variables in the same area of code in Picking.js, but decided against it because Scene.js split them up to be near the functions they are used. The thing is, some of these scratch variables are used in more than one function so their placement in code may appear random at times.

[cesium-concierge]

Thanks for the pull request @IanLilleyT!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[lilleyse]

One idea to have less coupling between Picking.js and Scene underscored member variables is to have Picking be a class instead of a collection of static functions. Things like _mostDetailedRayPicks, _pickPositionCache, _pickPositionCacheDirty, _pickOffscreenView, etc would move to underscored member variables in Picking, and Scene would contain a member variable called picking (or some better name now). Make sure that Picking has a destroy function and that it is called in Scene's destructor. Ideally Picking would not reference any underscored properties from Scene. Some properties like context and frameState already have getters/setters that you can use, but some new ones may have to be added (view and others).

Scene.js documentation lists the kinds of errors each function can trigger, but the actual handling happens in Picking.js. This could be good if other areas of code besides Scene.js call picking functions, but it makes the Scene.js documentation a bit confusing.

I don't think there's any other choice because the errors need to be listed in the Scene documentation.

updateMostDetailedRayPick and getPickDepth are the only two functions in Picking.js that are called from Scene.js but are not public api functions. Should these kinds of functions stay in Picking.js? Also, should updateMostDetailedRayPicks go in Scene.js or Picking.js?

I think it's fine to move more stuff into Picking including updateMostDetailedRayPicks.

I was tempted to put all scratch variables in the same area of code in Picking.js, but decided against it because Scene.js split them up to be near the functions they are used. The thing is, some of these scratch variables are used in more than one function so their placement in code may appear random at times.

It might make sense to create new variables to avoid confusion, with better names to disambiguate them.

[IanLilleyT]

@lilleyse ready for review

[lilleyse]

The updated code looks good. All the Sandcastle examples that are related to picking are still working normally.