-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Scene.pickPosition in 2D/CV #4990
Conversation
Source/Scene/Scene.js
Outdated
* @returns {Cartesian3} The cartesian position. | ||
* | ||
* @exception {DeveloperError} Picking from the depth buffer is not supported. Check pickPositionSupported. | ||
* @exception {DeveloperError} 2D is not supported. An orthographic projection matrix is not invertible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This exception is not longer valid, right?
Source/Scene/Scene.js
Outdated
* @exception {DeveloperError} 2D is not supported. An orthographic projection matrix is not invertible. | ||
*/ | ||
Scene.prototype.pickPosition = function(windowPosition, result) { | ||
Scene.prototype._pickPosition = function(windowPosition, result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it would be more clear to name this pickPosition3D
and make it @private
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or whatever a more precise name is.
@pjcozzi This is ready for another look. |
Do you get this test failure?
|
I was not seeing any failures. I changed the test to check the values returned instead of that it was defined. I updated the epsilon values so they should pass now. |
Very nice. |
Adds pick position support in 2D and Columbus view. Fixes #4084.