Differential D10234

LibInput: Queue native libinput events instead of LibInput::Event
AbandonedPublic
Actions

Authored by fvogt on Feb 1 2018, 3:56 PM.

Details

Reviewers
graesslin
Group Reviewers
Plasma
Summary

The KWin-native LibInput::Event type needs to know the KWin-native
LibInput::Device during construction. This is not guaranteed to be
known when the event queue is being filled.
So instead create the KWin-native Event only when it's needed.

BUG: 389674

Test Plan

kwin_wayland starts now without crashing.

Diff Detail

Repository
R108 KWin
Branch
Plasma/5.12
Lint
No Linters Available
Unit
No Unit Test Coverage
fvogt created this revision.Feb 1 2018, 3:56 PM
Restricted Application added a project: KWin. · View Herald TranscriptFeb 1 2018, 3:56 PM
Restricted Application added subscribers: KWin, kwin. · View Herald Transcript
fvogt requested review of this revision.Feb 1 2018, 3:56 PM
Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptFeb 1 2018, 3:56 PM
fvogt edited the summary of this revision. (Show Details)Feb 1 2018, 3:59 PM
Restricted Application edited projects, added KWin; removed Plasma. · View Herald TranscriptFeb 1 2018, 3:59 PM
graesslin added a comment.Feb 1 2018, 4:01 PM

I had another idea: when the Event gets processed and the m_device is null, we just update it again from the event.

Something like

if (!event->device()) {
    event->updateDevice();
}

with

void Device::updateDevice()
{
    m_device = Device::getDevice(libinput_event_get_device(m_event);
}
Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptFeb 1 2018, 4:01 PM
fvogt added a comment.Feb 1 2018, 4:27 PM
In D10234#199148, @graesslin wrote:

I had another idea: when the Event gets processed and the m_device is null, we just update it again from the event.

Something like

if (!event->device()) {
    event->updateDevice();
}

with

void Device::updateDevice()
{
    m_device = Device::getDevice(libinput_event_get_device(m_event);
}

I wouldn't do that manually, rather replace m_device references with a device() method and if m_device is nullptr, assign it lazily.

Restricted Application edited projects, added KWin; removed Plasma. · View Herald TranscriptFeb 1 2018, 4:27 PM
graesslin added a comment.Feb 1 2018, 4:39 PM
In D10234#199165, @fvogt wrote:
In D10234#199148, @graesslin wrote:

I had another idea: when the Event gets processed and the m_device is null, we just update it again from the event.

Something like

if (!event->device()) {
    event->updateDevice();
}

with

void Device::updateDevice()
{
    m_device = Device::getDevice(libinput_event_get_device(m_event);
}

I wouldn't do that manually, rather replace m_device references with a device() method and if m_device is nullptr, assign it lazily.

That's even better.

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptFeb 1 2018, 4:39 PM
graesslin added a comment.Feb 1 2018, 5:06 PM

alternative approach in D10236

Restricted Application edited projects, added KWin; removed Plasma. · View Herald TranscriptFeb 1 2018, 5:06 PM
mart added a subscriber: mart.Mar 23 2018, 2:58 PM

for @fvogt: as an alternative approach has been pushed, can this be closed?

fvogt abandoned this revision.Mar 23 2018, 6:38 PM
In D10234#232304, @mart wrote:

for @fvogt: as an alternative approach has been pushed, can this be closed?

Yup.

Revision Contents
Changeset List

PathPackages
Mlibinput/connection.h (4 lines)
Mlibinput/connection.cpp (12 lines)
Mlibinput/context.h (5 lines)
Mlibinput/context.cpp (4 lines)

Diff 26327

View Options

libinput/connection.h

Loading...
View Options

libinput/connection.cpp

Loading...
View Options

libinput/context.h

Loading...
View Options

libinput/context.cpp

Loading...