webknossos icon indicating copy to clipboard operation
webknossos copied to clipboard

[Hackathon] Faster On-Hover Raycasting in 3D viewport

Open daniel-wer opened this issue 1 year ago • 1 comments

Hackathon 2024 project

  • Use BVH optimized raycaster
  • Merge geometries of a single (agglomerate) mesh into one geometry, saving render passes
  • Don’t create groups for these meshes, so a single BVH is built per agglomerate mesh

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • abc

TODOs:

  • [ ] Restore 3D Viewport Proofreading
  • [ ] Mesh Colors changed after threeJS update

Issues:

  • fixes #7832

(Please delete unneeded items, merge only when none are left open)

  • [ ] Updated changelog
  • [ ] Updated migration guide if applicable
  • [ ] Updated documentation if applicable
  • [ ] Adapted wk-libs python client if relevant API parts change
  • [ ] Removed dev-only changes like prints and application.conf edits
  • [ ] Considered common edge cases
  • [ ] Needs datastore update after deployment

daniel-wer avatar Oct 01 '24 08:10 daniel-wer

Thanks for updating the title and PR description @fm3 :)

I'll continue with restoring the 3D viewport proofreading using vertex colors once I'm back in office. I'll also have a look at the washed out colors then.

daniel-wer avatar Oct 03 '24 14:10 daniel-wer

📝 Walkthrough

Walkthrough

This pull request introduces widespread modifications across geometry utilities, mesh controllers, view components, and configuration files. Key updates include enhanced tangent computation using MikkTSpace, asynchronous BVH processing via a new web worker module, refined raycasting and color handling in controllers and views, and the removal of deprecated parameters related to mesh merging. Dependency upgrades and cleanup of debounce functionality complement these changes.

Changes

Files Change Summary
.../libs/BufferGeometryUtils.ts, .../libs/compute_bvh_async.ts, .../libs/ThreeDMap.ts Added new geometry utility functions: introduced computeMikkTSpaceTangents, asynchronous BVH computation via a web worker, and minor comment adjustments.
.../oxalis/controller/{segment_mesh_controller.ts, td_controller.tsx, camera_controller.ts, mesh_helpers.ts, scene_controller.ts}, .../oxalis/api/wk_dev.ts Enhanced controllers and API with new methods (benchmarkRotate, optional onComplete callback), refined raycasting hit-handling and color management, and removed obsolete scene grouping properties.
.../oxalis/model/{sagas/mesh_saga.ts, actions/{segmentation_actions.ts, annotation_actions.ts}, reducers/annotation_reducer.ts}, .../oxalis/model/sagas/proofread_saga.ts, .../oxalis/store.ts Removed mergeChunks/areChunksMerged parameters and properties from mesh-loading flows, annotation actions, reducers, and sagas to simplify control logic.
.../oxalis/view/{left-border-tabs/mapping_settings_view.tsx, plane_view.ts, context_menu.tsx, right-border-tabs/{trees_tab/skeleton_tab_view.tsx, segments_tab/{segment_list_item.tsx, segments_view.tsx}} Updated view components by removing debounce wrappers, refining raycaster hit structures and deletion conditions, and shifting from static to dynamic multi-select menu retrieval.
package.json, webpack.config.js, CHANGELOG.unreleased.md, .../test/helpers/apiHelpers.ts Upgraded dependencies (added three-mesh-bvh, updated webpack-related packages), removed deprecated packages like react-debounce-render, expanded webpack warning ignores, and added test mocks and changelog notes.

Assessment against linked issues

Objective Addressed Explanation
Speed up raycasting with BVH (#7832)

Suggested labels

enhancement

Poem

I hopped through functions with a joyful leap,
Merging geometries and tangents to keep.
BVH work humming on a worker thread,
Speeding raycasts like magic ahead.
In this code garden my heart finds delight—
A rabbit’s cheer for changes so bright!

✨ Finishing Touches
  • [ ] 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Mar 10 '25 10:03 coderabbitai[bot]

During testing I noticed that it is possible to hover a mesh even when the rendered viewports are in the way (my mouse is not rendered in the screenshot :/): image

But some testing showed me that this is also the case on the master. Do you think it makes sense to make the rendered viewport planes "blocking" the raycasting in case they are configured? Would be a candidate for a new issue imo.

Besides this, I noticed that rendering precomputed meshes is broken in the current state. They shortly flicker into being rendered for a few frames when loaded and afterwards they are no longer rendered :ghost:

MichaelBuessemeyer avatar Mar 28 '25 13:03 MichaelBuessemeyer

@MichaelBuessemeyer Thank you for your valuable feedback! And sorry about the whole todop comments. I must have mistakenly thought that I already self-reviewed the PR when I in fact did not. Please have another look. I incorporated/commented on your feedback :)

philippotto avatar Apr 04 '25 12:04 philippotto

But I am terribly sorry: I found another bug that looks pretty tedious to debug. Here is the video of me reproducing this:

Ok turns out, this is already buggy on the current master :/ So I'd say this isn't a must fix before merging this PR :+1:

But I did some more testing an ended up with a mesh in a rendered state that does not support highlighting on hover :thinking:

https://github.com/user-attachments/assets/41d77856-fefd-4d53-9eb7-e2ece95f1239

I found 2 strange behaviour out of which the 1st is known to me: When the user loads a ad-hoc mesh & precomputed for the same user, the hovering is off, as both meshes exist in the scene.

The second one is that the mesh is rendered in the scene but now hoverable (see at the end of the video)

But here as well: I am not sure whether this PR is responsible for this behaviour. IMO this shouldn't block this PR as the reproduction steps are a bit of try an error loading the same mesh as ad-hoc / precomputed over and over again and deleting it. So this strange behaviour shouldn't really impact our users.

What do you think @philippotto I would vote for creating an issue for this

MichaelBuessemeyer avatar Apr 08 '25 14:04 MichaelBuessemeyer

What do you think @philippotto I would vote for creating an issue for this

I had another look and fixed it (and some other parts, too) hopefully :) thanks for testing thoroughly!

philippotto avatar Apr 08 '25 15:04 philippotto

thanks for having another look! will merge now :)

But the first one still exists :/. But as this is already the case on the master and the opacity feature is new I'd say it is ok if we fix this in a follow up. This shouldn't block this PR imo

yes, I think, that's orthogonal to this PR.

philippotto avatar Apr 09 '25 12:04 philippotto