feat: RGB support #41
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #41 +/- ##
==========================================
- Coverage 77.90% 77.87% -0.04%
==========================================
Files 13 13
Lines 1856 1885 +29
==========================================
+ Hits 1446 1468 +22
- Misses 410 417 +7 ☔ View full report in Codecov by Sentry. |
src/ndv/viewer/_components.py
Outdated
| self.setChecked(mode == ChannelMode.MONO) | ||
| super().__init__(parent, enum_class=ChannelMode) | ||
|
|
||
| def enable_rgb(self, enable: bool) -> None: |
There was a problem hiding this comment.
I would be happy to hear if the reviewers have any better ideas here - this seemed like solution that added the fewest lines of code.
|
Converting this back into a draft because additional changes are needed to support smooth transitions between modes |
|
I'm much happier with this after a couple of changes, however I've built it on top of #40, so that PR must be merged first |
|
merged #40, can you resolve conflicts? |
Sure! |
gselzer
force-pushed
the
rgb-support
branch
2 times, most recently
from
7e6e69f to
ba216c6
Compare
|
@tlambert03 conflicts resolved! |
|
how did i miss it 🤦♂️ ... sorry :) |
src/ndv/viewer/_viewer.py
Outdated
| self.set_visualized_dims(visualized_dims) | ||
|
|
||
| is_rgb = (self._channel_axis is not None) and (sizes[self._channel_axis] == 3) |
There was a problem hiding this comment.
we also need to allow a size of 4 for rgba
There was a problem hiding this comment.
also, just curious to discuss what we intend to imply by having "rgb" available here in the combo box. I guess it means "we could show this to you as rgb if you want"... and perhaps that's ok. But let's briefly discuss the case of something like a three-channel fluorescence image (3, Y, X). It's true that we could show you that as RGB, but chances are it won't look reasonable if it's not an 8bit file (since channel scaling would be way off), whereas something that comes in as (Y, X, 3) does imply something different about the input. In either case, it's harmless to show the rgb option in the dropdown, but it may not be useful. worth discussing
There was a problem hiding this comment.
also, just curious to discuss what we intend to imply by having "rgb" available here in the combo box.
Yeah, the thought was that a 3-channel image may or may not be RGB, and it would be wrong to assume such. If the user does not specify the mode, then 3-channel images still appear with ChannelMode.MONO. But then if you want to view in RGB, you can do so, as the option is now available in the combo box.
But let's briefly discuss the case of something like a three-channel fluorescence image
(3, Y, X). It's true that we could show you that as RGB, but chances are it won't look reasonable if it's not an 8bit file (since channel scaling would be way off),
Yup, this was the reasoning behind ChannelMode.MONO remaining the default.
whereas something that comes in as
(Y, X, 3)does imply something different about the input.
I'm unsure if this was explicitly mentioned, but passing data in either (Y, X, 3) or (3, Y, X) should enable RGB viewing i.e. if you have a channel axis (discovered using existing logic) with 3 indices, RGB mode is enabled (and we can add 4 indices if desired).
By and large, we wanted to avoid making assumptions about the data, enabling an RGB mode wherever possible without assuming that the user wants it.
There was a problem hiding this comment.
alpha support has been added with 339607c, and (Y, X, 3)/(Y, X, 4) images are now displayed as RGB(A) when the channel mode is auto. Notably, (3, Y, X) images are not.
gselzer
force-pushed
the
rgb-support
branch
from
19f3741 to
7e07137
Compare
|
@tlambert03 added a WIP commit fleshing out alpha support. Couple questions:
|
|
Oh, and one more discussion point @tlambert03: These
|
played around with this a bit... i feel like setting channel mode second "feels" more correct (since it's more likely to be changed in the course of viewing). does this work for you? gselzer#3 |
I took the liberty of renaming it! |
|
This is all looking good and we can finish it up this week, I was thinking that the main thing to fix that race condition is probably just to remove the conditional check for existing image handles (there's little harm in calling refresh anyway, and it will cancel any waiting futures anyway). So, that, combined with putting the call to set channel mode after set data mode in the init should be good I hope |
|
in e1ae37c I removed the conditional checks before calling |
I cannot see either issue now! No slider, and I'm seeing an rgba image! Great! |
|
seems that: # run rgb example then...
In [15]: wdg.set_data(ndv.data.cells3d())
In [16]: wdg.set_data(img)does show img as rgb, but composite is still selected in the combobox. (sidenote: cells3d is shown as noise ... which is weird, but probably unrelated here) |
|
@tlambert03 I'd really like to squash all these commits together soon, as the history is rather messy, and do one final review. This script is helpful for testing: import math
import numpy
import ndv
from qtpy.QtWidgets import QApplication, QPushButton
# 2D RGB dataset
rgb = numpy.zeros((256, 256, 4), dtype=numpy.uint8)
for x in range(256):
for y in range(256):
rgb[x, y, 0] = x
rgb[x, y, 1] = y
rgb[x, y, 2] = 255 - x
rgb[x, y, 3] = int(math.sqrt((x - 128) ** 2 + (y - 128) ** 2))
# 4D ZCYX dataset
cells = ndv.data.cells3d()
# Button to switch between them
def switch(checked: bool) -> None:
n.set_data(rgb if checked else cells)
app = QApplication([])
btn = QPushButton("Switch image")
btn.setCheckable(True)
btn.toggled.connect(switch)
# Run it!
n = ndv.NDViewer()
n._btns.addWidget(btn)
n.show()
app.exec() |
gselzer
force-pushed
the
rgb-support
branch
9 times, most recently
from
a364d4f to
165978f
Compare
For provenance, VisPy allows RGB data to come from unsigned integers and floating point data pre-nomalized within the range [0, 1].
It was only used once, and by inlining the functionality earlier on we can save some lines of code later
This PR aims to enable the viewing of RGB datasets. Users specify that their provided data is RGB using the new
ChannelModevalue. The current mechanism enables this mode when the channel axis (specified through existing means) has exactly three indices.To enable the selection of different modes, the
ChannelModeButtonis refactored into aChannelModeComboBox, which can be altered based on the data to contain thergbmode.This PR requires review, and likely additional minor cleanup.
Closes #20