Skip to content
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a08e507
[vv-like] Dynamically adjust colormap to cursor position
Mar 25, 2025
46ce9e5
[vv-like] Zoom in and out with i/o keys.
Mar 25, 2025
2ebee34
Revert "[vv-like] Zoom in and out with i/o keys."
Mar 26, 2025
3d7e541
Revert "[vv-like] Dynamically adjust colormap to cursor position"
Mar 26, 2025
fbc7b08
Merge branch 'main' into vvlike
Apr 14, 2025
c4974f7
bootstrap new mode: update dynamically colormap according to mouse po…
payno Apr 14, 2025
27949b1
Merge remote-tracking branch 'origin/fix_4263' into vvlike
Apr 14, 2025
90be74d
Dynamicl Colormap: draft implementation of a seperate mode
Apr 14, 2025
caade84
Remove prints, use math.combo.min_max
Apr 14, 2025
ef931e2
Remove prints.
Apr 14, 2025
f37f2ba
Move most of the code the a testable (static) function.
Apr 14, 2025
bd73b36
Add a static function to compute new colormap
Apr 15, 2025
41ec4ee
Fix propagation of interactive mode to plot widget
Apr 15, 2025
0b9db3a
Replace numpy.max/min with max/min
Apr 15, 2025
cbfc436
Add simple test on activatation of the correct mode and check min max…
Apr 15, 2025
d7176f4
Remove prints...
Apr 15, 2025
9a79361
Add shirtcuts to the 3 interaction modes W:Dynamic cmap, P: pan, Z: z…
Apr 15, 2025
c385029
Update src/silx/gui/plot/PlotInteraction.py
lesaintjerome Apr 16, 2025
bc10794
Update src/silx/gui/plot/PlotInteraction.py
lesaintjerome Apr 16, 2025
2fe6fca
Update src/silx/gui/plot/PlotInteraction.py
lesaintjerome Apr 16, 2025
eccb4a1
Update src/silx/gui/plot/PlotInteraction.py
lesaintjerome Apr 16, 2025
2846eea
Add doc to the dynamic_colormap example
Apr 16, 2025
3309167
Complete doc of example
Apr 16, 2025
1ad842a
Make compute_vmin_vmax protected
Apr 17, 2025
308b49d
Review by HP
Apr 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions examples/dynamic_colormap.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import numpy
from silx.gui import qt
from silx.gui.plot import Plot2D


def main():
app = qt.QApplication([])

# Create the ad hoc plot widget and change its default colormap
x = numpy.zeros((100, 100),dtype=numpy.float32)
x[:50,:50] = numpy.random.randn(50,50)
x[:50,50:] = 10 * numpy.random.randn(50,50)
x[50:,:50] = 100 * numpy.random.randn(50,50)
x[50:,50:] = 5 * numpy.random.randn(50,50)

example = Plot2D()
example.addImage(x)
example.show()

app.exec()


if __name__ == "__main__":
main()
Comment on lines +42 to +60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is nothing specific about dynamic colormap in this code sample, I would remove it.

11 changes: 11 additions & 0 deletions src/silx/gui/dialog/ColormapDialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,17 @@ def __init__(self, parent=None, title="Colormap Dialog"):
self._handleScaleToSelectionToggled, type=qt.Qt.QueuedConnection
)

# dynamic colormap
self._dynamicColormap = qt.QPushButton(self)
self._dynamicColormap.setCheckable(True)
self._dynamicColormap.setEnabled(False)
self._dynamicColormap.setText("Dynamic colormap")
self._dynamicColormap.setIcon(icons.getQIcon("add-shape-rectangle"))
self._dynamicColormap.setCheckable(True)
# self._dynamicColormap.toggled.connect(
# self._handleDynamicColormap, type=qt.Qt.QueuedConnection
# )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this is unused at the moment. It should be either removed or fully integrated on the ColormapDialog. As a reminder the ColormapDialog is this widget:

image

IMO better to remove this code first and have the processing working as a PlotAction

