fix: point cloud picking precision issue #2508
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pramodcog
added
auto-update
Codecov Report
@@ Coverage Diff @@
## master #2508 +/- ##
==========================================
+ Coverage 70.15% 70.16% +0.01%
==========================================
Files 215 215
Lines 9243 9243
Branches 1203 1203
==========================================
+ Hits 6484 6485 +1
+ Misses 2611 2610 -1
Partials 148 148
|
christjt
approved these changes
Oct 5, 2022
There was a problem hiding this comment.
Think this is fine for now, but down the road I would like this to be a little smarter. Having a larger window size is good in cases where users miss-click. I think a better schema is to always pick the center pixel, unless there is a pixel in the window that is a lot closer (by some relative threshold). Then we get the best of both worlds.
pramodcog
deleted the
pramodcog/fix-pointcloud-pixel-intersection-precision
branch
christjt
pushed a commit
that referenced
this pull request
Nov 2, 2022
Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>
christjt
added a commit
that referenced
this pull request
Nov 3, 2022
…ch (#2635) * chore: add workflow that deploys to NPM on Github release (#2586) * test: initial dry run publish * chore: trigger release on released published * fix: remove caching on release workflow Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com> * fix: expose and fix bug causing getBoundingBoxByNodeId/TreeIndex not to modify out parameter (#2472) * refact: keyboard and mouse event handling in viewer (#2492) * Intermediate commit * Removed isFocused check in keyboard handling * Keyboard active only which domElement is focused, mouse events are active when canvas is focused * Refactored axisview eventlistener * Reverted axisview event passed to domElement * updated examples yarn.lock * clean up * leftover clean up * Keyboard event listen on domElement, removed unnecessary event push to domElement from Axis cross * removed unused variables * clean up * Fixed visual test error Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com> * fixed point cloud picking precision issue (#2508) Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com> * improvement: use 'high-performance' powerPreference to avoid non-discrete GPU being used on certain systems (#2512) * feat: add metrics to MeasurementTool (#2523) * chore: backport to old file structure * fix: remove flat modifier, while maintaining TreeIndex precision (#2536) Contribution by @Strepto (Equinor) * fix: do not blend in-front and back frame buffers when back objects have not been rendered (#2540) Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com> * fix: visualization artifacts due to wrongfully handling cylinder clip planes (#2556) * fix: simplify shader * fix: set proper plane length * fix: reduce some matrix transformations, and use built-in face-forward * fix: plane magnitude not adjusting for scaled world transform * fix: bad merge * chore: revert CDF environments json * chore: add primitive test fixture Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com> * improvement: make CadModelUpdateHandler trigger sector loading immediately after the camera stops moving (#2573) * Make CadModelUpdateHandler trigger sector loading immediately after the camera stops moving * Remove setting of redraw flag * Update test * Fix test Co-authored-by: Christopher J. Tannum <[email protected]> * Use vec2 instead of struct for packing TreeIndexes (#2585) This works around an issue with struct precision on ardeno gpus: KhronosGroup/WebGL#3351 * fix: general cylinder inside rendering (#2620) * fix: rendering if camera is inside primitive * fix: reverse normal direction if hitting inside of cylinder * fix: enable depth writing for in-front pass (#2621) * Enable depth writing for in-front pass * Set depth to minimum of in-front and back pass Co-authored-by: Christopher J. Tannum <[email protected]> * fix: remove unused pcMaterialManager from bad merge * fix: re-create v8 test image due to cylinder clipping planes fix Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com> Co-authored-by: Lars Moastuen <[email protected]> Co-authored-by: Pramod S <[email protected]> Co-authored-by: Nils Henrik Hals <[email protected]> Co-authored-by: eiriklegernaes <[email protected]> Co-authored-by: Håkon Flatval <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Precision error in pixel intersection in point cloud was caused due to
PickingWindowSizewas higher20, reduced the value to5pixel to bring the precision error REV-523While identifying the cause of the issue, I found some of the calculation done in
PointCloudOctreePickerHelper.findHit()are not needed. We should have a detail look into the potree library if need to refactor it for our needChecklist:
Here is a checklist that should completed before merging this given feature.
Any shortcomings from the items below should be explained and detailed within the contents of this PR.