yukijoou/kwin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Avoid QPointer in return types of Input methods

Browse source 
QPointer is a really useful way to store a pointer over time.
It doesn't make have any value as a return value used by a short-lived
method.

There isn't a good copy constructor, it's effectively the same as
creating a new QWeakPointer reference that has to be cleaned up.

Testing if something is null is still the same. A new QPointer can be
made by the caller if it actually is needed.

Input handling is a very hot path called many times a frame, so it's
important to keep this light. focus() and at() are called a lot which
added up to slightly over 1% of CPU time when moving the mouse about.
This commit is contained in:
David Edmundson 2020-09-07 09:11:07 +01:00 committed by Vlad Zahorodnii
parent e563b83b43
commit 6acf35e4cc
7 changed files with 47 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
autotests/integration/pointer_constraints_test.cpp
View file

@ -211,7 +211,7 @@ void TestPointerConstraints::testConfinedPointer()
QVERIFY(unconfinedSpy2.isValid());
// activate it again, this confines again
workspace()->activateClient(static_cast<AbstractClient*>(input()->pointer()->focus().data()));
workspace()->activateClient(static_cast<AbstractClient*>(input()->pointer()->focus()));
QVERIFY(confinedSpy2.wait());
QCOMPARE(input()->pointer()->isConstrained(), true);
@ -220,7 +220,7 @@ void TestPointerConstraints::testConfinedPointer()
QVERIFY(unconfinedSpy2.wait());
QCOMPARE(input()->pointer()->isConstrained(), false);
// activate it again, this confines again
workspace()->activateClient(static_cast<AbstractClient*>(input()->pointer()->focus().data()));
workspace()->activateClient(static_cast<AbstractClient*>(input()->pointer()->focus()));
QVERIFY(confinedSpy2.wait());
QCOMPARE(input()->pointer()->isConstrained(), true);
@ -325,7 +325,7 @@ void TestPointerConstraints::testLockedPointer()
QVERIFY(lockedSpy2.isValid());
// activate the client again, this should lock again
workspace()->activateClient(static_cast<AbstractClient*>(input()->pointer()->focus().data()));
workspace()->activateClient(static_cast<AbstractClient*>(input()->pointer()->focus()));
QVERIFY(lockedSpy2.wait());
QCOMPARE(input()->pointer()->isConstrained(), true);

4
autotests/integration/pointer_input.cpp
View file

@ -1003,7 +1003,7 @@ void PointerInputTest::testCursorImage()
// move cursor to center of window, this should first set a null pointer, so we still show old cursor
cursor->setPos(window->frameGeometry().center());
QCOMPARE(p->focus().data(), window);
QCOMPARE(p->focus(), window);
QCOMPARE(cursor->image(), fallbackCursor);
QVERIFY(enteredSpy.wait());
@ -1058,7 +1058,7 @@ void PointerInputTest::testCursorImage()
// move cursor somewhere else, should reset to fallback cursor
Cursors::self()->mouse()->setPos(window->frameGeometry().bottomLeft() + QPoint(20, 20));
QVERIFY(p->focus().isNull());
QVERIFY(!p->focus());
QVERIFY(!cursor->image().isNull());
QCOMPARE(cursor->image(), fallbackCursor);
}

26
autotests/integration/window_selection_test.cpp
View file

