Skip to content
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

Allow batch reading of mesh chunks #7001

Merged
merged 35 commits into from
May 8, 2023
Merged

Conversation

frcroth
Copy link
Member

@frcroth frcroth commented Apr 21, 2023

URL of deployed dev instance (used for testing):

Steps to test:

  • open l4dense_motta_et_al_dev_v2 in view mode
  • enable mapping 80
  • load a precomputed mesh for a segment (e.g., at 2905, 4566, 1786)

TODO

  • Adapt front end
  • Use new format on other route
  • Tweak performance so that it generalizes over different mesh files
  • [ ] Use uint32 instead of uint64 for jump table so that Uint32Array can be used in JS
  • ensure proper error handling when decoding fails (independent of this PR, strictly speaking)
  • debug why some requests cannot be decoded (independent of this PR, strictly speaking)

Issues:


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

@frcroth frcroth changed the title Implement reading of mesh chunks with list input WIP: Implement reading of mesh chunks with list input Apr 21, 2023
@frcroth
Copy link
Member Author

frcroth commented Apr 24, 2023

@philippotto Backend should be ready (though not completely tested)

@philippotto philippotto changed the title WIP: Implement reading of mesh chunks with list input Allow batch reading of mesh chunks Apr 28, 2023
@philippotto
Copy link
Member

I just noticed that THREE.DRACOLoader: Unexpected geometry type. is thrown on the dev instance for some chunk (most of the mesh loads fine). This is already the case on master, too. See the testing steps in the PR description which trigger this.

The front-end doesn't handle this correctly currently (the spinner just keeps spinning). I'll add proper error handling and then we should be able to debug this further.

@philippotto philippotto marked this pull request as ready for review May 2, 2023 16:53
@philippotto philippotto requested review from daniel-wer and fm3 May 2, 2023 16:53
@philippotto
Copy link
Member

I think, the PR is ready for review. @daniel-wer It can make sense to review the PR by commit range (I did some refactoring of the isosurface saga were I simply extracted/moved some code). Also, I upgraded the draco libs.

@frcroth @fm3 See my previous comment regarding an error where a mesh chunk cannot be decoded. I added better error handling and reporting, but I couldn't really figure out what's happening. I only found one segment where one mesh request fails and that is:

Requested chunk: {position: [1024, 5120, 1024], byteOffset: 3920109208, byteSize: 1818}
Error: {type: 'error', id: 9592, error: 'THREE.DRACOLoader: Unexpected geometry type.'}

It happens with l4dense_motta_et_al_dev_v2/segmentation/meshes/meshfile_mapping0.hdf5 with mesh id 355 and mapping 80. The mesh file is 13 GB big..

Maybe you have an idea what could be causing this. However, I think, it's not blocking this PR as this is already an issue on master.

@philippotto
Copy link
Member

Bug can be seen isolated on this dev instance: https://provokemeshdecodingerror.webknossos.xyz/datasets/sample_organization/l4dense_motta_et_al_dev_v2/view#2816,4353,1792,0,1.3 (enable mapping 80 and load a mesh).

Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me :) Great speed increase! I added a note on parallelization.

@philippotto could you write an issue for the draco decoding problem?

@philippotto
Copy link
Member

@philippotto could you write an issue for the draco decoding problem?

Done! #7044

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works very well, awesome speed boost!

CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
public/wasm/draco_decoder.wasm Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/model/sagas/isosurface_saga.ts Outdated Show resolved Hide resolved
public/wasm/draco_wasm_wrapper.js Outdated Show resolved Hide resolved
@philippotto
Copy link
Member

@frcroth just a small ping in case this PR slipped through your inbox :)

@frcroth
Copy link
Member Author

frcroth commented May 5, 2023

@frcroth just a small ping in case this PR slipped through your inbox :)

@philippotto I'm a bit out of the loop on this one. What should I do?

@philippotto
Copy link
Member

@frcroth just a small ping in case this PR slipped through your inbox :)

@philippotto I'm a bit out of the loop on this one. What should I do?

Only the PR feedback was missing, but I guess this is good now 👍 @fm3 Do you want to leave a final approval?

Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boom 🚀

@philippotto philippotto merged commit 58bce4e into master May 8, 2023
@philippotto philippotto deleted the optimize-pre-meshes-overseg branch May 8, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize loading of precomputed meshes of oversegmentation
4 participants