ros-visualization / rqt_common_plugins

45 stars 132 forks source link

[rqt_image_view] Add a scroll area #433

Closed contradict closed 7 years ago

contradict commented 7 years ago

When selecting a 1-to-1 pixel view, the previous implementation resized the window to the full image size. This is inconvenient for images much larger than the screen resolution. This patch places the RatioLayoutedFrame in a ScrollArea so the window still fits on the screen and the user can pan on the full image.

dirk-thomas commented 7 years ago

Can you please clarify what you mean with "selecting a 1-to-1 pixel view".

If I understand your use case correctly you want images which are larger than the available space not to be scaled down but show them in their native resolution and use a scroll area to show specific areas of the image.

The RatioLayoutedFrame is designed to scale the image to fit into the space available for the widget. Many users want to see the "full" image even if it is being scaled down (or up).

While I can see your use case for a scroll area I don't think this aligns with the use case of all users. Therefore it should be added as an option rather than the default behavior.

contradict commented 7 years ago

There is a button in the default interface pixelview Which causes the image to be displayed with no scaling. This is great if the image resolution is smaller than the resolution of your screen. However, when the image resolution is much larger than your screen resolution (imagine an 10Mpix image on a 1920x1080 screen), this results in a window much larger than your screen. The only way to inspect the image is to move the whole window on your screen which is frequently awkward.

With this patch, if this button is not pressed, the tool works just as before with the image scaled to fit the available area. When this button is pressed, the image is displayed without scaling, just as before, except now the window is not resized and the scroll area is activated. Instead of moving the whole window, you can now pan the image inside the scroll area.

If window larger than screen is a desirable feature, perhaps I could modify the maximize button to resize the window to match the image instead of the screen?

DorianScholz commented 7 years ago

I agree, having the "1" button resize the window, is often very awkward. Especially when the image viewer is embedded in a rqt layout with other widgets. So I'm all for this patch. Maybe an additional button could be added though, that resizes the window to fit the image? This would make the previous behavior available, which is nice to have for small images, by clicking the "1" first and then the "fit window" button.

dirk-thomas commented 7 years ago

I just compared the current behavior (which resized the rqt window when using 1:1 mapping) with the new behavior of this PR (keeping the window size and showing the image in a scroll area. While the patch "looses" the functionality to automatically resize the window it is certainly better for large images.

Since the resizing of the windows was not always without problems (sometime the plugin was resized to a smaller size than the parent dock widget which results in weird empty space around it) I think it makes sense to merge this.

Thanks!