-
Notifications
You must be signed in to change notification settings - Fork 0
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
In 1108 improve cesium picking with non gpu buffer tree #4
base: main
Are you sure you want to change the base?
In 1108 improve cesium picking with non gpu buffer tree #4
Conversation
🔴 There was an error checking the CLA! If this is your first contribution, please send in a Contributor License Agreement.
|
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.
I think this looks pretty good! just two lines of dead code to remove and i think it'll be okay
however, don't merge just yet! We're sitting pretty with this branch, and we'll merge up to date cesium into this later (once I figure out how to upgrade cesium without crashing everything)
box[3] = this._boxENU[3]; | ||
box[4] = this._boxENU[4]; | ||
box[5] = this._boxENU[5]; | ||
// bfsPrint(indexedTree, positions, box); |
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.
remove this dead code
const p = partition(positions, sortedPnts, 0, numberOfPoints - 1, 0); | ||
|
||
quicksort(positions, sortedPnts, 0, p, numberOfPoints - 1, 1, tree); | ||
return [tree, sortedPnts[p], boundingBox]; |
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.
I'd prefer this to be a ES4 Javascript object:
function Tree3D(positions) {
...
this._tree = tree;
this._startNode = sortedPnts[p];
this._boundingBox = boundingBox;
}
...
let tree = new Tree3D(...);
tree._tree; // etc
* @param {Float64Array} v2, the second point as [x2, y2, z2] | ||
* @returns {number} the distance between the two points | ||
*/ | ||
function distance(v1, v2) { |
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.
*/ | ||
function normalize(v) { | ||
const newVector = new Float64Array(3); | ||
const denom = Math.sqrt(v[0] ** 2 + v[1] ** 2 + v[2] ** 2); |
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.
Description
Issue number and link
Testing plan
Author checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change