Skip to content

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Mar 4, 2019

URL of deployed dev instance (used for testing):

Steps to test:

  • open a segmentation in view mode
  • shift click on a cell to render an isosurface
  • shift click on the isosurface in the 3d viewport
  • the position should jump to the clicked position
  • also, there should be hovering "effects" when hovering over isosurfaces

Issues:


@philippotto philippotto self-assigned this Mar 5, 2019
@philippotto philippotto requested a review from daniel-wer March 5, 2019 17:27
@philippotto philippotto marked this pull request as ready for review March 5, 2019 17:27
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.

Very impressive! The hover effect is awesome and makes this feature look and feel very futuristic, great job 👍

Out of interest, how did you solve the problem you had during development, where the hit test did not work all the time? With the clippingOffsetFactor?

const pos = voxelToNm(dataset.dataSource.scale, getPosition(state.flycam));

const aspectRatio = getInputCatcherAspectRatio(state, OrthoViews.TDView);
const clippingOffsetFactor = 100000;
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment explaining why this is needed? :)

);

raycaster.setFromCamera(mouse, this.cameras[OrthoViews.TDView]);
const intersections = raycaster.intersectObjects(isosurfacesRootGroup.children, true);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to intersect recursively? Is this because the segments for one id are in a group or is there another reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's because the segments are in a group and the group is only traversed if recursive is true. I'll add a comment!

}
}

performIsosurfaceHitTest(): ?THREE.Vector3 {
Copy link
Member

Choose a reason for hiding this comment

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

Did you experiment with throttling this function? To me, during testing, the TD view felt a little sluggish and I was wondering whether this could help to make it feel more performant :) I do understand that the "instantness" of the hover effect contributes to the "coolness" of the future and I do want to keep that, so maybe just throttling it a little is enough.
It's not a blocker though as the performance hit is only noticeable when isosurfaces are loaded.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are absolutely right. I definitely needed to add throttling, as it doesn't make sense to compute this on every render. I opted for 150 ms as a throttling delay, which felt like a good trade-off between performance and delay to me.

@philippotto philippotto merged commit 1be5c84 into master Mar 12, 2019
@normanrz normanrz deleted the iso-hittest branch August 12, 2019 09:45
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.

Hit-test on isosurface
2 participants