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

UI: Support drag and move projector when clicking inside the window #8138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fsworld009
Copy link

@fsworld009 fsworld009 commented Jan 24, 2023

Description

Support drag and move projector when clicking inside the window.

This is based on WizardCM@9ee79ac which is mentioned in #5818 (comment)
but I changed how the new location is computed when dragging:

It is now computed by comparing initial mouse absolute position and the current mouse absolute position, and the diff is always applied to the initial window position. This way, the window's top left corner doesn't snap to mouse cursor like the mentioned commit, the behavior is now the same as dragging by clicking the window title.

08.14.2023 Update

Redco the PR with following changes:

  1. Use this->window()->move() to move window instead of this->move(). This is much smoother and works well by only computing diff based on mouse event position. Ref: https://stackoverflow.com/a/39821779/3973896
  2. Show drag icon when mouse is hovered on widget
  3. Disable drag to move when it's in full screen mode

Note: I can't change target branch to be another PR branch, so currently I can't enable drag to move only for frameless window. Will need to wait until related PR is merged in.

Motivation and Context

I suppose this functionality is essential for frameless window support (#5818, #7892, or #9430), which I look forward to, so I made this PR.

How Has This Been Tested?

Built the code on my Windows 10 Machine and manually tested my changes.

Edition	Windows 10 Pro
Version	21H2
Installed on	‎6/‎6/‎2020
OS build	19044.2251
Experience	Windows Feature Experience Pack 120.2212.4180.0

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.
    • (No projector documentation per my knowledge)

@fsworld009
Copy link
Author

Not sure how to resolve

FAILED: UI/CMakeFiles/obs.dir/window-projector.cpp.o 
/usr/bin/ccache /usr/bin/c++ -DBROWSER_AVAILABLE -DDL_D3D11=\"\" -DDL_D3D9=\"\" -DDL_METAL=\"\" -DDL_OPENGL=\"libobs-opengl.so.1\" -DENABLE_HEVC -DHAVE_OBSCONFIG_H -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_SVG_LIB -DQT_WIDGETS_LIB -DQT_XML_LIB -DWHATSNEW_ENABLED -I/home/runner/work/obs-studio/obs-studio/obs-studio/build/UI/obs_autogen/include -I/home/runner/work/obs-studio/obs-studio/obs-studio/UI -I/home/runner/work/obs-studio/obs-studio/obs-studio/build/UI -I/home/runner/work/obs-studio/obs-studio/obs-studio/deps/json11 -I/home/runner/work/obs-studio/obs-studio/obs-studio/deps/libff -I/home/runner/work/obs-studio/obs-studio/obs-studio/plugins/obs-browser -I/home/runner/work/obs-studio/obs-studio/obs-studio/plugins/obs-browser/panel -I/home/runner/work/obs-studio/obs-studio/obs-studio/libobs -I/home/runner/work/obs-studio/obs-studio/obs-studio/build/config -I/home/runner/work/obs-studio/obs-studio/obs-studio/UI/obs-frontend-api -I/home/runner/work/obs-studio/obs-studio/obs-studio/deps/blake2/src -isystem /usr/include/x86_64-linux-gnu/qt6/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt6 -isystem /usr/include/x86_64-linux-gnu/qt6/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt6/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt6/QtGui -isystem /usr/include/x86_64-linux-gnu/qt6/QtSvg -isystem /usr/include/x86_64-linux-gnu/qt6/QtXml -isystem /usr/include/x86_64-linux-gnu/qt6/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt6/QtGui/6.2.4 -isystem /usr/include/x86_64-linux-gnu/qt6/QtGui/6.2.4/QtGui -isystem /usr/include/x86_64-linux-gnu/qt6/QtCore/6.2.4 -isystem /usr/include/x86_64-linux-gnu/qt6/QtCore/6.2.4/QtCore -isystem /usr/include/python3.10 -O2 -g -DNDEBUG -Werror -Wextra -Wvla -Wswitch -Wno-error=switch -Wformat -Wformat-security -Wunused-parameter -Wno-unused-function -Wno-missing-field-initializers -fno-strict-aliasing -Wconversion-null -fPIC -mmmx -msse -msse2 -std=c++17 -MD -MT UI/CMakeFiles/obs.dir/window-projector.cpp.o -MF UI/CMakeFiles/obs.dir/window-projector.cpp.o.d -o UI/CMakeFiles/obs.dir/window-projector.cpp.o -c /home/runner/work/obs-studio/obs-studio/obs-studio/UI/window-projector.cpp
/home/runner/work/obs-studio/obs-studio/obs-studio/UI/window-projector.cpp: In member function ‘virtual void OBSProjector::mouseReleaseEvent(QMouseEvent*)’:
/home/runner/work/obs-studio/obs-studio/obs-studio/UI/window-projector.cpp:315:51: error: unused parameter ‘event’ [-Werror=unused-parameter]
  315 | void OBSProjector::mouseReleaseEvent(QMouseEvent *event)
      |                                      ~~~~~~~~~~~~~^~~~~

since I suppose I can't change QT event function signature. Is there anyway to add exceptions?

@RytoEX
Copy link
Member

RytoEX commented Jan 24, 2023

In C++, you can simply omit the parameter name from the function signature if you're not going to use it, like so:

void OBSProjector::mouseReleaseEvent(QMouseEvent *)
{
	setCursor(Qt::ArrowCursor);
}

Otherwise, you would use UNUSED_PARAMETER at the top of the function definition.

@RytoEX RytoEX added the New Feature New feature or plugin label Jan 24, 2023
@PatTheMav
Copy link
Member

IMO the cursor should default to the resize variant when the projector is set to be borderless to clearly communicate the functionality a user can expect.

When the projector has a border, the functionality should be disabled, as the correct way to move a window is to use its title bar.

@fsworld009
Copy link
Author

@RytoEX thanks, I have updated PR and now drone builds are passed.

@PatTheMav I see your point, but since this PR is based on master branch, there's no borderless support yet, so I can't add what you asked. These requirements might need to be in a separate PR, or we need to wait until borderless support is merged.

@WizardCM WizardCM self-assigned this Jan 28, 2023
@WizardCM
Copy link
Member

Finally had some time to test this PR. Note I'm testing on Windows with Qt 6.4.2, with one display at a 125% DPI (though this happens on all 100% DPI screens).

2023-03-17_20-40-20.mp4

@fsworld009 fsworld009 marked this pull request as draft August 14, 2023 14:55
@fsworld009 fsworld009 force-pushed the projector-drag-window-to-move branch from e59b592 to 04694d5 Compare August 14, 2023 16:01
@fsworld009 fsworld009 marked this pull request as ready for review August 14, 2023 16:27
@fsworld009
Copy link
Author

@WizardCM Hi, I wasn't able to reproduce the issue you experienced, but I changed the implementation anyway, could you test again?

@fsworld009
Copy link
Author

08.14.2023 Update

Redco the PR with following changes:

  1. Use this->window()->move() to move window instead of this->move(). This is much smoother and works well by only computing diff based on mouse event position. Ref: https://stackoverflow.com/a/39821779/3973896
  2. Show drag icon when mouse is hovered on widget
  3. Disable drag to move when it's in full screen mode

@ogmkp
Copy link

ogmkp commented Nov 13, 2023

I can already do this move by holding down the ALT key on MATE desktop, will there be conflict?

@Warchamp7
Copy link
Member

Warchamp7 commented Apr 10, 2024

@fsworld009 I have the same problems with movement as @WizardCM. Seems more likely to happen at higher movement speeds / high DPIs. I use 5200 so it's very easy for me to cause it

Qt docs actually warn against this approach
https://doc.qt.io/qt-6/qwidget.html#pos-prop

Warning: Calling move() or setGeometry() inside moveEvent() can lead to infinite recursion.

The docs also suggestion using globalPosition() instead when moving a window via a widget

The position() function gives the cursor position relative to the widget or item that receives the mouse event. If you move the widget as a result of the mouse event, use the global position returned by globalPosition() to avoid a shaking motion.

@Warchamp7
Copy link
Member

Warchamp7 commented Apr 10, 2024

I've pushed a commit to this branch that uses globalPosition() instead which appears to fix the issue.

If this works fine for others, it can be squashed.

I also agree with @PatTheMav that I'd like to see this only apply to frameless mode, so I'll make further adjustments when #9430 is merged.

@Warchamp7 Warchamp7 added the UI/UX Anything to do with changes or additions to UI/UX elements. label Apr 10, 2024
@Warchamp7 Warchamp7 self-assigned this Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature New feature or plugin UI/UX Anything to do with changes or additions to UI/UX elements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants