pyapp-kit / ndv

Simple, fast-loading, n-dimensional array viewer with minimal dependencies.
BSD 3-Clause "New" or "Revised" License
40 stars 7 forks source link

fix: fix canvas update on set_data #40

Closed tlambert03 closed 2 months ago

tlambert03 commented 3 months ago

fixes #39
cc @gselzer

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.33%. Comparing base (abbbeb3) to head (2cfbc9b). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/ndv/viewer/_viewer.py 92.85% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #40 +/- ## ========================================== - Coverage 77.78% 77.33% -0.45% ========================================== Files 13 13 Lines 1814 1827 +13 ========================================== + Hits 1411 1413 +2 - Misses 403 414 +11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tlambert03 commented 3 months ago

fixed, thanks

gselzer commented 3 months ago

Aha, didn't realize it could be fixed simply using signals_blocked. Good to know!

gselzer commented 3 months ago

@tlambert03 I've been experimenting with this branch within my UI, and I've come back to report that NDViewer.refresh() has a prohibitive performance impact. I'm not yet sure why, but I'm guessing it's due to slow event handling as performance seems to get worse over time. Here's an MCVE you can try:

EDIT: It's actually NDViewer._clear_images that seems to be slowing the whole thing down, if you comment that invocation out of NDViewer.refresh things work great 🤯

Do you think it's the PImageHandle creation in every NDViewer._on_data_slice_ready call that takes time?

from qtpy.QtCore import QCoreApplication, Qt, QTimerEvent
from qtpy.QtWidgets import QApplication
from ndv import NDViewer
import numpy
import sys

# Generate 10 random frames beforehand
frames = [
    numpy.random.randint(0, 255, [256, 256]) for _ in range(10)
]

class ExampleBad(NDViewer):
    """Calls set_data on each timerEvent"""

    def __init__(self) -> None:
        super().__init__()
        self.i = 0
        # Timer triggering frame change every 10 ms
        self._live_timer_id = self.startTimer(
            10, Qt.TimerType.PreciseTimer
        )

    def timerEvent(self, a0: QTimerEvent) -> None:
        self.i = (self.i + 1) % len(frames)
        self.set_data(frames[self.i])

class ExampleAlsoBad(NDViewer):
    """Overwrites buffer + calls refresh()` on each timerEvent"""

    def __init__(self) -> None:
        super().__init__()
        # Generate 10 random frames beforehand
        frames = [
            numpy.random.randint(0, 255, [256, 256]) for _ in range(10)
        ]
        self.buffer = numpy.zeros((256, 256))
        self.i = 0
        # Timer triggering frame change every 10 ms
        self._live_timer_id = self.startTimer(
            10, Qt.TimerType.PreciseTimer
        )
        self.set_data(self.buffer)

    def timerEvent(self, a0: QTimerEvent) -> None:
        self.i = (self.i + 1) % len(frames)
        self.buffer[:, :] = frames[self.i]
        self.refresh()

class ExampleGood(NDViewer):
    """Overwrites buffer + calls set_current_index()` on each timerEvent"""

    def __init__(self) -> None:
        super().__init__()
        # Generate 10 random frames beforehand
        frames = [
            numpy.random.randint(0, 255, [256, 256]) for _ in range(10)
        ]
        self.buffer = numpy.zeros((256, 256))
        self.i = 0
        # Timer triggering frame change every 10 ms
        self._live_timer_id = self.startTimer(
            10, Qt.TimerType.PreciseTimer
        )
        self.set_data(self.buffer)

    def timerEvent(self, a0: QTimerEvent) -> None:
        self.i = (self.i + 1) % len(frames)
        self.buffer[:, :] = frames[self.i]
        self.set_current_index()

## Most of the code below is copied from ndv.util

def _get_app() -> tuple[QCoreApplication, bool]:
    is_ipython = False
    if (app := QApplication.instance()) is None:
        app = QApplication([])
        app.setApplicationName("ndv")
    elif (ipy := sys.modules.get("IPython")) and (shell := ipy.get_ipython()):
        is_ipython = str(shell.active_eventloop).startswith("qt")

    return app, not is_ipython

app, should_exec = _get_app()
viewer = ExampleGood()
viewer.show()
viewer.raise_()
if should_exec:
    app.exec()
tlambert03 commented 3 months ago

yes, this is a very heavy handed refresh approach. I do not really expect set_data to be called that often. perhaps your MCVE is too refined? :). tell me a bit more about the use case that brought you to this. calling set_data on every data event is not particularly what I imagined we'd do here

gselzer commented 3 months ago

perhaps your MCVE is too refined? :). tell me a bit more about the use case that brought you to this. calling set_data on every data event is not particularly what I imagined we'd do here

My use case is handling a live view from pymmcore-plus (currently using the demo camera). Currently, I've subclassed the NDViewer so to override NDViewer.setData to be closer to the good example (there may be some excess cruft that should be trimmed off). In an ideal world I wouldn't have to override it, though.

tlambert03 commented 3 months ago

Not that ndv shouldnt also be able to do that... but have you tried https://pymmcore-plus.github.io/pymmcore-widgets/widgets/ImagePreview/

Thats what we use for rapid image preview

gselzer commented 3 months ago

Not that ndv shouldnt also be able to do that... but have you tried https://pymmcore-plus.github.io/pymmcore-widgets/widgets/ImagePreview/

I do believe that I originally used it, however there are some ndv features (e.g. ROIs, eventually histograms) that @marktsuchida and I are excited to make use of in live views.

tlambert03 commented 3 months ago

Yep! Totally understand/agree