Note: in case you want to integrate it 'setCheckable` seems repeated.
'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I remove it completely since I see no point in having a checkbox in this dialog which does nothing but what the icon in the widget toolbar does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think removing is the best. But indeed those are covering two 'different' uses cases / concept.

  1. The QAction
    Each PlotWindow (like Plot2D) can define a set of action tunable by the users. By default not all are here (and yours might not be part of the default one - to be discussed).
    But anyway everyone defining PlotWindow can define which action are present on the toolbar or not.
  2. The ColorbarDialog
    Well this widget is common to everyone and everyone will access to all the options for now. IMO at the end we should find your option. But again it can come in a second step. As we might also want to define the shape; the radius...

# define modal buttons
types = qt.QDialogButtonBox.Ok | qt.QDialogButtonBox.Cancel
self._buttonsModal = qt.QDialogButtonBox(parent=self)
Expand Down
85 changes: 83 additions & 2 deletions src/silx/gui/plot/PlotInteraction.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
from typing import NamedTuple

from silx.gui import qt
from silx.math.combo import min_max

from .. import colors
from . import items
from .Interaction import (
Expand Down Expand Up @@ -1643,6 +1645,72 @@ def endDrag(self, startPos, endPos, btn):
return super().endDrag(startPos, endPos, btn)


class DynamicColormapMode(ItemsInteraction):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PlotInteraction.py is not the place for "specific" interaction (i.e., that depends on specific items), and it's best to not add features in this module since it would benefit from a large refactoring.
I'll look for an alternative.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I understand that this interaction only aims 2d plots. Then, its place could be in the ImageToolbar. In the same time, it is really an interactive mode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is an interaction mode so independent of the ImageToolbar.
I'll look at it an propose a way to structure this

"""This mode automatically adjusts the colormap range of the image
based on a 20x20 ROI centered on the current cursor position.

:param plot: The Plot to which this interaction is attached
"""

ROI_SIZE = (10, 10) # (y,x). The ROI <<radius>>

@staticmethod
def compute_vmin_vmax(data, dataPos):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider moving this API from public to protected API ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. By this, you simply mean renaming it _compute_vmin_vmax() right?

roi_size = DynamicColormapMode.ROI_SIZE
"""Compute the min and max values of the data in a ROI centered on (x,y)"""
idx_x, idx_y = int(dataPos[0]), int(dataPos[1])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't take care of origin and scale of the Image item

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. But it is not incompatible. In the worst case, it does not improve the local contrast. The user can choose either a specific color scale or to use that feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image can have an offset from origin and a scaling so the coordinates of the plot dataPos cannot be converted directly to indices in the image array.
As it is, the local contrast will be computed on a wrong area of the image if this image has an offset or scale.

This should be handled and it should not be an issue to do so

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You retrieve the data from the an item upper (on the stack).
I think this data is an instance of ImageData. If I m right you should have all your need there.

x_start = max((0, idx_x - roi_size[1]))
x_end = min((idx_x + roi_size[1], data.shape[1]))
y_start = max((0, idx_y - roi_size[0]))
y_end = min((idx_y + roi_size[0], data.shape[0]))

data_values = data[y_start:y_end, x_start:x_end]
vmin, vmax = min_max(data_values)
bb_x = (x_start, x_start, x_end, x_end)
bb_y = (y_start, y_end, y_end, y_start)
return vmin, vmax, bb_x, bb_y

def handleEvent(self, eventName, *args, **kwargs):

super().handleEvent(eventName, *args, **kwargs)

try:
x, y = args[:2]
except ValueError:
return
Comment on lines +1680 to +1681
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the typical use case of this exception ?
Should we log this exception some how ?
Note: a comment might be enought


# Get data
result = self.plot._pickTopMost(x, y, lambda i: isinstance(i, items.ImageBase))
if result is None:
return
else:
item = result.getItem()
Comment on lines +1687 to +1688
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else:
item = result.getItem()
item = result.getItem()

colormap = item.getColormap()
dataPos = self.plot.pixelToData(x, y)
data = item.getData()

# Extract ROI min and max
vmin, vmax, bb_x, bb_y = self.compute_vmin_vmax(data, dataPos)

# Add a blue rectangle that shows the ROI
self.plot.addShape(
bb_x,
bb_y,
legend="ColorMap reference",
replace=False,
fill=False,
color="blue",
gapcolor=None,
linestyle="--",
overlay=True,
z=1,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a part where we will clearly need @t20100 expertise. Is there is convention here to display those kind of information ?

)

# Set new min and max
colormap.setVRange(vmin, vmax)
item.setColormap(colormap)
Comment on lines +1710 to +1712
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misslead you sorry, indeed the update will be applied even if 'item.setColormap(colormap)' is not called. I had the old object in mind my bad 🙏

Suggested change
# Set new min and max
colormap.setVRange(vmin, vmax)
item.setColormap(colormap)
# Set new min and max
colormap.setVRange(vmin, vmax)



# Interaction mode control ####################################################

# Mapping of draw modes: event handler
Expand Down Expand Up @@ -1795,6 +1863,9 @@ def _getInteractiveMode(self):
elif isinstance(self._eventHandler, PanAndSelect):
return {"mode": "pan"}

elif isinstance(self._eventHandler, DynamicColormapMode):
return {"mode": "dynamic_colormap"}

Comment on lines +1867 to +1869
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid to add a "generic" interaction for a specific case

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I get what is behind the 'generic' interaction. Do you suggest that we have some kind of interaction subset / categories ?
Because at the moment we need a specific interaction mode to avoid conflict with other modes.

else:
return {"mode": "select"}

Expand Down Expand Up @@ -1824,8 +1895,14 @@ def _setInteractiveMode(
:param str label: Only for 'draw' mode.
:param float width: Width of the pencil. Only for draw pencil mode.
"""
assert mode in ("draw", "pan", "select", "select-draw", "zoom")

assert mode in (
"draw",
"pan",
"select",
"select-draw",
"zoom",
"dynamic_colormap",
)
plotWidget = self.parent()
assert plotWidget is not None

Expand All @@ -1848,6 +1925,10 @@ def _setInteractiveMode(
self._eventHandler = ZoomAndSelect(plotWidget, color)
self._eventHandler.zoomEnabledAxes = self.getZoomEnabledAxes()

elif mode == "dynamic_colormap":
self._eventHandler.cancel()
self._eventHandler = DynamicColormapMode(plotWidget)

else: # Default mode: interaction with plot objects
# Ignores color, shape and label
self._eventHandler.cancel()
Expand Down
19 changes: 16 additions & 3 deletions src/silx/gui/plot/PlotWidget.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,13 @@ class PlotWidget(qt.QMainWindow):
It provides the source as passed to :meth:`setInteractiveMode`.
"""

# sigDynamicColormapModeChanged = qt.Signal(object)
# """
# Signal emitted when the dynamic colormap changed

# It provides the source as passed to :meth:`setInteractiveMode`.
# """

Comment on lines +335 to +341
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# sigDynamicColormapModeChanged = qt.Signal(object)
# """
# Signal emitted when the dynamic colormap changed
# It provides the source as passed to :meth:`setInteractiveMode`.
# """

sigItemAdded = qt.Signal(items.Item)
"""Signal emitted when an item was just added to the plot

Expand Down Expand Up @@ -3667,7 +3674,7 @@ def getInteractiveMode(self):
"""Returns the current interactive mode as a dict.

The returned dict contains at least the key 'mode'.
Mode can be: 'draw', 'pan', 'select', 'select-draw', 'zoom'.
Mode can be: 'draw', 'pan', 'select', 'select-draw', 'zoom', 'dynamic_colormap'.
It can also contains extra keys (e.g., 'color') specific to a mode
as provided to :meth:`setInteractiveMode`.
"""
Expand Down Expand Up @@ -3695,7 +3702,7 @@ def setInteractiveMode(
"""Switch the interactive mode.

:param mode: The name of the interactive mode.
In 'draw', 'pan', 'select', 'select-draw', 'zoom'.
In 'draw', 'pan', 'select', 'select-draw', 'zoom', 'dynamic_colormap'.
:param color: Only for 'draw' and 'zoom' modes.
Color to use for drawing selection area. Default black.
:type color: Color description: The name as a str or
Expand All @@ -3719,7 +3726,7 @@ def setInteractiveMode(
finally:
self.__isInteractionSignalForwarded = True

if mode in ["pan", "zoom"]:
if mode in ["pan", "zoom", "dynamic_colormap"]:
self._previousDefaultMode = mode, zoomOnWheel

self.notify("interactiveModeChanged", source=source)
Expand Down Expand Up @@ -3785,6 +3792,12 @@ def keyPressEvent(self, event):
# that even if mouse didn't move on the screen, it moved relative
# to the plotted data.
self.__simulateMouseMove()
elif key == qt.Qt.Key_W:
self.setInteractiveMode("dynamic_colormap")
elif key == qt.Qt.Key_P:
self.setInteractiveMode("pan")
elif key == qt.Qt.Key_Z:
self.setInteractiveMode("zoom")
Comment on lines +3795 to +3800
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keyboard shortcut could be added to the PR description. It could be useful to get them back quickly... and to discuss them.
On this PR I would only propose a shortcut for the 'dynamic_colormap'. Others can be propose on dedicated PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we want to add this to PlotWidget, at least without a review of all keyboard shortcuts + it's easy to add in an application

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the point, but it's really a key feature to me: to make it light... : in that respect, adding the icon in the colormap widget would not make much sense IMO. But maybe it is independent where the icon is and how to activate the interaction mode...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But maybe it is independent where the icon is and how to activate the interaction mode...

Yes!

it's really a key feature to me

Sure, but this doesn't mean it is a key feature for other usage of PlotWidget (e.g., when plotting curves). That's why I propose to start by having this shortcut in your application rather than as a default in PlotWidget

else:
# Only call base class implementation when key is not handled.
# See QWidget.keyPressEvent for details.
Expand Down
34 changes: 34 additions & 0 deletions src/silx/gui/plot/actions/mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,37 @@ def _actionTriggered(self, checked=False):
plot = self.plot
if plot is not None:
plot.setInteractiveMode("pan", source=self)


class DynamicColormapAction(PlotAction):
"""QAction controlling the colormap mode of a :class:`.PlotWidget`.

:param plot: :class:`.PlotWidget` instance on which to operate
:param parent: See :class:`QAction`
"""

def __init__(self, plot, parent=None):
super().__init__(
plot,
icon="dynamic_colormap", # TODO: add a dedicated icon
text="Dynamic Colormap mode",
tooltip="Update the colormap according to the mouse position in the plot",
triggered=self._actionTriggered,
checkable=True,
parent=parent,
)
# Listen to mode change
self.plot.sigInteractiveModeChanged.connect(self._modeChanged)
# Init the state
self._modeChanged(None)

def _modeChanged(self, source):
modeDict = self.plot.getInteractiveMode()
old = self.blockSignals(True)
self.setChecked(modeDict["mode"] == "dynamic_colormap")
self.blockSignals(old)

def _actionTriggered(self, checked=False):
plot = self.plot
if plot is not None:
plot.setInteractiveMode("dynamic_colormap", source=self)
29 changes: 28 additions & 1 deletion src/silx/gui/plot/test/test_plotinteraction.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
__date__ = "01/09/2017"

import pytest

import numpy
from silx.gui import qt
from silx.gui.plot import PlotWidget
from .utils import PlotWidgetTestCase
from silx.gui.plot.PlotInteraction import DynamicColormapMode



class _SignalDump:
Expand All @@ -49,6 +51,31 @@ def received(self):
return list(self._received)


class TestSelectDynamicColormap():

def test_evaluate_roi(self):
"""Test evaluate_roi() method"""
plot = PlotWidget()
plot.setInteractiveMode("dynamic_colormap", shape="rectangle", label="test")

interaction = plot.getInteractiveMode()
assert interaction["mode"] == "dynamic_colormap"
assert isinstance(plot.interaction()._eventHandler, DynamicColormapMode)

# Test with a rectangle
roi = numpy.arange(484).reshape((22,22))
vmin, vmax, bb_x, bb_y = DynamicColormapMode.compute_vmin_vmax(roi,(11,11))
assert vmin == 23 and vmax == 460 and bb_x == (1,1,21,21) and bb_y == (1,21,21,1)

roi = numpy.arange(484).reshape((22,22))
vmin, vmax, bb_x, bb_y = DynamicColormapMode.compute_vmin_vmax(roi,(0,0))
assert vmin == 0 and vmax == 207 and bb_x == (0,0,10,10) and bb_y == (0,10,10,0)

roi = numpy.arange(484).reshape((22,22))
vmin, vmax, bb_x, bb_y = DynamicColormapMode.compute_vmin_vmax(roi,(21,21))
assert vmin == 253 and vmax == 483 and bb_x == (11,11,22,22) and bb_y == (11,22,22,11)
Comment on lines 54 to 78
Copy link
Member Author

@payno payno Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new test must be done with pytest instead of unittest.
And maybe it could be split into two tests instead of one.

So more something like

def test_dynamic_colormap_interaction(qapp):
        plot = PlotWidget()
        plot.setInteractiveMode("dynamic_colormap", shape="rectangle", label="test")

        interaction = plot.getInteractiveMode()
        assert interaction["mode"] == "dynamic_colormap"
        assert isinstance(plot.interaction()._eventHandler, DynamicColormapMode)

def test_dynamic_colormap_vmin_vmax_calculation(qapp):
        roi = numpy.arange(484).reshape((22,22))
        vmin, vmax, bb_x, bb_y = DynamicColormapMode.compute_vmin_vmax(roi,(11,11))
        assert vmin == 23 and vmax == 460 and bb_x == (1,1,21,21) and bb_y == (1,21,21,1)

        roi = numpy.arange(484).reshape((22,22))
        vmin, vmax, bb_x, bb_y = DynamicColormapMode.compute_vmin_vmax(roi,(0,0))
        assert vmin == 0 and vmax == 207 and bb_x == (0,0,10,10) and bb_y == (0,10,10,0)

        roi = numpy.arange(484).reshape((22,22))
        vmin, vmax, bb_x, bb_y = DynamicColormapMode.compute_vmin_vmax(roi,(21,21))
        assert vmin == 253 and vmax == 483 and bb_x == (11,11,22,22) and bb_y == (11,22,22,11)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand the comment on unittest vs. pytest. OK for the split. DOne.



class TestSelectPolygon(PlotWidgetTestCase):
"""Test polygon selection interaction"""

Expand Down
8 changes: 8 additions & 0 deletions src/silx/gui/plot/tools/toolbars.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ def __init__(self, parent=None, plot=None, title="Plot Interaction"):
self._panModeAction = actions.mode.PanModeAction(parent=self, plot=plot)
self.addAction(self._panModeAction)

self._dynamicColormapAction = actions.mode.DynamicColormapAction(
parent=self, plot=plot
)
self.addAction(self._dynamicColormapAction)

Comment on lines +56 to +60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will add it to all kind of plots (curves, scatters) while it only works with images, so it's not the place.
This could be added to the colormap dialog, though it is already quite crowded or the ImageToolBar... or not included in any toolbar and added by the application that wants it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same remark as before: could be in ImageToolbar but is truly an interactive mode. On the top of that, this ImageToolbar does not seem to be used on the PlotWindow/ImageView that displays images. I can add the action in the Plot2D toolbar. But don't really know how to handle the interaction mode (hence the initial positioning in the PlotWidget/InteractionToolbar.

def getZoomModeAction(self):
"""Returns the zoom mode QAction.

Expand All @@ -67,6 +72,9 @@ def getPanModeAction(self):
"""
return self._panModeAction

def getDynamicColormapAction(self):
return self._dynamicColormapAction


class OutputToolBar(qt.QToolBar):
"""Toolbar providing icons to copy, save and print a PlotWidget
Expand Down
71 changes: 71 additions & 0 deletions src/silx/resources/gui/icons/dynamic_colormap.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.