Fix screenshots (again) & update dependencies #346
Conversation
|
@annehaley it's looking good. Did you verify that reasonable things happen for non-square images? I believe we want minimal black bars to keep it as 512x512, but another reasonable solution is to not have the bars and make sure one of the dimensions is 512 and the other is <= 512. I don't remember where this feature landed in the past, do you recall @dzenanz? |
Thanks, Jeff! From my testing, We do fill out the rest of the space with a black background when the image slice is not square. Although, the long dimension is not perfectly 512 in this case; we get a bit of black margin. Here's an example:
Additionally, if the image is saved as png instead of jpeg, the background is transparent (not black) |
|
Is there is a way to ensure there are only black bars in the horizontal or vertical dimensions, not both? Do you know what is causing that? |
|
@jeffbaumes A way to ensure no black bars is to extract a slice from the image. As MRIs are mostly 512 and lower resolution (384, 256, 224, 192) along the bigger side, that would ensure no unnecessarily big images. I am not sure how to accomplish that using vtk.js. We would need the axis of slicing and the current index. If we don't need to upsample the smaller images, that should be relatively easy. |
|
The side ones are because of the scale adjustment we do. We can make the image be the full width or height if we wanted. |
|
Just substract 1 to the scale here |
I tried this subtraction, but the same padding appears on the edges. I'm not sure what causes this. |
|
@jourdain and I attempted to fix this scaling problem that causes this no-content margin around the edge of the screenshot, but the math appears sound and a solution without magic numbers is unclear. We resolved to place a |
* Update vtk.js and other dependencies * Lint fixes on most vue files according to eslint update * Incorporate upstream changes to place camera correctly * Remove comments * TODO for fixing scale later
* Update vtk.js and other dependencies * Lint fixes on most vue files according to eslint update * Incorporate upstream changes to place camera correctly * Remove comments * TODO for fixing scale later
This PR is the result of working with @jourdain to ensure that the screenshot fix previously merged also considers the following:
These two considerations were the reasons behind the use of the extra three hidden "Screenshot2DView"s, which were removed in #335.
@jourdain and I agreed that using the existing three views in the proxyManager is still a cleaner solution than adding an extra hidden "Screenshot2DView"s. The considerations above can still be accomplished using the existing three views. This just requires a newer version of vtk.js, as this change had been requested of vtk.js when the problem was first discovered. Perhaps the three additional views were added as a quick fix while waiting for the upstream changes? Regardless, we still needed one additional upstream change in vtk.js that Sebastien was able to do this evening, so this truly means we need the latest version.
We were fairly behind on our version (we had 14.16.5, latest is now 22.5.7), so updating this dependency required changing others in order to compile. And, inspired by today's D&A meeting, I updated some more that were raising deprecation warnings and vulnerabilities. Among these was eslint, so a lot of Vue component files were reformatted with the updated rules.
Thus, this ended up being a pretty large PR. Functional changes for the screenshot fix are actually in just the third commit.