[wayland] Don't use hardcoded move-resize cursor
ClosedPublic

Authored by zzag on Oct 30 2016, 11:38 AM.

Details

Summary

Currently, when resizing a window the cursor doesn't match the resize
direction. The reason for that is the move-resize cursor is hardcoded.

To fix that, CursorImage::updateMoveResize has to use AbstractClient::cursor.
Also, because the move-resize cursor is updated after calling startMoveResize,
we have to connect to AbstractClient::moveResizeCursorChanged.

BUG: 370339
FIXED-IN: 5.15

Diff Detail

Repository
R108 KWin
Branch
fix-hardcoded-cursor
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6362
Build 6380: arc lint + arc unit
broulik updated this revision to Diff 7753.Oct 30 2016, 11:38 AM
broulik retitled this revision from to [Pointer Input] Update cursor on moveResizeCursorChanged.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added a reviewer: KWin.
broulik set the repository for this revision to R108 KWin.
Restricted Application added a project: KWin. · View Herald TranscriptOct 30 2016, 11:38 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript

Could you please extend the test cases. I think it would go well into DecorationInputTest::testPressToMove, DecorationInputTest::testTapToMove, DecorationInputTest::testResizeOutsideWindow and the MoveResizeWindowTest

pointer_input.cpp
606

I think it's dangerous to remove the moveResizedChanged connection. It's possible that we lose a change there

Also it looks dangerous to me that we don't have any evaluation about workspace()->getMovingClient(). The moveResizeCursorChanged might be emitted in situations where no window is being move/resized

I think I'll need to add a getter for moveResizedCursor so I can access it also from moveResizedChanged where previously the cursor was hardcoded?

I think I'll need to add a getter for moveResizedCursor so I can access it also from moveResizedChanged where previously the cursor was hardcoded?

There is a getter already: AbstractClient::cursor - it was just a stupid mistake to use hardcoded values there.

How can I access the actual cursor shape? The client's cursor() was always right but the CursorImage was wrong. I don't know how to access CursorImage and it also doesn't even store the Qt::CursorShape

How can I access the actual cursor shape? The client's cursor() was always right but the CursorImage was wrong. I don't know how to access CursorImage and it also doesn't even store the Qt::CursorShape

The access to the cursor image is through the KWin::Platform

Platform::softwareCursor() returns the actual QImage and Platform::softwareCursorHotspot() the hotspot. With that you can verify whether it's the correct cursor. Example for the verification of the cursor can be found in scene_qpainter_test.cpp. Other example is in autotests/integration/pointer_input.cpp

What's important is to force the correct cursor theme:

whoops pressed return by accident.

What's important is to set the correct cursor theme through:

qputenv("XCURSOR_THEME", QByteArrayLiteral("DMZ-White"));
qputenv("XCURSOR_SIZE", QByteArrayLiteral("24"));

This patch got somehow forgotten. What's the current status?

davidedmundson requested changes to this revision.Jan 23 2018, 12:49 PM
davidedmundson added a subscriber: davidedmundson.

Marking as needs reply from author based on MGs' comments.

This revision now requires changes to proceed.Jan 23 2018, 12:49 PM
zzag commandeered this revision.Nov 26 2018, 9:41 AM
zzag added a reviewer: broulik.
zzag retitled this revision from [Pointer Input] Update cursor on moveResizeCursorChanged to [wayland] Update cursor on moveResizeCursorChanged.
zzag updated this revision to Diff 46270.Nov 26 2018, 4:18 PM

Don't use hardcoded resize cursor

zzag retitled this revision from [wayland] Update cursor on moveResizeCursorChanged to [wayland] Don't use hardcoded resize cursor.Nov 26 2018, 4:19 PM
zzag edited the summary of this revision. (Show Details)
zzag edited the test plan for this revision. (Show Details)
romangg requested changes to this revision.Dec 2 2018, 9:05 PM

It's now just the normal cursor when resizing with Alt + Right click drag.

This revision now requires changes to proceed.Dec 2 2018, 9:05 PM
zzag updated this revision to Diff 46760.Dec 2 2018, 10:30 PM

Listen to moveResizeCursorChanged.

The move-resize cursor is usually updated after clientStartUserMovedResized
or clientFinishUserMovedResized, so we need to listen to the corresponding
signal as well.

zzag added a comment.Dec 2 2018, 10:45 PM

Also, because we update cursor after calling startMoveResize, I think we don't need connect(c, &AbstractClient::moveResizedChanged, this, &CursorImage::updateMoveResize);.

zzag updated this revision to Diff 46767.Dec 3 2018, 8:27 AM

Connect only to moveResizeCursorChanged.

zzag updated this revision to Diff 46776.Dec 3 2018, 10:46 AM
zzag edited the summary of this revision. (Show Details)

Edit commit message.

zzag updated this revision to Diff 48197.Dec 25 2018, 6:20 PM

We still need moveResizedChanged

zzag updated this revision to Diff 48203.Dec 25 2018, 10:25 PM

Add tests

zzag edited the test plan for this revision. (Show Details)Dec 25 2018, 10:25 PM
zzag updated this revision to Diff 48206.Dec 25 2018, 10:33 PM

Use PlatformCursorImage instead of custom cursor image wrapper class

zzag updated this revision to Diff 48207.Dec 25 2018, 10:41 PM

Reformat test table

zzag retitled this revision from [wayland] Don't use hardcoded resize cursor to [wayland] Don't use hardcoded move-resize cursor.Dec 25 2018, 10:51 PM
zzag edited the summary of this revision. (Show Details)Dec 29 2018, 1:41 PM
graesslin accepted this revision.Dec 31 2018, 5:51 AM

Very nice idea with the unit test.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 31 2018, 10:35 AM
This revision was automatically updated to reflect the committed changes.