Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict zoom out to data min/max range #2487

Open
DanAurea opened this issue May 11, 2023 · 12 comments
Open

Restrict zoom out to data min/max range #2487

DanAurea opened this issue May 11, 2023 · 12 comments

Comments

@DanAurea
Copy link
Contributor

Hi,

I tried to inherit from the PanZoom() to modify its behavior and restrict zoom out whenever the viewbox would be out of a specific range.
This is for preventing user to zoom out/zoom in inadvertently far away from the scene.

The issue is that overriding zoom function, the xaxis/yaxis aren't updated anymore accordingly to user zoom while I called inherited zoom() function.
I do understand that the function responsible of such axis update is domain() but it shall be triggered due to link_view() and it seems not.

Here's the extracted code sample:

class RestrictedPanZoomCamera(PanZoomCamera):
    def zoom(self, factor, center=None):
        super(RestrictedPanZoomCamera, self).zoom(factor, center)

camera = RestrictedPanZoomCamera(parent=plt.view.scene, name='restrictedpanzoom')

plt.camera = camera
plt.view.camera = camera
plt.view.camera._rect = (min(t), 0, max(t), max(t))
plt.view.camera.rect = (min(t), 0, max(t), max(t))
plt.view.camera.set_range(x = (min(t), max(t)), y = (0, -150))

Best regards.

@djhoese
Copy link
Member

djhoese commented May 15, 2023

Are you working with @h21ak9 who made this similar issue: #2486

In your example code, what is plt? Why do you set camera on plt and on plt.view? Why do you have ._rect and .rect set?

The only other recommendation I can make at this time would be to not set parent when you create your camera instance. The ViewBox will do this when you do view.camera = camera.

@h21ak9
Copy link

h21ak9 commented May 15, 2023

We are not working together. It's nice to know others are asking similar questions!

@DanAurea
Copy link
Contributor Author

No we are not working together but it look like it's also a feature other users are interested on.
I'm used to OpenGL and Qwt in C++, I had implemented some features in Qwt that I would like to reimplement with VisPy and such scrolling limitation is part of it.
I digged into VisPy code but confused camera rect meaning, I will put a more complete code later and retry few things.

If I succeed to limit the zoom in an elegant way as I did with qwt then it could be something I could propose as a feature inside a PR.

@DanAurea
Copy link
Contributor Author

DanAurea commented May 15, 2023

I manage to make it works with a workaround and found when the issue arise but not the specific root cause yet.
The issue is that if we use plotwidget and trigger _configure_2d then the default camera would be panzoom.
So the user can updated it afterwards with custom camera but x/y axis wouldn't be linked to the view properly (didn't understood why).

To be able to have correct linking of x/y axis with camera and view, I had to recreate the view after triggering _configured2d and link again x/y axis to the newly created view, below is full code example, line 107/108 and line 115/116 are required to ensure x/y axis updates:

import numpy as np
import vispy.plot as vp
from vispy.color import get_colormap
from vispy.scene.cameras import BaseCamera
from vispy.geometry import Rect
from vispy.scene.cameras import PanZoomCamera
# load example data
from vispy.io import load_data_file
data = np.load(load_data_file('electrophys/iv_curve.npz'))['arr_0']
data *= 1000  # V -> mV
dt = 1e-4  # this data is sampled at 10 kHz

# create figure with plot
fig = vp.Fig()
plt = fig[0, 0]
selected = None

class RestrictedPanZoomCamera(PanZoomCamera):
    
    def __init__(self, limits:tuple=(-np.Inf, np.Inf, -np.Inf, np.Inf), *args, **kwargs):
        if len(limits) != 4:
            raise ArgumentError("Input 'limits' must have 4 elements")
        self._left_limit = limits[0]
        self._right_limit = limits[1]
        self._bottom_limit = limits[2]
        self._top_limit = limits[3]
        super(RestrictedPanZoomCamera, self).__init__(*args, **kwargs)
    
    def zoom(self, factor, center=None):
        """
        This overwrites the behavior of the parent class
        to prevent the user from zooming outside 
        boundaries set by self._limits
        """
        super(RestrictedPanZoomCamera, self).zoom(factor, center)
        
        # Make a new object (copy), so that allocation will trigger view_changed
        # TBH it's not probably the best way since it's duplicated with super class method
        # but that's a VisPy requirement.
        rect = Rect(self.rect)

        # Scale these spaces
        rect.left = max(rect.left, self._left_limit)
        rect.right = min(rect.right, self._right_limit)
        rect.bottom = max(rect.bottom, self._bottom_limit)
        rect.top = min(rect.top, self._top_limit)
        
        self.rect = rect
    
    def viewbox_mouse_event(self, event):
        """
        This overwrites the behavior of the parent class
        to prevent the user from panning outside 
        the boundaries set by self._limits
        """
        # Scrolling
        BaseCamera.viewbox_mouse_event(self, event)

        if event.type == "mouse_wheel":
            center = self._scene_transform.imap(event.pos)
            self.zoom((1 + self.zoom_factor)**(-event.delta[1] * 30), center)
            event.handled = True

        if event.type == "mouse_move":
            if event.press_event is None:
                return

            modifiers = event.mouse_event.modifiers
            p1 = event.mouse_event.press_event.pos
            p2 = event.mouse_event.pos

            if 1 in event.buttons and not modifiers:
                # Translate
                p1 = np.array(event.last_event.pos)[:2]
                p2 = np.array(event.pos)[:2]
                p1s = self._transform.imap(p1)
                p2s = self._transform.imap(p2)
                delta = p1s - p2s
                new_rect = self.rect + delta
                if new_rect.left <= self._left_limit or new_rect.right >= self._right_limit:
                    delta[0] = 0.0
                if new_rect.top >= self._top_limit or new_rect.bottom <= self._bottom_limit:
                    delta[1] = 0.0
                self.pan(delta)
                event.handled = True
            elif 2 in event.buttons and not modifiers:
                # Zoom
                p1c = np.array(event.last_event.pos)[:2]
                p2c = np.array(event.pos)[:2]
                scale = ((1 + self.zoom_factor)**((p1c - p2c) * 
                                                  np.array([1, -1])))
                center = self._transform.imap(event.press_event.pos[:2])
                self.zoom(scale, center)
                event.handled = True
        elif event.type == "mouse_press":
            # accept the event if it is button 1 or 2.
            # This is required in order to receive future events
            event.handled = event.button in [1,2]
        else:
            event.handled = False

plt._configure_2d()

# Update camera since by default configure_2d will create a panzoom camera
# We have to recreate viewbox otherwise the camera will not be linked correctly anymore to
# axis.
plt.grid.remove_widget(plt.view)
plt.view = plt.grid.add_view(row=2, col = 4)

plt.view.camera = RestrictedPanZoomCamera(limits = (0, 400, -150, 0))
plt.camera = plt.view.camera
plt.view.camera.set_range()

# Since view has been recreated we have to relink x/y axis
plt.xaxis.link_view(plt.view)
plt.yaxis.link_view(plt.view)

# plot data
cmap = get_colormap('hsl', value=0.5)
colors = cmap.map(np.linspace(0.1, 0.9, data.shape[0]))
t = np.arange(data.shape[1]) * (dt * 1000)
for i, y in enumerate(data):
    line = plt.plot((t, y), color=colors[i])
    line.interactive = True
    line.unfreeze()  # make it so we can add a new property to the instance
    line.data_index = i
    line.freeze()

if __name__ == '__main__':
    fig.app.run()

@djhoese
Copy link
Member

djhoese commented May 16, 2023

I suppose the "easier" solution would be to subclass the PlotWidget to allow for a custom camera. I guess either way we're talking about adding stuff to upstream vispy either as functionality to the camera (where something like a .set_limits method sets scrollable limits) or as a plot widget update that allows for custom cameras. I'm not sure I like the change to plot widget going in vispy.

@h21ak9
Copy link

h21ak9 commented May 16, 2023

@DanAurea Is there a reason you decided to go with vispy.plot instead of vispy.scene? I've managed to make a nice looking plot using vispy.scene, and I think it avoids the problem you have been grappling with (correct me if I am wrong). See the attached code (I could not directly upload .py files -- you will have to change the extensions from .txt to .py). Hope this helps!

PS -- because I have restricted the camera's range, you will have to zoom in with your scroll wheel before you can move the plot around and confirm that the axes update accordingly.

test_5a_modified_panzoom.txt
restricted_panzoom.txt

@DanAurea
Copy link
Contributor Author

@h21ak9 No reason, I just get the example from the gallery for testing purpose since I wanted to implement also a restricted zoom but seeing axis not being updated in first place I just stopped.
On my tool I'm using as well SceneCanvas with a reworked "PlotWidget" since I'm not a huge fan of how it's being designed so I shall not being annoyed by such issue.

I just wanted to raise so contributors would be aware of it ;)

@djhoese On QWT I was used to add such feature as part of the camera but here it depends on the meaning of each entity. It look like limits are being set on viewbox through events forwarded to camera so it would be probably a good idea to implement such feature as part of current PanZoom (I don't see the point for turntable/fly/magnify etc...).
If you are ok with it I could propose a PR with a modified PanZoom allowing such limitation reusing Rect instead of right/left/top/limit.

@djhoese
Copy link
Member

djhoese commented May 16, 2023

with a reworked "PlotWidget" since I'm not a huge fan of how it's being designed

The plotting API in vispy is NOT being designed. That's part of the problem. It was added by the original vispy maintainers long long ago as a proof of concept, but with relatively little thought for the future. It also had the challenge of bending the SceneCanvas to fit in a generic plotting use case which isn't always the most flexible. If you have ideas for a redesign please file an issue or a pull request with suggestions.

As for a PZ camera update, I'd love to see what you had in mind. As long as it doesn't break existing functionality I don't see a reason we can't merge it.

@DanAurea
Copy link
Contributor Author

@djhoese Sure I do understand that this was exposed as a guideline more than a specific API, it's just that the current implementation/design lead to flaw such as the one observed whenever subclassing of some feature is required.

In my opinion right now it depends on user need, for instance in my case on the project I'm working on I'm not using PlotWidget but instead directly rely on SceneCanvas and implement something closer to what I did with Qwt and so the observed issue wouldn't be present.

For the plotting API do you have any specific PR/issue already opened ? I was also thinking about how to implement a more complex plot API and was trying to implement heat map/hist2D smoothly so if I'm succeeding to achieve something nice that could be helpful for others I would be glad to share it ;)

@djhoese
Copy link
Member

djhoese commented May 16, 2023

it's just that the current implementation/design lead to flaw such as the one observed whenever subclassing of some feature is required.

IMO Any good API shouldn't require subclassing unless that is how you use the "API".

For the plotting API do you have any specific PR/issue already opened ?

There are multiple issues and PRs open for various improvements. I don't recall any high level "this is what plotting should be" issues. Some PRs are half finished or are only proof of concept type implementations (ex. datetime handling in axes widgets/visuals, etc). I haven't looked it up but I know there are issues or conversations on stackoverflow/gitter that were basically answered by me saying "the plotting API isn't feature complete or well tested". Some vispy users come to vispy expecting matplotlib amounts of functionality but with better performance. That amount of functionality just isn't reasonable with the 2 semi-active maintainers we have especially considering neither of us use the plotting API.

So in summary I'm open to quite a few changes. Backwards compatibility or a path to migrate to a new interface would be nice. There is also low-level VisPy 2.0 work going on in other repositories by other former VisPy maintainers, but I'm not sure that is going to affect us any time soon.

@DanAurea
Copy link
Contributor Author

DanAurea commented May 16, 2023

Yes I have followed a bit the path/story of VisPy since I had the same goal that initiator of VisPy/Glumpy and other stuff like this.
I wanted to provide a high performant way to plot with Python through usage OpenGL and had worked on different backends as well such as Vulkan/legacy/modern OpenGL and fancy libaries (moderngl/py opengl...).
I'm not expecting from VisPy that it does what Matplotlib does no worries, I'm just looking in a way to evolve myself VisPy in a way that it could fit in with some requests you could have already being confronted to.

In the end I just wanted to share about issue I faced off to highlight some flaw that other users could encounter if they are expecting to use it as it is, it was not a blaming on API no worries.

For subclassing I do agree that it's most often due to not complete "API" but still depend on what you want to achieve and what's your goal, I mean it's better for sure to include new features straight into the library itself
But if you don't want to shot yourself because you want to still profit from further bugfixes and your designed features wouldn't be merged to VisPy for any reasons (different expectations etc...) then the only way to profit from both world is to subclass in a way that you can extend and match to your needs and profit from new updates without any "merge conflicts".

@djhoese
Copy link
Member

djhoese commented May 16, 2023

Yeah I think the easiest method for doing this is to update the PlotWidget in vispy to be more flexible and allow more configuring from the user. Even if it is at a class attribute level or an instance attribute level, you (the user) could still assign a new default (plot) widget for your Figure class/instance if you made your own plot widget subclass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants