feat(picker): Add volume picker to CellPicker

[rodrigobasilio2022]

Context

This PRs adds volume picker capabilities to Cellpicker Object. I recorded a video of the functionality working.

Gravando.2023-10-26.123920.mp4

Results

Changes

• Documentation and TypeScript definitions were updated to match those changes

PR and Code Checklist

• semantic-release commit messages
• Run npm run reformat to have correctly formatted code

Testing

• This change adds or fixes unit tests
• Tested environment:
  • vtk.js:
  • OS:
  • Browser:

[floryst]

CI is failing due to old node versions. I will update it.

[rodrigobasilio2022]

Ok. Thanks. I tried to make fewer changes so you can review effectively. Do you want me to create a video?

[floryst]

If you have an example, that would be a great addition to demonstrate its capabilities.

One note I had skimming your changes is that the package.json changes are irrelevant to the functionality at hand. If the build works then I'm fine with the upgrades.

[rodrigobasilio2022]

THey are irrelevant. I wrongly executed npm install --force, that s it. I will make a video an put in the comments

[sedghi]

is this PR related to #2885 ?

[floryst]

is this PR related to #2885 ?

That appears to be a separate effort that hasn't been updated.

[finetjul]

It's not clear why you had to modify package.json / package-lock.json

Please update CellPicker documentation accordingly.

[rodrigobasilio2022]

Can you guys help me in git? I think I am doing a mess here. I want to delete the modifications I did in the package.json files

[finetjul]

Can you please also update index.d.ts files accordingly ?

[finetjul]

Can you guys help me in git? I think I am doing a mess here. I want to delete the modifications I did in the package.json files

You will be able to squash all your branch commits eventually (I personnally use git rebase -i to do so)

[rodrigobasilio2022]

Added the example. Something i noticed is why you guys do not use the clipLine information in the following lines...

vtk-js/Sources/Rendering/Core/CellPicker/index.js

Line 180 in d61d2e4

const clipLine = clipLineWithPlane(

[finetjul]

Added the example. Something i noticed is why you guys do not use the clipLine information in the following lines...

vtk-js/Sources/Rendering/Core/CellPicker/index.js

Line 180 in d61d2e4

const clipLine = clipLineWithPlane(

Not sure what do you mean, we do use clipLine afterwards

[rodrigobasilio2022]

The t1 and t2 updated values that restricts the ray

[finetjul]

The t1 and t2 updated values that restricts the ray

I see, looks like a bug indeed. t1 and t2 should be extracted from the returned value...

[finetjul]

LGTM

[floryst]

CI is red because this PR has some tests still failing. Please run npm test locally to identify the issue.

[rodrigobasilio2022]

I run the npm run test command and this was the final output

My machine is a Windows 11

[rodrigobasilio2022]

I executed the command again. In fact was executed yarn run test, and that was the output

[finetjul]

It looks like a test is hanging (that's why you do not see any failure)

[rodrigobasilio2022]

How do I solve this problem?

[rodrigobasilio2022]

Found the problem. It was a point picker function which not correspond with the changes in vtkPicker

[rodrigobasilio2022]

@finetjul I nned to do something else? I saw the PR was pulled out from merge queue, despite no conflict with the base branch

[floryst]

Can you rebase locally onto an updated master to see if there are any conflicts? Github recently had operational issues, so I don't know if there is an actual conflict or if the merge queues are currently broken.

[rodrigobasilio2022]

@floryst Made the change. It had one commit out from the base branch, despite no warnings in this PR. Please try again...

[rodrigobasilio2022]

@floryst @finetjul Now i think i got it right

[finetjul]

LGTM

[rodrigobasilio2022]

@finetjul I think the branch is ok right. It already passed theses tests?

[github-actions]

🎉 This PR is included in version 29.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