nanogui
nanogui copied to clipboard
Bare bones resize feature
Resize window feature. There are 5 different rectangles to manages this. 2 at the corners, one at the right, one at the left, one at he bottom. You can use corner rectangles to resize diagonally.
I think it needs to give more feedback to user though. For instance mouse could change in resize ares. The thing GLFW does not have mouse icon for diagonal resize, only horizontal or vertical.
Also, I think there should be event for resize. Widgets may want to know that they are going to be resized.
But if we put these aside, resizing works as it is.
Hi @byECHO,
thank you for the PR. I took a look at the implementation but unfortunately don't think that it's something I am willing to merge at least in this form. The main issues are:
- It should be possible to do all of the resize management without the heavy additions -- per-window
mResizeRects
(which basically just store constant offsets) and theRect
data structure. Basically I think this part is way more complicated than it should be. - There is no kind of visual feedback (via a cursor) that would indicate when something can be resized.
- The interaction with the windows feels quite weird -- the logic that manages size increments behaves differently from the standard behavior for user interfaces.
Hi,
It should be possible to do all of the resize management without the heavy additions -- per-window mResizeRects (which basically just store constant offsets) and the Rect data structure. Basically I think this part is way more complicated than it should be.
I'm open the suggestions, if you have more simple way to do it I'll gladly work on it. Meanwhile this came to my mind. Instead of checking against rectangles we can set radius variable to tell if the mouse is close enough to borders. Then according to offset of mouse we can determine what type of resize is this(vertical, horizontal or both).
There is no kind of visual feedback (via a cursor) that would indicate when something can be resized.
Agreed, main problem is there is no cursor for both horizontal and vertical. They are available as separate. Most of the people I know use corner to resize. I don't know what to do with. I can add this when we finalize the implementation for resizing.
The interaction with the windows feels quite weird -- the logic that manages size increments behaves differently from the standard behavior for user interfaces.
Well this is how it looks like in action https://imgur.com/XR6Ekzd . Suggestions?
Hi @byECHO,
I saw that you pushed some code. Do you want to me to take another look, or are you still working on it?
Best, Wenzel
Hi,
Yeah, sure go ahead thats all of it.
Hi, Any update on this?
Hi @byECHO,
cool -- I just had another look at your PR. This is starting to look very promising! I have a few more comments that should be relatively straightforward to integrate though.
-
There is something weird about how the resizing behaves when the mouse cursor goes left or above of the window. It's hard to describe with words, so I made a video: https://www.mitsuba-renderer.org/files/repository/nanogui-resize.mp4
This could be avoided by keeping track of resizing motion in absolute rather than relative terms (i.e. with a
mDragStart
field). -
When going to the corner, I would have expected a diagonal drag handle to appear. Currently, only horizontal and vertical drag modes work.
-
For many windows, it will not make sense for them to be resized at all. Thus, windows should have a
mResizable
field that is false by default, but which can be enabled to permit resizing. -
Nanogui generally compiles without warnings (at least on Linux/OSX). However, I get the following warnings from your code.
[14/47] Building CXX object CMakeFiles/nanogui-obj.dir/src/window.cpp.o
src/window.cpp:23:5: warning: field 'mMinSize' will be initialized after field 'mResizeDir' [-Wreorder]
mMinSize(Vector2i::Zero()), mResizeDir(Vector2i::Zero()) { }
^
src/window.cpp:161:45: warning: unused parameter 'p' [-Wunused-parameter]
bool Window::mouseDragEvent(const Vector2i &p, const Vector2i &rel,
^
src/window.cpp:184:80: warning: unused parameter 'rel' [-Wunused-parameter]
bool Window::mouseMotionEvent(const Eigen::Vector2i &p, const Eigen::Vector2i &rel, int button, int modifiers) {
^
src/window.cpp:184:89: warning: unused parameter 'button' [-Wunused-parameter]
bool Window::mouseMotionEvent(const Eigen::Vector2i &p, const Eigen::Vector2i &rel, int button, int modifiers) {
^
src/window.cpp:184:101: warning: unused parameter 'modifiers' [-Wunused-parameter]
bool Window::mouseMotionEvent(const Eigen::Vector2i &p, const Eigen::Vector2i &rel, int button, int modifiers) {
^
I'm really looking forward to this feature.
Concerning @wjakob's 1. point, I have something to add though: The same happens just by moving the windows and passing the Screen
's boundaries (I made a video for showing what I mean: http://optinetics.net/move.webm).
So this might actually be related to some underlying mouse movement handling in NanoGUI rather than only the resize code. Or it could be the same problem in two places, I didn't look at the code yet.
Hi @wjakob ,
There is something weird about how the resizing behaves
when the mouse cursor goes left or above of the window.
This could be avoided by keeping track of resizing motion in
absolute rather than relative terms (i.e. with a mDragStart field).
Yeah, I got what you mean. Thanks for the video. I'll see what I can do.
When going to the corner, I would have expected a diagonal drag handle to appear. Currently, only horizontal and vertical drag modes work.
Yeah about that. It is actually very easy to add that but the problem is cursor. I'm not a guru on GLFW so I can be wrong, but I could not find cursor type for diagonal resize. There is only vertical and horizontal one.(GLFW_VRESIZE_CURSOR and GLFW_HRESIZE_CURSOR)
For many windows, it will not make sense for them to be resized at all. Thus, windows should have a mResizable field that is false by default, but which can be enabled to permit resizing.
Makes sense but what if the programmer sets fixed size and mResizable to true? What should be the behaviour than? Currently code uses fixed size to check if window can resize.
Nanogui generally compiles without warnings (at least on Linux/OSX). However, I get the following warnings from your code.
My bad. I should have paid more attention to warnings.
Sincerely.
How about using just using the V/Hresize cursors but allowing diagonal motion as well? There is an open GLFW bugreport about the diagonal cursors, which will probably added at some point in the future.
About your other point: if mFixedSize
is set, resizing does not make much sense I suppose. This could actually be useful to constrain resizing to only be possible along one axis (e.g. horizontal or vertical).
One more thing: your Window
modifications cause it not to forward certain kinds of events that are not locally consumed (drags, etc.). You'll need to call the superclass event handler in such cases.
This currently breaks the image widget in example1
where it's still possible to zoom in/out with the mouse wheel, but dragging the image around with the left mouse button no longer works.
Hi,
These should fix all the problems. I have also added requested features.
Sincerely.
Hi @wjakob ,
I agree, performLayout should be called only if window gets resized but mResize not only used for that. mResize also used like mDrag. Checking for resize in mouse drag event fails most of the time. Event if captures it, if you move mouse very quick it loses it.
Thats why there is mResize and mResizeDir. I can of course remove mResizeDir and check for mouse position against center of the window to decide where to resize. If I remove mResizeDir than of course those functions should return bool not int. Sorry for the other things, they are typos and leftovers. They are fixed now.
Sincerely.
Hi @byECHO,
Sorry for the other things, they are typos and leftovers. They are fixed now.
Did you see the 4 or 5 comments from me above? Or did you forget to push? I just checked and didn't see any changes regarding those.
Thanks, Wenzel
Hi @wjakob ,
I think my connection cut out while pushing changes to repo. Anyway.
Why does checHorizontalResize or checkHorizontalResize returns int instead of bool, is connected to mResizeDir. It is used to determine which direction you can resize when you first click mouse in resize area. These functions returns where you have clicked. There are 3 cases involving mResizeDir, 4th one is not used because you cant resize window upwards. I could not exactly understand what did you mean crunching down 4 cases to 2 cases with sign
variable and removing mResizeDir.
Sincerely.
Also, Should there be an event for resize to let know widgets or layouts that their parent is resized? For instance, resizing kinda messes up image view. Some of the elements does not correctly resize too. Maybe the future widgets will need to know if they are being resized.
Sincerely.
(I'm currently traveling but will take a closer look at this soon)
This looks like a really useful PR. Any news on merging?
@wjakob will please take another look at this?
Is there any news on the resize feature?