DSEGOG-128 Image centroids/crosshairs #521
Conversation
Just basic API hookup & crosshair drawing for now Had to refactor some of the canvas code to reduce blur
- Had to slightly change the chart.js code to be able to load multiple plots in a window
- Also make it so the plot is sized correctly when the image loads
…viour & hide plots when being redrawn
…de to only be set when the image size is known
…useImageCrosshairs
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #521 +/- ##
===========================================
- Coverage 97.90% 97.86% -0.05%
===========================================
Files 93 94 +1
Lines 12108 12531 +423
Branches 1974 2025 +51
===========================================
+ Hits 11854 12263 +409
- Misses 252 264 +12
- Partials 2 4 +2 ☔ View full report in Codecov by Sentry. |
- fix some assertions which weren't actually retrying due to await in wrong place - add some extra assertions to help checking if images change
src/images/imagePlot.component.tsx
Outdated
| width: imageElement?.naturalWidth | ||
| ? imageElement.naturalWidth + 29 | ||
| : 200, | ||
| height: 200, | ||
| } | ||
| : { | ||
| width: 200, | ||
| height: imageElement?.naturalHeight | ||
| ? imageElement.naturalHeight + 22 | ||
| : 200, |
There was a problem hiding this comment.
Why does the natural width and height need 29 and 22 added ?
There was a problem hiding this comment.
Ah, I should add a comment for this, but I needed to add some padding to account for the width/height of the Chart.js axis ticks (aka the numbers) so that the actual axis line itself aligns with the image.
Maybe I can try and calculate it based on the default config variables in Chart.js - but they may just have to remain magic numbers...
I will at least extract both out to variables at the top of the file (to make them easy to edit if we need) and explain what they're for
|
Should an exported images also include the crosshairs ?? |
|
@joshuadkitenge no it shouldn't (I think - they're not exported in eCat) - if they do it's an oversight on my part (although I wouldn't think they would as I would have thought we download the raw image itself, not the canvas overlay where the crosshairs are drawn...) |
- this helps reduce blurriness when moving between displays with different devicePixelRatio
…re reliable after the responsive: true change
- the calculation stuff was unreliable across browsers, instead just use the constants we know currently work and rely on our snapshot testing to highlight any issues if defaults change in the future
- fix alignment issues caused by difference between 16-bit & 8-bit images
|
@joshuadkitenge this is ready for review again as Stephen got back to me on how to handle 16-bit images. He picked the option where we let our plotting library autoscale the plot. See below example of a 16-bit image with intensity plots activated
|
|
@joshuadkitenge I forgot to add spacing to the corresponding grid for the top row. I can't get it to align perfectly with the control panel (as the horizontal spacing of the right-hand graph kind of depends on the scale of the image pixel axis) but it aligns with the numbers on the other graph now
|
|
That looks good. |
Description
Adds the ability to go into "crosshairs" mode. When switching into this mode, it will display the centroid for the image and the corresponding intensity plots. The user can click on the image to reposition the crosshairs. Resetting the view resets back to the centroid.
It should work when switching via the thumbnail selector, we always default to the centroid though.
Testing instructions
Add a set up instructions describing how the reviewer should test the code
Agile board tracking
Closes DSEGOG-128