The fourth dimension and the case of the missing slider

[jni]

I think these are the conditions for this bug to manifest:

• have a 4D dataset
• start the animation with ndisplay=2 and at some point switch to ndisplay=3

In this scenario, the viewer should have 2 sliders at the start, then 1 at the end. But in fact it ends up with 1 at the start and 0 at the end.

Here's a reproducible example:

import numpy as np
import imageio.v3 as iio
import napari
from napari_animation import Animation

image4d = np.random.random((10, 20, 30, 40))

viewer = napari.Viewer()
layer = viewer.add_image(image4d)

animation = Animation(viewer)
animation.capture_keyframe()

viewer.dims.current_step = (9, 19, 15, 20)
animation.capture_keyframe(steps=10)

iio.imwrite('random.png', viewer.screenshot(canvas_only=False, flash=False))

viewer.dims.current_step = (9, 0, 15, 20)
animation.capture_keyframe(steps=20)

viewer.dims.ndisplay = 3
animation.capture_keyframe(steps=1)

viewer.camera.angles = (90, 0, 0)
animation.capture_keyframe(steps=30)

iio.imwrite('random2.png', viewer.screenshot(canvas_only=False, flash=False))

animation.animate('random.mov', canvas_only=False, fps=10)

The iio.imwrites are there to show that viewer.screenshot is itself working fine. 😅

The result is:

https://user-images.githubusercontent.com/492549/222425560-05d1b09a-a0fd-4463-b8f7-01d9c42ce037.mov

and the intermediate images:

[jni]

I hypothesised that the state at the end of the animation was the important thing, but no: adding viewer.dims.ndisplay = 2 and a keyframe grab at the end of the script doesn't help. 😢

[alisterburt]

woah what on earth - just to be clear: the sliders are there in the viewer, not there in the resulting animation but are there in viewer screenshots?!

[alisterburt]

this is nuts

[jni]

I'm glad you agree 😂 but sad you're not all like "oh yeah I've seen this before and had a good lead on how to fix this but never got a chance to trying it". 😂 Yes your interpretation was correct. 🤯

[jni]

at the Pacific community meeting, @andy-sweet pointed to this bit of code as a potential culprit:

https://github.com/napari/napari/blob/c43a2e7c0181b5c4ed877fb1f59cae6f4d0ff0d2/napari/_qt/widgets/qt_dims.py#L100-L118

The if conditions are a bit, ahem, iffy, 😜 more specifically they are non-trivial, so it's worth checking whether there's some spurious reason there that the sliders are invisible in these circumstances.

Regarding the script itself and the discrepancy between the animation and the screenshots — if I'm not mistaken @alisterburt, the first two screenshots are taken on the first pass, whereas the animation that happens at the end actually replays all of these things based on the past viewer state, so they aren't exactly happening simultaneously, but rather at the end of everything, is that correct?

[alisterburt]

interesting - I have some time booked in with @katherine-hutchings next week so maybe we can dig into this together, this looks interesting for sure!

the first two screenshots are taken on the first pass, whereas the animation that happens at the end actually replays all of these things based on the past viewer state, so they aren't exactly happening simultaneously, but rather at the end of everything, is that correct?

I'm not 100% what you've said is correct so I'll state what's happening: each ViewerState in the FrameSequence (a Sequence[ViewerState]) is applied on the Viewer in order and a screenshot is taken. The ViewerState is pretty all encompassing so most things get updated at each frame, rather than it being a small 'diff' between states

[jni]

each ViewerState in the FrameSequence (a Sequence[ViewerState]) is applied on the Viewer in order and a screenshot is taken.

Sure, that's fine, I don't think that contradicts what I said. What I meant is that in my script, I do, for example:

# 1
viewer.dims.current_step = (9, 19, 15, 20)
animation.capture_keyframe(steps=10)

# 2
iio.imwrite('random.png', viewer.screenshot(canvas_only=False, flash=False))

# 3
viewer.dims.current_step = (9, 0, 15, 20)
animation.capture_keyframe(steps=20)

# [...]

# 4
animation.animate('random.mov', canvas_only=False, fps=10)

So, in step 1, I set an attribute, current_step to some value. This happens immediately. Then capture_keyframe writes down what the viewer state is at that point — but it doesn't take any screenshots or anything, it just writes down the viewer state. (?). Then step 2, I take a screenshot — this screenshot makes sure all the Qt events are processed (so the UI matches the viewer state) and then it takes the screenshot of the viewer. Then in step 3, again, I set a new value, and this happens instantly, and capture_keyframe writes down the viewer state again. Finally, in step 4, napari_animation looks up all of the viewer states, resets the viewer to the first state, way back in the script, and then plays them back (with interpolation), taking a screenshot at each moment. So, what this means is that the time between my "manual" screenshot in step 2, and when napari_animation replays that state and takes its own screenshot of that state, can be large, so all kinds of mischief might have happened in that interval.

Correct?

[alisterburt]

correct, I'm with you :)

[jni]

at the Pacific community meeting, @andy-sweet pointed to this bit of code as a potential culprit:

Dang. Just stepped through it and it was saying all the right things. 😂 WEIRD!!!

[jni]

Ok, another bit of info, which I guess is totally expected from all the discussion above: if you take a screenshot after the .animate call, indeed, the slider is still missing at the end.


[jni]

More info, sorry for the noise, but I could stop at any minute so I want to write things down. 😅