@ -102,7 +102,7 @@ void TestWindowSelection::testSelectOnWindowPointer()
QVERIFY(client);
QVERIFY(keyboardEnteredSpy.wait());
KWin::Cursors::self()->mouse()->setPos(client->frameGeometry().center());
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
QVERIFY(pointerEnteredSpy.wait());
Toplevel *selectedWindow = nullptr;
@ -129,11 +129,11 @@ void TestWindowSelection::testSelectOnWindowPointer()
// should not have ended the mode
QCOMPARE(input()->isSelectingWindow(), true);
QVERIFY(!selectedWindow);
QVERIFY(input()->pointer()->focus().isNull());
QVERIFY(!input()->pointer()->focus());
// updating the pointer should not change anything
input()->pointer()->update();
QVERIFY(input()->pointer()->focus().isNull());
QVERIFY(!input()->pointer()->focus());
// updating keyboard should also not change
input()->keyboard()->update();
@ -147,7 +147,7 @@ void TestWindowSelection::testSelectOnWindowPointer()
kwinApp()->platform()->pointerButtonReleased(BTN_LEFT, timestamp++);
QCOMPARE(input()->isSelectingWindow(), false);
QCOMPARE(selectedWindow, client);
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
// should give back keyboard and pointer
QVERIFY(pointerEnteredSpy.wait());
if (keyboardEnteredSpy.count() != 2) {
@ -227,7 +227,7 @@ void TestWindowSelection::testSelectOnWindowKeyboard()
kwinApp()->platform()->keyboardKeyPressed(key, timestamp++);
QCOMPARE(input()->isSelectingWindow(), false);
QCOMPARE(selectedWindow, client);
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
// should give back keyboard and pointer
QVERIFY(pointerEnteredSpy.wait());
if (keyboardEnteredSpy.count() != 2) {
@ -323,7 +323,7 @@ void TestWindowSelection::testCancelOnWindowPointer()
QVERIFY(client);
QVERIFY(keyboardEnteredSpy.wait());
KWin::Cursors::self()->mouse()->setPos(client->frameGeometry().center());
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
QVERIFY(pointerEnteredSpy.wait());
Toplevel *selectedWindow = nullptr;
@ -350,7 +350,7 @@ void TestWindowSelection::testCancelOnWindowPointer()
kwinApp()->platform()->pointerButtonReleased(BTN_RIGHT, timestamp++);
QCOMPARE(input()->isSelectingWindow(), false);
QVERIFY(!selectedWindow);
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
// should give back keyboard and pointer
QVERIFY(pointerEnteredSpy.wait());
if (keyboardEnteredSpy.count() != 2) {
@ -382,7 +382,7 @@ void TestWindowSelection::testCancelOnWindowKeyboard()
QVERIFY(client);
QVERIFY(keyboardEnteredSpy.wait());
KWin::Cursors::self()->mouse()->setPos(client->frameGeometry().center());
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
QVERIFY(pointerEnteredSpy.wait());
Toplevel *selectedWindow = nullptr;
@ -408,7 +408,7 @@ void TestWindowSelection::testCancelOnWindowKeyboard()
kwinApp()->platform()->keyboardKeyPressed(KEY_ESC, timestamp++);
QCOMPARE(input()->isSelectingWindow(), false);
QVERIFY(!selectedWindow);
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
// should give back keyboard and pointer
QVERIFY(pointerEnteredSpy.wait());
if (keyboardEnteredSpy.count() != 2) {
@ -441,7 +441,7 @@ void TestWindowSelection::testSelectPointPointer()
QVERIFY(client);
QVERIFY(keyboardEnteredSpy.wait());
KWin::Cursors::self()->mouse()->setPos(client->frameGeometry().center());
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
QVERIFY(pointerEnteredSpy.wait());
QPoint point;
@ -475,11 +475,11 @@ void TestWindowSelection::testSelectPointPointer()
// should not have ended the mode
QCOMPARE(input()->isSelectingWindow(), true);
QCOMPARE(point, QPoint());
QVERIFY(input()->pointer()->focus().isNull());
QVERIFY(!input()->pointer()->focus());
// updating the pointer should not change anything
input()->pointer()->update();
QVERIFY(input()->pointer()->focus().isNull());
QVERIFY(!input()->pointer()->focus());
// updating keyboard should also not change
input()->keyboard()->update();
@ -493,7 +493,7 @@ void TestWindowSelection::testSelectPointPointer()
kwinApp()->platform()->pointerButtonReleased(BTN_LEFT, timestamp++);
QCOMPARE(input()->isSelectingWindow(), false);
QCOMPARE(point, input()->globalPointer().toPoint());
QCOMPARE(input()->pointer()->focus().data(), client);
QCOMPARE(input()->pointer()->focus(), client);
// should give back keyboard and pointer
QVERIFY(pointerEnteredSpy.wait());
if (keyboardEnteredSpy.count() != 2) {

16
input.cpp
View file

@ -1318,7 +1318,7 @@ public:
if (event->type() != QEvent::MouseButtonPress) {
return false;
}
AbstractClient *c = dynamic_cast<AbstractClient*>(input()->pointer()->focus().data());
AbstractClient *c = dynamic_cast<AbstractClient*>(input()->pointer()->focus());
if (!c) {
return false;
}
@ -1333,7 +1333,7 @@ public:
// only actions on vertical scroll
return false;
}
AbstractClient *c = dynamic_cast<AbstractClient*>(input()->pointer()->focus().data());
AbstractClient *c = dynamic_cast<AbstractClient*>(input()->pointer()->focus());
if (!c) {
return false;
}
@ -1350,7 +1350,7 @@ public:
if (seat->isTouchSequence()) {
return false;
}
AbstractClient *c = dynamic_cast<AbstractClient*>(input()->touch()->focus().data());
AbstractClient *c = dynamic_cast<AbstractClient*>(input()->touch()->focus());
if (!c) {
return false;
}
@ -2718,6 +2718,16 @@ void InputDeviceHandler::update()
}
}
Toplevel *InputDeviceHandler::at() const
{
return m_at.at.data();
}
Toplevel *InputDeviceHandler::focus() const
{
return m_focus.focus.data();
}
QWindow* InputDeviceHandler::findInternalWindow(const QPoint &pos) const
{
if (waylandServer()->isScreenLocked()) {

13
input.h
View file

@ -417,18 +417,19 @@ public:
* @brief First Toplevel currently at the position of the input device
* according to the stacking order.
* @return Toplevel* at device position.
*
* This will be null if no toplevel is at the position
*/
QPointer<Toplevel> at() const {
return m_at.at;
}
Toplevel *at() const;
/**
* @brief Toplevel currently having pointer input focus (this might
* be different from the Toplevel at the position of the pointer).
* @return Toplevel* with pointer focus.
*
* This will be null if no toplevel has focus
*/
QPointer<Toplevel> focus() const {
return m_focus.focus;
}
Toplevel *focus() const;
/**
* @brief The Decoration currently receiving events.
* @return decoration with pointer focus.

14
pointer_input.cpp
View file

@ -179,7 +179,7 @@ void PointerInputRedirection::updateToReset()
setDecoration(nullptr);
}
if (focus()) {
if (AbstractClient *c = qobject_cast<AbstractClient*>(focus().data())) {
if (AbstractClient *c = qobject_cast<AbstractClient*>(focus())) {
c->leaveEvent();
}
disconnect(m_focusGeometryConnection);
@ -636,7 +636,7 @@ void PointerInputRedirection::setEnableConstraints(bool set)
void PointerInputRedirection::updatePointerConstraints()
{
if (focus().isNull()) {
if (!focus()) {
return;
}
const auto s = focus()->surface();
@ -660,7 +660,7 @@ void PointerInputRedirection::updatePointerConstraints()
}
return;
}
const QRegion r = getConstraintRegion(focus().data(), cf.data());
const QRegion r = getConstraintRegion(focus(), cf.data());
if (canConstrain && r.contains(m_pos.toPoint())) {
cf->setConfined(true);
m_confined = true;
@ -674,7 +674,7 @@ void PointerInputRedirection::updatePointerConstraints()
return;
}
const auto cf = s->confinedPointer();
if (!getConstraintRegion(focus().data(), cf.data()).contains(m_pos.toPoint())) {
if (!getConstraintRegion(focus(), cf.data()).contains(m_pos.toPoint())) {
// pointer no longer in confined region, break the confinement
cf->setConfined(false);
m_confined = false;
@ -706,7 +706,7 @@ void PointerInputRedirection::updatePointerConstraints()
}
return;
}
const QRegion r = getConstraintRegion(focus().data(), lock.data());
const QRegion r = getConstraintRegion(focus(), lock.data());
if (canConstrain && r.contains(m_pos.toPoint())) {
lock->setLocked(true);
m_locked = true;
@ -782,7 +782,7 @@ QPointF PointerInputRedirection::applyPointerConfinement(const QPointF &pos) con
return pos;
}
const QRegion confinementRegion = getConstraintRegion(focus().data(), cf.data());
const QRegion confinementRegion = getConstraintRegion(focus(), cf.data());
if (confinementRegion.contains(pos.toPoint())) {
return pos;
}
@ -1338,7 +1338,7 @@ void CursorImage::reevaluteSource()
setSource(CursorSource::Decoration);
return;
}
if (!m_pointer->focus().isNull() && waylandServer()->seat()->focusedPointer()) {
if (m_pointer->focus() && waylandServer()->seat()->focusedPointer()) {
setSource(CursorSource::PointerSurface);
return;
}

4
touch_input.cpp
View file

@ -109,11 +109,11 @@ void TouchInputRedirection::focusUpdate(Toplevel *focusOld, Toplevel *focusNow)
seat->setFocusedTouchSurface(focusNow->surface(), -1 * focusNow->inputTransformation().map(focusNow->pos()) + focusNow->pos());
m_focusGeometryConnection = connect(focusNow, &Toplevel::frameGeometryChanged, this,
[this] {
if (focus().isNull()) {
if (!focus()) {
return;
}
auto seat = waylandServer()->seat();
if (focus().data()->surface() != seat->focusedTouchSurface()) {
if (focus()->surface() != seat->focusedTouchSurface()) {
return;
}
seat->setFocusedTouchSurfacePosition(-1 * focus()->inputTransformation().map(focus()->pos()) + focus()->pos());