From 9ae519f6faed20eab90129c15fbfee7ff43b45e7 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Fri, 4 Nov 2022 16:28:45 -0400 Subject: [PATCH] Re-work chart-overlay event broadcasting Drop all attempts at rewiring `ViewBox` signals, monkey-patching relayee handlers, and generally modifying event source public attributes. Instead take a much simpler approach where the event source graphics object simply has it's handler dynamically overridden by a broadcaster function which relays to all consumers using a Python loop. The benefits of this much simplified approach include: - avoiding the tedious and often complex (re)connection of signals between the source plot and the overlayed consumers. - requiring zero modification of the public interface of any of the publisher or consumer `ViewBox`s, no decoration, extra signal definitions (eg. previous `mouseDragEventRelay` or the like). - only a single dynamic method override on the event source graphics object (`ViewBox`) which does the broadcasting work and requires no modification to handler implementations. Detailed `.ui._overlay` changes: - drop `mk_relay_signal()`, `enable_relays()` which removes signal/slot hacking methodology. - drop unused `ComposedGridLayout.grid` and `.reverse`, change some method names: `.insert()` -> `.insert_plotitem()`, `append()` -> `.append_plotitem()`. - in `PlotOverlay`, again drop all signal/slot rewiring in `.add_plotitem()` and instead add our new closure based python-loop in `broadcast()` routine which is used to override the event-source object's handler. - comment out all the auxiliary/want-to-have event source selection methods for now. --- piker/ui/_interaction.py | 62 +++-- piker/ui/_overlay.py | 474 ++++++++++++++++----------------------- 2 files changed, 233 insertions(+), 303 deletions(-) diff --git a/piker/ui/_interaction.py b/piker/ui/_interaction.py index d6899b60..e17e662e 100644 --- a/piker/ui/_interaction.py +++ b/piker/ui/_interaction.py @@ -329,7 +329,6 @@ async def handle_viewmode_mouse( ): # when in order mode, submit execution # msg.event.accept() - # breakpoint() view.order_mode.submit_order() @@ -346,16 +345,6 @@ class ChartView(ViewBox): ''' mode_name: str = 'view' - # "relay events" for making overlaid views work. - # NOTE: these MUST be defined here (and can't be monkey patched - # on later) due to signal construction requiring refs to be - # in place during the run of meta-class machinery. - mouseDragEventRelay = QtCore.Signal(object, object, object) - wheelEventRelay = QtCore.Signal(object, object, object) - - event_relay_source: 'Optional[ViewBox]' = None - relays: dict[str, QtCore.Signal] = {} - def __init__( self, @@ -479,7 +468,7 @@ class ChartView(ViewBox): self, ev, axis=None, - relayed_from: ChartView = None, + # relayed_from: ChartView = None, ): ''' Override "center-point" location for scrolling. @@ -490,6 +479,13 @@ class ChartView(ViewBox): TODO: PR a method into ``pyqtgraph`` to make this configurable ''' + linked = self.linked + if ( + not linked + ): + # print(f'{self.name} not linked but relay from {relayed_from.name}') + return + if axis in (0, 1): mask = [False, False] mask[axis] = self.state['mouseEnabled'][axis] @@ -609,9 +605,20 @@ class ChartView(ViewBox): self, ev, axis: Optional[int] = None, - relayed_from: ChartView = None, + # relayed_from: ChartView = None, ) -> None: + # if relayed_from: + # print(f'PAN: {self.name} -> RELAYED FROM: {relayed_from.name}') + + # NOTE since in the overlay case axes are already + # "linked" any x-range change will already be mirrored + # in all overlaid ``PlotItems``, so we need to simply + # ignore the signal here since otherwise we get N-calls + # from N-overlays resulting in an "accelerated" feeling + # panning motion instead of the expect linear shift. + # if relayed_from: + # return pos = ev.pos() lastPos = ev.lastPos() @@ -849,33 +856,37 @@ class ChartView(ViewBox): ) -> None: ''' - Assign callback for rescaling y-axis automatically - based on data contents and ``ViewBox`` state. + Assign callbacks for rescaling and resampling y-axis data + automatically based on data contents and ``ViewBox`` state. ''' if src_vb is None: src_vb = self - # splitter(s) resizing + # widget-UIs/splitter(s) resizing src_vb.sigResized.connect(self._set_yrange) + # re-sampling trigger: # TODO: a smarter way to avoid calling this needlessly? # 2 things i can think of: # - register downsample-able graphics specially and only # iterate those. - # - only register this when certain downsampleable graphics are + # - only register this when certain downsample-able graphics are # "added to scene". src_vb.sigRangeChangedManually.connect( self.maybe_downsample_graphics ) - # mouse wheel doesn't emit XRangeChanged src_vb.sigRangeChangedManually.connect(self._set_yrange) - # src_vb.sigXRangeChanged.connect(self._set_yrange) - # src_vb.sigXRangeChanged.connect( - # self.maybe_downsample_graphics - # ) + # XXX: enabling these will cause "jittery"-ness + # on zoom where sharp diffs in the y-range will + # not re-size right away until a new sample update? + # if src_vb is not self: + # src_vb.sigXRangeChanged.connect(self._set_yrange) + # src_vb.sigXRangeChanged.connect( + # self.maybe_downsample_graphics + # ) def disable_auto_yrange(self) -> None: @@ -916,7 +927,6 @@ class ChartView(ViewBox): self, autoscale_overlays: bool = True, ): - profiler = Profiler( msg=f'ChartView.maybe_downsample_graphics() for {self.name}', disabled=not pg_profile_enabled(), @@ -931,8 +941,12 @@ class ChartView(ViewBox): # TODO: a faster single-loop-iterator way of doing this XD chart = self._chart + plots = {chart.name: chart} + linked = self.linked - plots = linked.subplots | {chart.name: chart} + if linked: + plots |= linked.subplots + for chart_name, chart in plots.items(): for name, flow in chart._flows.items(): diff --git a/piker/ui/_overlay.py b/piker/ui/_overlay.py index 0bbba413..af66a735 100644 --- a/piker/ui/_overlay.py +++ b/piker/ui/_overlay.py @@ -18,23 +18,27 @@ Charting overlay helpers. ''' -from typing import Callable, Optional - -from pyqtgraph.Qt.QtCore import ( - # QObject, - # Signal, - Qt, - # QEvent, +from collections import defaultdict +from functools import partial +from typing import ( + Callable, + Optional, ) from pyqtgraph.graphicsItems.AxisItem import AxisItem from pyqtgraph.graphicsItems.ViewBox import ViewBox -from pyqtgraph.graphicsItems.GraphicsWidget import GraphicsWidget +# from pyqtgraph.graphicsItems.GraphicsWidget import GraphicsWidget from pyqtgraph.graphicsItems.PlotItem.PlotItem import PlotItem -from pyqtgraph.Qt.QtCore import QObject, Signal, QEvent -from pyqtgraph.Qt.QtWidgets import QGraphicsGridLayout, QGraphicsLinearLayout - -from ._interaction import ChartView +from pyqtgraph.Qt.QtCore import ( + QObject, + Signal, + QEvent, + Qt, +) +from pyqtgraph.Qt.QtWidgets import ( + # QGraphicsGridLayout, + QGraphicsLinearLayout, +) __all__ = ["PlotItemOverlay"] @@ -89,16 +93,11 @@ class ComposedGridLayout: def __init__( self, item: PlotItem, - grid: QGraphicsGridLayout, - reverse: bool = False, # insert items to the "center" ) -> None: - self.items: list[PlotItem] = [] - # self.grid = grid - self.reverse = reverse - # TODO: use a ``bidict`` here? - self._pi2axes: dict[ + self.items: list[PlotItem] = [] + self._pi2axes: dict[ # TODO: use a ``bidict`` here? int, dict[str, AxisItem], ] = {} @@ -120,12 +119,13 @@ class ComposedGridLayout: if name in ('top', 'bottom'): orient = Qt.Vertical + elif name in ('left', 'right'): orient = Qt.Horizontal layout.setOrientation(orient) - self.insert(0, item) + self.insert_plotitem(0, item) # insert surrounding linear layouts into the parent pi's layout # such that additional axes can be appended arbitrarily without @@ -159,7 +159,7 @@ class ComposedGridLayout: # enter plot into list for index tracking self.items.insert(index, plotitem) - def insert( + def insert_plotitem( self, index: int, plotitem: PlotItem, @@ -171,7 +171,9 @@ class ComposedGridLayout: ''' if index < 0: - raise ValueError('`insert()` only supports an index >= 0') + raise ValueError( + '`.insert_plotitem()` only supports an index >= 0' + ) # add plot's axes in sequence to the embedded linear layouts # for each "side" thus avoiding graphics collisions. @@ -220,7 +222,7 @@ class ComposedGridLayout: return index - def append( + def append_plotitem( self, item: PlotItem, @@ -232,7 +234,7 @@ class ComposedGridLayout: ''' # for left and bottom axes we have to first remove # items and re-insert to maintain a list-order. - return self.insert(len(self.items), item) + return self.insert_plotitem(len(self.items), item) def get_axis( self, @@ -249,16 +251,16 @@ class ComposedGridLayout: named = self._pi2axes[name] return named.get(index) - def pop( - self, - item: PlotItem, + # def pop( + # self, + # item: PlotItem, - ) -> PlotItem: - ''' - Remove item and restack all axes in list-order. + # ) -> PlotItem: + # ''' + # Remove item and restack all axes in list-order. - ''' - raise NotImplementedError + # ''' + # raise NotImplementedError # Unimplemented features TODO: @@ -279,194 +281,6 @@ class ComposedGridLayout: # axis? -# TODO: we might want to enabled some kind of manual flag to disable -# this method wrapping during type creation? As example a user could -# definitively decide **not** to enable broadcasting support by -# setting something like ``ViewBox.disable_relays = True``? -def mk_relay_method( - - signame: str, - slot: Callable[ - [ViewBox, - 'QEvent', - Optional[AxisItem]], - None, - ], - -) -> Callable[ - [ - ViewBox, - # lol, there isn't really a generic type thanks - # to the rewrite of Qt's event system XD - 'QEvent', - - 'Optional[AxisItem]', - 'Optional[ViewBox]', # the ``relayed_from`` arg we provide - ], - None, -]: - - def maybe_broadcast( - vb: 'ViewBox', - ev: 'QEvent', - axis: 'Optional[int]' = None, - relayed_from: 'ViewBox' = None, - - ) -> None: - ''' - (soon to be) Decorator which makes an event handler - "broadcastable" to overlayed ``GraphicsWidget``s. - - Adds relay signals based on the decorated handler's name - and conducts a signal broadcast of the relay signal if there - are consumers registered. - - ''' - # When no relay source has been set just bypass all - # the broadcast machinery. - if vb.event_relay_source is None: - ev.accept() - return slot( - vb, - ev, - axis=axis, - ) - - if relayed_from: - assert axis is None - - # this is a relayed event and should be ignored (so it does not - # halt/short circuit the graphicscene loop). Further the - # surrounding handler for this signal must be allowed to execute - # and get processed by **this consumer**. - # print(f'{vb.name} rx relayed from {relayed_from.name}') - ev.ignore() - - return slot( - vb, - ev, - axis=axis, - ) - - if axis is not None: - # print(f'{vb.name} handling axis event:\n{str(ev)}') - ev.accept() - return slot( - vb, - ev, - axis=axis, - ) - - elif ( - relayed_from is None - and vb.event_relay_source is vb # we are the broadcaster - and axis is None - ): - # Broadcast case: this is a source event which will be - # relayed to attached consumers and accepted after all - # consumers complete their own handling followed by this - # routine's processing. Sequence is, - # - pre-relay to all consumers *first* - ``.emit()`` blocks - # until all downstream relay handlers have run. - # - run the source handler for **this** event and accept - # the event - - # Access the "bound signal" that is created - # on the widget type as part of instantiation. - signal = getattr(vb, signame) - # print(f'{vb.name} emitting {signame}') - - # TODO/NOTE: we could also just bypass a "relay" signal - # entirely and instead call the handlers manually in - # a loop? This probably is a lot simpler and also doesn't - # have any downside, and allows not touching target widget - # internals. - signal.emit( - ev, - axis, - # passing this demarks a broadcasted/relayed event - vb, - ) - # accept event so no more relays are fired. - ev.accept() - - # call underlying wrapped method with an extra - # ``relayed_from`` value to denote that this is a relayed - # event handling case. - return slot( - vb, - ev, - axis=axis, - ) - - return maybe_broadcast - - -# XXX: :( can't define signals **after** class compile time -# so this is not really useful. -# def mk_relay_signal( -# func, -# name: str = None, - -# ) -> Signal: -# ( -# args, -# varargs, -# varkw, -# defaults, -# kwonlyargs, -# kwonlydefaults, -# annotations -# ) = inspect.getfullargspec(func) - -# # XXX: generate a relay signal with 1 extra -# # argument for a ``relayed_from`` kwarg. Since -# # ``'self'`` is already ignored by signals we just need -# # to count the arguments since we're adding only 1 (and -# # ``args`` will capture that). -# numargs = len(args + list(defaults)) -# signal = Signal(*tuple(numargs * [object])) -# signame = name or func.__name__ + 'Relay' -# return signame, signal - - -def enable_relays( - widget: GraphicsWidget, - handler_names: list[str], - -) -> list[Signal]: - ''' - Method override helper which enables relay of a particular - ``Signal`` from some chosen broadcaster widget to a set of - consumer widgets which should operate their event handlers normally - but instead of signals "relayed" from the broadcaster. - - Mostly useful for overlaying widgets that handle user input - that you want to overlay graphically. The target ``widget`` type must - define ``QtCore.Signal``s each with a `'Relay'` suffix for each - name provided in ``handler_names: list[str]``. - - ''' - signals = [] - for name in handler_names: - handler = getattr(widget, name) - signame = name + 'Relay' - # ensure the target widget defines a relay signal - relay = getattr(widget, signame) - widget.relays[signame] = name - signals.append(relay) - method = mk_relay_method(signame, handler) - setattr(widget, name, method) - - return signals - - -enable_relays( - ChartView, - ['wheelEvent', 'mouseDragEvent'] -) - - class PlotItemOverlay: ''' A composite for managing overlaid ``PlotItem`` instances such that @@ -482,16 +296,18 @@ class PlotItemOverlay: ) -> None: self.root_plotitem: PlotItem = root_plotitem + self.relay_handlers: defaultdict[ + str, + list[Callable], + ] = defaultdict(list) - vb = root_plotitem.vb - vb.event_relay_source = vb # TODO: maybe change name? - vb.setZValue(1000) # XXX: critical for scene layering/relaying + # NOTE: required for scene layering/relaying; this guarantees + # the "root" plot receives priority for interaction + # events/signals. + root_plotitem.vb.setZValue(1000) self.overlays: list[PlotItem] = [] - self.layout = ComposedGridLayout( - root_plotitem, - root_plotitem.layout, - ) + self.layout = ComposedGridLayout(root_plotitem) self._relays: dict[str, Signal] = {} def add_plotitem( @@ -499,8 +315,10 @@ class PlotItemOverlay: plotitem: PlotItem, index: Optional[int] = None, - # TODO: we could also put the ``ViewBox.XAxis`` - # style enum here? + # event/signal names which will be broadcasted to all added + # (relayee) ``PlotItem``s (eg. ``ViewBox.mouseDragEvent``). + relay_events: list[str] = [], + # (0,), # link x # (1,), # link y # (0, 1), # link both @@ -510,58 +328,155 @@ class PlotItemOverlay: index = index or len(self.overlays) root = self.root_plotitem - # layout: QGraphicsGridLayout = root.layout self.overlays.insert(index, plotitem) vb: ViewBox = plotitem.vb - # mark this consumer overlay as ready to expect relayed events - # from the root plotitem. - vb.event_relay_source = root.vb - # TODO: some sane way to allow menu event broadcast XD # vb.setMenuEnabled(False) - # TODO: inside the `maybe_broadcast()` (soon to be) decorator - # we need have checks that consumers have been attached to - # these relay signals. - if link_axes != (0, 1): + # wire up any relay signal(s) from the source plot to added + # "overlays". We use a plain loop instead of mucking with + # re-connecting signal/slots which tends to be more invasive and + # harder to implement and provides no measurable performance + # gain. + if relay_events: + for ev_name in relay_events: + relayee_handler: Callable[ + [ + ViewBox, + # lol, there isn't really a generic type thanks + # to the rewrite of Qt's event system XD + QEvent, - # wire up relay signals - for relay_signal_name, handler_name in vb.relays.items(): - # print(handler_name) - # XXX: Signal class attrs are bound after instantiation - # of the defining type, so we need to access that bound - # version here. - signal = getattr(root.vb, relay_signal_name) - handler = getattr(vb, handler_name) - signal.connect(handler) + AxisItem | None, + ], + None, + ] = getattr(vb, ev_name) + + sub_handlers: list[Callable] = self.relay_handlers[ev_name] + + # on the first registry of a relayed event we pop the + # root's handler and override it to a custom broadcaster + # routine. + if not sub_handlers: + + src_handler = getattr( + root.vb, + ev_name, + ) + + def broadcast( + ev: 'QEvent', + + # TODO: drop this viewbox specific input and + # allow a predicate to be passed in by user. + axis: 'Optional[int]' = None, + + *, + + # these are bound in by the ``partial`` below + # and ensure a unique broadcaster per event. + ev_name: str = None, + src_handler: Callable = None, + relayed_from: 'ViewBox' = None, + + # remaining inputs the source handler expects + **kwargs, + + ) -> None: + ''' + Broadcast signal or event: this is a source + event which will be relayed to attached + "relayee" plot item consumers. + + The event is accepted halting any further + handlers from being triggered. + + Sequence is, + - pre-relay to all consumers *first* - exactly + like how a ``Signal.emit()`` blocks until all + downstream relay handlers have run. + - run the event's source handler event + + ''' + ev.accept() + + # broadcast first to relayees *first*. trigger + # relay of event to all consumers **before** + # processing/consumption in the source handler. + relayed_handlers = self.relay_handlers[ev_name] + + assert getattr(vb, ev_name).__name__ == ev_name + + # TODO: generalize as an input predicate + if axis is None: + for handler in relayed_handlers: + handler( + ev, + axis=axis, + **kwargs, + ) + + # run "source" widget's handler last + src_handler( + ev, + axis=axis, + ) + + # dynamic handler override on the publisher plot + setattr( + root.vb, + ev_name, + partial( + broadcast, + ev_name=ev_name, + src_handler=src_handler + ), + ) + + else: + assert getattr(root.vb, ev_name) + assert relayee_handler not in sub_handlers + + # append relayed-to widget's handler to relay table + sub_handlers.append(relayee_handler) # link dim-axes to root if requested by user. - # TODO: solve more-then-wanted scaled panning on click drag - # which seems to be due to broadcast. So we probably need to - # disable broadcast when axes are linked in a particular - # dimension? for dim in link_axes: # link x and y axes to new view box such that the top level # viewbox propagates to the root (and whatever other # plotitem overlays that have been added). vb.linkView(dim, root.vb) - # make overlaid viewbox impossible to focus since the top - # level should handle all input and relay to overlays. - # NOTE: this was solved with the `setZValue()` above! + # => NOTE: in order to prevent "more-then-linear" scaled + # panning moves on (for eg. click-drag) certain range change + # signals (i.e. ``.sigXRangeChanged``), the user needs to be + # careful that any broadcasted ``relay_events`` are are short + # circuited in sub-handlers (aka relayee's) implementations. As + # an example if a ``ViewBox.mouseDragEvent`` is broadcasted, the + # overlayed implementations need to be sure they either don't + # also link the x-axes (by not providing ``link_axes=(0,)`` + # above) or that the relayee ``.mouseDragEvent()`` handlers are + # ready to "``return`` early" in the case that + # ``.sigXRangeChanged`` is emitted as part of linked axes. + # For more details on such signalling mechanics peek in + # ``ViewBox.linkView()``. - # TODO: we will probably want to add a "focus" api such that - # a new "top level" ``PlotItem`` can be selected dynamically - # (and presumably the axes dynamically sorted to match). + # make overlaid viewbox impossible to focus since the top level + # should handle all input and relay to overlays. Note that the + # "root" plot item gettingn interaction priority is configured + # with the ``.setZValue()`` during init. vb.setFlag( vb.GraphicsItemFlag.ItemIsFocusable, False ) vb.setFocusPolicy(Qt.NoFocus) + # => TODO: add a "focus" api for switching the "top level" + # ``PlotItem`` dynamically. + # append-compose into the layout all axes from this plot - self.layout.insert(index, plotitem) + self.layout.insert_plotitem(index, plotitem) plotitem.setGeometry(root.vb.sceneBoundingRect()) @@ -579,25 +494,6 @@ class PlotItemOverlay: root.vb.setFocus() assert root.vb.focusWidget() - # XXX: do we need this? Why would you build then destroy? - def remove_plotitem(self, plotItem: PlotItem) -> None: - ''' - Remove this ``PlotItem`` from the overlayed set making not shown - and unable to accept input. - - ''' - ... - - # TODO: i think this would be super hot B) - def focus_item(self, plotitem: PlotItem) -> PlotItem: - ''' - Apply focus to a contained PlotItem thus making it the "top level" - item in the overlay able to accept peripheral's input from the user - and responsible for zoom and panning control via its ``ViewBox``. - - ''' - ... - def get_axis( self, plot: PlotItem, @@ -630,8 +526,9 @@ class PlotItemOverlay: return axes - # TODO: i guess we need this if you want to detach existing plots - # dynamically? XXX: untested as of now. + # XXX: untested as of now. + # TODO: need this as part of selecting a different root/source + # plot to rewire interaction event broadcast dynamically. def _disconnect_all( self, plotitem: PlotItem, @@ -646,3 +543,22 @@ class PlotItemOverlay: disconnected.append(sig) return disconnected + + # XXX: do we need this? Why would you build then destroy? + # def remove_plotitem(self, plotItem: PlotItem) -> None: + # ''' + # Remove this ``PlotItem`` from the overlayed set making not shown + # and unable to accept input. + + # ''' + # ... + + # TODO: i think this would be super hot B) + # def focus_plotitem(self, plotitem: PlotItem) -> PlotItem: + # ''' + # Apply focus to a contained PlotItem thus making it the "top level" + # item in the overlay able to accept peripheral's input from the user + # and responsible for zoom and panning control via its ``ViewBox``. + + # ''' + # ...