I wanted to see whether the issue was that the slider was getting hidden, or that the height of the slider space was getting reduced. Turns out it's the latter! In:

https://github.com/napari/napari/blob/c43a2e7c0181b5c4ed877fb1f59cae6f4d0ff0d2/napari/_qt/widgets/qt_dims.py#L117

I hardcoded the height to have space for two sliders, and then the sliders were in the video, and shown and hidden at the right times!

https://user-images.githubusercontent.com/492549/222863168-60e624d6-b6ca-4aa1-8fce-c2340cabaf05.mov

Weirdly, as happened when I was stepping through the code, if I print nsliders, it's always being set to the right number. 🤔

[jni]

Ok, more weirdness (which you can also see in the video above, looking closely):

• If I set the minimumHeight before hiding/showing the sliders, this sort of fixes the problem, except
• (see above): the slider annotations on the right are no longer nicely aligned and have weird sizing issues. Compare and contrast:

Before napari-animation works its magic:

After napari-animation has messed with the viewer:

So, there's something different about how napari-animation updates the viewer state, indeed hinted at by @alisterburt above: it updates all the dims attributes, not just those that have changed, and (this is then almost certainly a napari bug, not napari-animation) doing this actually results in some weird intermediate state that is somehow wonky.

[jni]

Ok, so the updating all the attributes is not the issue, the issue is that those attributes are updated in EventedModel with events blocked:

https://github.com/napari/napari/blob/4b71b83b41918acdd48251929f4f675d31e24eda/napari/utils/events/evented_model.py#L314-L320

And, notably, I ran the code with a breakpoint on this line, where the blocked events are supposed to all be emitted in a batch, and it was never called:

https://github.com/napari/napari/blob/4b71b83b41918acdd48251929f4f675d31e24eda/napari/utils/events/evented_model.py#L322-L323

So I'm wondering if there's a bug in the self.events.blocker context manager?

I also think it might not actually be necessary? If I delete the context manager, just update attributes willy-nilly and let the events fire when they may, (a) it has no measurable effect on performance (the EventedModel setattr itself checks for equality so won't emit events unnecessarily), (b) the sliders work! 🎉 Actually, the wonkiness is still there, but it happens even when just taking the screenshots (I added an extra viewer.screenshot call right before animate and confirmed that the wonkiness is there).

I'd love to get the opinion of some EventedModel experts about whether that is the right approach here... CC @sofroniewn @tlambert03 @brisvag...

[alisterburt]

Oh wow - excellent digging @jni ! I haven't looked at the old evented model code but this could be an excuse to update to the psygnal backed one inside napari...

[jni]

old evented model code but this could be an excuse to update to the psygnal backed one inside napari...

details/links? But this seems like a bigger lift than I want to do for this particular bug. 😂 Do we have a mix of things within napari already? Cos if not, I think I'd rather avoid it — we should convert the whole codebase in one go.

At any rate, as far as I can tell, events.blocker only blocks events that were fired as a group, not individual events from within that group... Will try to come up with a minimal reproducible example soon...

[tlambert03]

events.blocker only blocks events that were fired as a group, not individual events from within that group...

you can use EmitterGroup.blocker_all() to achieve that

[jni]

@tlambert03 sure but then you lose the ability to "capture" the emissions and emit them all in a batch at the end... Right?

[jni]

Also, would you say that this use here was in fact intended to be a blocker_all? (but with counting/re-emitting)?

[jni]

(Thank you for dropping by btw 😅 it is appreciated!)

[tlambert03]

Also, would you say that this use here was in fact intended to be a blocker_all? (but with counting/re-emitting)?

reading that code now, I would say that would be my expectation of what that code should do 😄 ... but I can't remember

let me take a closer look at your example above

[tlambert03]

alright... looks like this is an issue with event-loop driven things not having a chance to execute before the frame is captured.
While I do think it's indicative of a deeper problem/race condition in the napari event connection, you can also fix it quite simply here by adding a call to QApplication.processEvents() at the end of ViewerState.apply

https://user-images.githubusercontent.com/1609449/223140495-73fa2d59-8f09-4433-a488-a9069a58cd05.mov

(side note: that has the side-effect of showing the viewer while rendering... which is probably undesirable in some cases, but I think that's gonna be the situation you're stuck in for a while, similar to how some of the napari tests only work if you use show())

[tlambert03]

alternatively, you could place that call to processEvents upstream in napari at the beginning of qt_main_window.Window._screenshot

[andy-sweet]

alternatively, you could place that call to processEvents upstream in napari at the beginning of qt_main_window.Window._screenshot

FWIW, this was also a proposed solution for making screenshot wait for async slicing/rendering (though that's not on by default yet): https://github.com/napari/napari/pull/5052/files#discussion_r971040485

In that case, we emit a napari event on the slicing thread and thus use @ensure_main_thread on the connected QtViewer mostly to ensure that Qt updates occur safely on the main thread. But the best we can do there is enqueue the connected callback, so also need to rely on processEvents.

I don't think the details here are quite the same (i.e. I don't think there's another thread), but my assumption was that most (all?) Qt events will always be enqueued to their associated event loop, so I'm not sure of another obvious fix to handle that generally.

[jni]

alternatively, you could place that call to processEvents upstream in napari at the beginning of qt_main_window.Window._screenshot

I thought we already did this!! 😂