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

fix(remote rendering): round view size #69

Merged

Conversation

bourdaisj
Copy link
Contributor

It seems like vue-vtk-js uses Math.round for the view size. For instance: https://github.com/Kitware/vue-vtk-js/blob/master/src/core/VtkRemoteView.js#L218

So it seems like I went into issues with the view size on the server not matching the view size on the browser, which could trigger an infinite loop in vue-vtk-js, requesting an new render then throwing it away over and over again: https://github.com/Kitware/vue-vtk-js/blob/master/src/core/VtkRemoteView.js#L234

The fix proposed here is to update the paraview protocol to comply with vue-vtk-js vtkRemoteView implementation which uses Math.round for setting view size. I used it for a few days and my problem did not ever happen again.
How does that look? @jourdain
Should I update the VTK protocol the same way?

@jourdain
Copy link
Collaborator

yes please. Thx that seems reasonable.

@bourdaisj
Copy link
Contributor Author

will do asap.

@bourdaisj bourdaisj force-pushed the round-view-size-instead-of-int-conversion branch from a0dd18b to dfb0487 Compare May 2, 2024 08:47
@bourdaisj bourdaisj requested a review from jourdain May 2, 2024 08:48
comply with vue-vtk-js vtkRemoteView implementation which uses Math.round for
setting view size. python builtin round function does not work the same
way: it rounds half to even.
@bourdaisj bourdaisj force-pushed the round-view-size-instead-of-int-conversion branch from dfb0487 to 0a72916 Compare May 2, 2024 08:48
@bourdaisj
Copy link
Contributor Author

@jourdain please review.
I stopped using Python round because it rounds half to even while JS Math.round rounds half up.

@jourdain jourdain merged commit e6777b9 into Kitware:master May 2, 2024
3 checks passed
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.

None yet

2 participants