Open ctrueden opened 1 month ago
Attention: Patch coverage is 33.33333%
with 8 lines
in your changes missing coverage. Please review.
Project coverage is 77.62%. Comparing base (
10abe13
) to head (5d2e83f
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
hell yes :)
there is a slight typing issue here, which requires some explanation. the self._canvas
issue you've found is an interface that needs to be implemented for each backend (vispy and pygfx). self._canvas._camera.zoom
is actually a vispy-specific API.
So, we either need to add a zoom()
method to that PCanvas
interface (to be implemented by both backends), or we need to use the existing self._canvas.set_range()
method instead (which, then leads to the question "how do I get the current range :joy: ... lemme dig a moment)(
thanks for the update @ctrueden. Couple remaining issues. (Depending on your degree of interest, I'm happy to fix them from here):
pip install pygfx
and then use NDV_CANVAS_BACKEND=pygfx python examples/numpy_arr.py
in order to pick backend)isinstance(camera, ...)
check on the backend and just return if it's not the right type of camera)center
not being as easy/relevant in the 3d camera modes (for example, using the mouse wheel in 3d mode doesn't pay attention to the mouse position)@tlambert03 Thanks. Sorry about the pygfx wrong-direction zoom; I didn't notice that when I tested briefly. I did know about the mouse position limitation though—I wrote about it in the commit message.
In my view, the +/- keys should behave the same as the mouse wheel scroll (in both 2D and 3D), so of course the first place I looked for implementing +/- was how the mouse wheel code works. But I, embarrassingly, could not figure out how/where the mouse wheel code is... reading the source, all I could see was stuff about a selection, which did not seem relevant. I concluded that maybe the mouse wheel behavior is handled upstream in the backends themselves, rather than in ndv? But I didn't yet dive into a debugger to confirm that. If you know how it works already, I think it makes sense for you to do your thing and wrap up this work. But if you would also need to dig, I can try to make time to do it myself some time soon. Just let me know your preference!
I concluded that maybe the mouse wheel behavior is handled upstream in the backends themselves, rather than in ndv?
indeed! all of the mouse interactions are currently just passed to the backends. This was done for now so as to avoid duplicating all of the mouse/controller logic that has already been implemented in these backends. It does of course mean that cases like this can be a little hard to figure out exactly what is handling mouse interactions. For now, until we fully abstract away all mouse interactions and key interactions, this is the state of things. We can certainly intercept mouse/key interaction and do something else, but by default, we just pass them through to the backends (which, thankfully, do mostly the same thing with them)
I'm also more than happy to dig into this (i appreciate what you've already done :)). not sure when I'll get to it with busy upcoming weeks, but whoever gets there first can update the other. thanks @ctrueden 🙏
This patch adds keyboard shortcuts for zooming the canvas in and out using +/= and -/_ respectively. It also tweaks the hover logic to save the last computed data point, so that the canvas can be zoomed more precisely at that coordinate.