From 28c0f80e6d633204dafeca8aed2e3089b115a048 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Mon, 23 Jan 2023 15:22:42 -0500 Subject: [PATCH] Factor color and cache mode settings into `FlowGraphics` Curve-path colouring and cache mode settings are used (and can thus be factored out of) all child types; this moves them into the parent type's `.__init__()` and adjusts all sub-types match: - the bulk was moved out of the `Curve.__init__()` including all previous commentary around cache settings. - adjust `BarItems` to use a `NoCache` mode and instead use the `last_step_pen: pg.Pen` and `._pen` inside it's `.pain()` instead of defining functionally duplicate vars. - adjust all (transitive) calls to `BarItems` to use the new kwargs names. --- piker/ui/_chart.py | 2 - piker/ui/_curve.py | 149 ++++++++++++++++++++++++------------------- piker/ui/_display.py | 9 +-- piker/ui/_ohlc.py | 42 +++--------- 4 files changed, 98 insertions(+), 104 deletions(-) diff --git a/piker/ui/_chart.py b/piker/ui/_chart.py index 64c091d5..cf33335f 100644 --- a/piker/ui/_chart.py +++ b/piker/ui/_chart.py @@ -1152,8 +1152,6 @@ class ChartPlotWidget(pg.PlotWidget): if is_ohlc: graphics = BarItems( - linked=self.linked, - plotitem=pi, color=color, name=name, **graphics_kwargs, diff --git a/piker/ui/_curve.py b/piker/ui/_curve.py index f4dba90b..72d61d84 100644 --- a/piker/ui/_curve.py +++ b/piker/ui/_curve.py @@ -60,11 +60,82 @@ class FlowGraphic(pg.GraphicsObject): ''' # sub-type customization methods - declare_paintables: Optional[Callable] = None - sub_paint: Optional[Callable] = None + declare_paintables: Callable | None = None + sub_paint: Callable | None = None - # TODO: can we remove this? - # sub_br: Optional[Callable] = None + # XXX-NOTE-XXX: graphics caching B) + # see explanation for different caching modes: + # https://stackoverflow.com/a/39410081 + cache_mode: int = QGraphicsItem.DeviceCoordinateCache + # XXX: WARNING item caching seems to only be useful + # if we don't re-generate the entire QPainterPath every time + # don't ever use this - it's a colossal nightmare of artefacts + # and is disastrous for performance. + # QGraphicsItem.ItemCoordinateCache + # TODO: still questions todo with coord-cacheing that we should + # probably talk to a core dev about: + # - if this makes trasform interactions slower (such as zooming) + # and if so maybe if/when we implement a "history" mode for the + # view we disable this in that mode? + + def __init__( + self, + *args, + name: str | None = None, + + # line styling + color: str = 'bracket', + last_step_color: str = 'original', + fill_color: Optional[str] = None, + style: str = 'solid', + + **kwargs + + ) -> None: + + self._name = name + + # primary graphics item used for history + self.path: QPainterPath = QPainterPath() + + # additional path that can be optionally used for appends which + # tries to avoid triggering an update/redraw of the presumably + # larger historical ``.path`` above. the flag to enable + # this behaviour is found in `Renderer.render()`. + self.fast_path: QPainterPath | None = None + + # TODO: evaluating the path capacity stuff and see + # if it really makes much diff pre-allocating it. + # self._last_cap: int = 0 + # cap = path.capacity() + # if cap != self._last_cap: + # print(f'NEW CAPACITY: {self._last_cap} -> {cap}') + # self._last_cap = cap + + # all history of curve is drawn in single px thickness + self._color: str = color + pen = pg.mkPen(hcolor(color), width=1) + pen.setStyle(_line_styles[style]) + + if 'dash' in style: + pen.setDashPattern([8, 3]) + + self._pen = pen + self._brush = pg.functions.mkBrush( + hcolor(fill_color or color) + ) + + # last segment is drawn in 2px thickness for emphasis + self.last_step_pen = pg.mkPen( + hcolor(last_step_color), + width=2, + ) + self._last_line: QLineF = QLineF() + + super().__init__(*args, **kwargs) + + # apply cache mode + self.setCacheMode(self.cache_mode) def x_uppx(self) -> int: @@ -112,82 +183,32 @@ class Curve(FlowGraphic): updates don't trigger a full path redraw. ''' - cache_mode: int = QGraphicsItem.DeviceCoordinateCache + # TODO: can we remove this? + # sub_br: Optional[Callable] = None def __init__( self, *args, - step_mode: bool = False, - color: str = 'default_lightest', - fill_color: Optional[str] = None, - style: str = 'solid', - name: Optional[str] = None, + # color: str = 'default_lightest', + # fill_color: Optional[str] = None, + # style: str = 'solid', **kwargs ) -> None: - self._name = name - # brutaaalll, see comments within.. self.yData = None self.xData = None - # self._last_cap: int = 0 - self.path: Optional[QPainterPath] = None - - # additional path that can be optionally used for appends which - # tries to avoid triggering an update/redraw of the presumably - # larger historical ``.path`` above. the flag to enable - # this behaviour is found in `Renderer.render()`. - self.fast_path: QPainterPath | None = None - # TODO: we can probably just dispense with the parent since # we're basically only using the pen setting now... super().__init__(*args, **kwargs) - # all history of curve is drawn in single px thickness - pen = pg.mkPen(hcolor(color)) - pen.setStyle(_line_styles[style]) - - if 'dash' in style: - pen.setDashPattern([8, 3]) - - self._pen = pen - - # last segment is drawn in 2px thickness for emphasis - # self.last_step_pen = pg.mkPen(hcolor(color), width=2) - self.last_step_pen = pg.mkPen(pen, width=2) - self._last_line: QLineF = QLineF() - # flat-top style histogram-like discrete curve - # self._step_mode: bool = step_mode - # self._fill = True - self._brush = pg.functions.mkBrush(hcolor(fill_color or color)) - - # NOTE: this setting seems to mostly prevent redraws on mouse - # interaction which is a huge boon for avg interaction latency. - - # TODO: one question still remaining is if this makes trasform - # interactions slower (such as zooming) and if so maybe if/when - # we implement a "history" mode for the view we disable this in - # that mode? - # don't enable caching by default for the case where the - # only thing drawn is the "last" line segment which can - # have a weird artifact where it won't be fully drawn to its - # endpoint (something we saw on trade rate curves) - self.setCacheMode(self.cache_mode) - - # XXX-NOTE-XXX: graphics caching. - # see explanation for different caching modes: - # https://stackoverflow.com/a/39410081 seems to only be useful - # if we don't re-generate the entire QPainterPath every time - # don't ever use this - it's a colossal nightmare of artefacts - # and is disastrous for performance. - # self.setCacheMode(QtWidgets.QGraphicsItem.ItemCoordinateCache) # allow sub-type customization declare = self.declare_paintables @@ -318,14 +339,10 @@ class Curve(FlowGraphic): p.setPen(self.last_step_pen) p.drawLine(self._last_line) - profiler('.drawLine()') - p.setPen(self._pen) + profiler('last datum `.drawLine()`') + p.setPen(self._pen) path = self.path - # cap = path.capacity() - # if cap != self._last_cap: - # print(f'NEW CAPACITY: {self._last_cap} -> {cap}') - # self._last_cap = cap if path: p.drawPath(path) @@ -370,7 +387,7 @@ class Curve(FlowGraphic): # from last datum to current such that # the end of line touches the "beginning" # of the current datum step span. - x_2last , y[-2], + x_2last, y[-2], x_last, y[-1], ) diff --git a/piker/ui/_display.py b/piker/ui/_display.py index 9e18b55e..fccd0af4 100644 --- a/piker/ui/_display.py +++ b/piker/ui/_display.py @@ -447,7 +447,8 @@ async def graphics_update_loop( # and quote_rate >= _quote_throttle_rate * 2 and quote_rate >= display_rate ): - log.warning(f'High quote rate {symbol.key}: {quote_rate}') + pass + # log.warning(f'High quote rate {symbol.key}: {quote_rate}') last_quote_s = time.time() @@ -493,9 +494,9 @@ def graphics_update_cycle( profiler = Profiler( msg=f'Graphics loop cycle for: `{ds.fqsn}`', - delayed=True, disabled=not pg_profile_enabled(), ms_threshold=ms_slower_then, + delayed=True, # ms_threshold=4, ) @@ -1319,7 +1320,7 @@ async def display_symbol_data( is_ohlc=True, color=bg_chart_color, - last_bar_color=bg_last_bar_color, + last_step_color=bg_last_bar_color, ) # ensure the last datum graphic is generated @@ -1356,7 +1357,7 @@ async def display_symbol_data( is_ohlc=True, color=bg_chart_color, - last_bar_color=bg_last_bar_color, + last_step_color=bg_last_bar_color, ) rt_pi.vb.maxmin = partial( rt_chart.maxmin, diff --git a/piker/ui/_ohlc.py b/piker/ui/_ohlc.py index c23bd290..104b860c 100644 --- a/piker/ui/_ohlc.py +++ b/piker/ui/_ohlc.py @@ -18,13 +18,8 @@ Super fast OHLC sampling graphics types. """ from __future__ import annotations -from typing import ( - Optional, - TYPE_CHECKING, -) import numpy as np -import pyqtgraph as pg from PyQt5 import ( QtGui, QtWidgets, @@ -33,18 +28,14 @@ from PyQt5.QtCore import ( QLineF, QRectF, ) - +from PyQt5.QtWidgets import QGraphicsItem from PyQt5.QtGui import QPainterPath from ._curve import FlowGraphic from .._profile import pg_profile_enabled, ms_slower_then -from ._style import hcolor from ..log import get_logger from .._profile import Profiler -if TYPE_CHECKING: - from ._chart import LinkedSplits - log = get_logger(__name__) @@ -100,30 +91,18 @@ class BarItems(FlowGraphic): "Price range" bars graphics rendered from a OHLC sampled sequence. ''' + # XXX: causes this weird jitter bug when click-drag panning + # where the path curve will awkwardly flicker back and forth? + cache_mode: int = QGraphicsItem.NoCache + def __init__( self, - linked: LinkedSplits, - plotitem: 'pg.PlotItem', # noqa - color: str = 'bracket', - last_bar_color: str = 'original', - - name: Optional[str] = None, + *args, + **kwargs, ) -> None: - super().__init__() - self.linked = linked - # XXX: for the mega-lulz increasing width here increases draw - # latency... so probably don't do it until we figure that out. - self._color = color - self.bars_pen = pg.mkPen(hcolor(color), width=1) - self.last_bar_pen = pg.mkPen(hcolor(last_bar_color), width=2) - self._name = name - # XXX: causes this weird jitter bug when click-drag panning - # where the path curve will awkwardly flicker back and forth? - self.setCacheMode(QtWidgets.QGraphicsItem.DeviceCoordinateCache) - - self.path = QPainterPath() + super().__init__(*args, **kwargs) self._last_bar_lines: tuple[QLineF, ...] | None = None def x_last(self) -> None | float: @@ -218,12 +197,12 @@ class BarItems(FlowGraphic): # as is necesarry for what's in "view". Not sure if this will # lead to any perf gains other then when zoomed in to less bars # in view. - p.setPen(self.last_bar_pen) + p.setPen(self.last_step_pen) if self._last_bar_lines: p.drawLines(*tuple(filter(bool, self._last_bar_lines))) profiler('draw last bar') - p.setPen(self.bars_pen) + p.setPen(self._pen) p.drawPath(self.path) profiler(f'draw history path: {self.path.capacity()}') @@ -299,5 +278,4 @@ class BarItems(FlowGraphic): # date / from some previous sample. It's weird though # because i've seen it do this to bars i - 3 back? - # return ohlc['time'], ohlc['close'] return ohlc[index_field], ohlc['close']