Thanks. Don't forget to add the isValid check to the KWin patches now. Maybe also add a comment to the KWayland code, what's the reason for the separate create() call and that we want to change this in KF6 via a virtualized create method.

In D20186#444431, @atulbi wrote:

In D20186#443596, @romangg wrote:

Nice work. Most of the stuff in libinputtouchpad.cpp and its header file are copy-paste from the Wayland backend. It would make sense to have a new abstract parent class such that the code is shared.

Initially, I also wanted to implement it that way, but it'll mess up existing code, and especially because LibinputTouchpad class already extends XlibTouchpad class. Or split out the common parts into a helper-class.

Can it multi-inherit?

The xml file is missing form the diff now. Don't forget to add it when pushing.

Note that keepassx != keepassxc.

Thanks. Having a two-level structure with system defaults (that can get updated via new versions) and static user-overrides would be nice, but I assume it's not easy to do with KGlobalaccel or a "replacement"?

In D20066#443692, @graesslin wrote:

You are aware that kglobalaccel doesn't support removing global shortcuts?

Nice work. Most of the stuff in libinputtouchpad.cpp and its header file are copy-paste from the Wayland backend. It would make sense to have a new abstract parent class such that the code is shared.

Fixed with 1f3fd6379079

• Rebase on master

In D13284#430583, @ngraham wrote:

@romangg, any chance you'd like to rebase the patch on current master and proceed with review?

Without going through it in detail the overall structure looks really nice now. I'm really happy with the integration in the new version, in particular that our normal AMS code path is used for mode switching. Wait for another +1 by someone who can test it on a live system. But from my side it's good to go.

Superseded by D19255.

No, it wouldn't. But you know what, fix your faulty patches yourselves. I have enough time wasted on this super-small issue. Get your priorities straight!

What's the use case? Why would someone want to enable only half tiling, but not quarter tiling?

Ok, maybe then change the wording in the paragraph a bit on comit. To me it sounded like there is still a problem going on, but it seems since the variable with improper name has been removed, everything is fine now. Your decision.

In D19255#418729, @davidedmundson wrote:

Maybe add a TODO comment on merge to the setLogicalSize (that it does not handle the size plus scale change correctly in every case).

Can you expand on what you mean?

Code makes sense, but what exactly do you mean by:

Also CCBUG: 400987

Maybe add a TODO comment on merge to the setLogicalSize (that it does not handle the size plus scale change correctly in every case).

I just tested it and it works. As you said in D19253 has same caveat as I pointed out not being in sync with wl_output::done event in case resolution and scale factor change at the same time such that logical size stays the same.

In D19253#418085, @davidedmundson wrote:

Long-term it would be nicer to have the synchronization directly in KWayland,

In terms of long term, I want to have explicit synchronization handled by kwin.
We should expose OutputInterface::done() to be explicit rather than being done implicitly in setBlah. The attempt to make things simpler actually ends up making things a lot harder to do properly.

Although this doesn't directly work, I assume because xdg_output property changes can come in on bind, while the wl_output properties are sent explicity. I'm leaving this diff as it is now again up for review, so we can discuss a proper solution. I fear we need to remodel the XdgOutputInterface class completely.

Instead of checking against xdg_output current values, check if wl_output's values have changed resulting in a done event.

Nicely structured. Great!

@bshah noted that there is prior conceptual work about Plasma on TVs (or in general with only very few inputs) here:
https://docs.google.com/document/d/18B8GfIdLWhceTkdyxs9-I5NPbn4duni-qO8kjX_xzl4/edit#

• Review comments

Will be pushed after 5.56 branched off.

In D15907#417286, @zzag wrote:

Alright, I'm done.

QRectF with negative size is untested.

• Rebase on master
• Review comments

Tests fixed with e96da56f8ced.

In D15907#417151, @zzag wrote:

Tests are not less important than the rest of the project. It's not good that we don't care about their readability.

Oh I do care about readability. Did you notice the huge diagram so auto test readers have a picture of what the test is doing?

I'm not convinced by "current autotest fails without the change on current master and works with the patch". Please make the test simpler. I don't think that it will take too much time to reorganize the test table.

What's not to be convinced here? It's a fact, not an argument. No, I won't rewrite the test. If you want the autotest differently, do it yourself.

27 autotests fail now.

In D15907#415347, @zzag wrote:

Looks good to me.

I think the test is too much complicated. Would it be simpler to have something like

QTest::addColumn<QRectF>("geometry");
QTest::addColumn<QPointF>("pos");
QTest::addColumn<bool>("contains");

?

• Long variable names

I'm fine with that. If later on we can use KScreen the config entry will become simply irrelevant.

In T10462#176648, @nicolasfella wrote:

What's missing here is the Tablet use-case since I'm not sure if Phone and Tablet are just the same thing or should be treated as separate patterns.

That depends a bit on your definition of Tablet.

A 7'' tablet is not much different than a phone, but a larger tablet with an attachable keyboard is a very different device

In D18570#415752, @davidedmundson wrote:

In this case it seems better to me to not write the EGLStreams support directly into the DRM Backend at all, but create a new EGLStream Backend on same level as DRM Backend. Th

That sounds pointless.
There's less of a difference between EGL streams and GBM than there is between GBM and QPainter.

Other aspect which is important to me long-term: is multi-GPU support of different vendors possible with the current patches or a separate backend? Current patches don't seem to allow that easily to add later on. Depending on DRM access in a separate backend it might be simpler to do later.

In T10502#176649, @nicolasfella wrote:

For a large tablet a 3x2 split is also thinkable. What changes with the number of splits is the way they are controlled.

Right, but how is the way changed?

Other cases where the window geometry on the phone changes is when the screen is rotated or the virtual keyboard is pushing the window like @mart proposed

In D18570#412443, @ekurzinger wrote:

In D18570#408853, @romangg wrote:

First off: a driver not supporting Atomic Mode Setting is nothing modern. Does Nvidia has a plan to improve in this regard?

I'm really not happy, that the DrmOutput::present is not run and we have to export these internals from DrmOutput to the EglStreamBackend, which replaces the flip with an EGL function call. Why does the Nvidia driver not integrate with the libdrm API in regards to page flipping? It should be possible to at least offer a wrapper around it or not?

Well, our DRM-KMS driver technically does support atomic modesetting, however currently we only allow the presentation of GPU-accessible buffers through the EGLDevice / EGLOutput extensions. In fact, these use atomic modesetting capabilities in their implementation. As far as I know this will remain the case until work on the Unix device memory allocation library is complete and while this is still on the table it's a ways off yet. The new backend really just uses DrmOutputs to track whether a page flip is pending. If we don't want to expose these internals, an alternative might be to track this separately from DrmOutput::m_pageFlipPending and maybe install a different page flip handler when using EGL streams that would just clear this flag (and re-start the compositor). I'll see if I can get something like that thrown together, it might be a bit cleaner, actually.

FYI there is previous discussion in T9780 in regards to touch window resize/move. The goal must be that whatever is he result here is usable in all IOUPs with primary or additional touch input. Therefore let us continue discussion there.

True. Anyway I'm not sure if changing the scale is something we want or if the KWin internal scale should really only be influenced by the resolution/size in a uniform way over all devices.

Because of the low to middle viewing distance and primary input device being keyboard and mouse the window border size can be "None" till "Normal". Currently it's "Normal" per default I believe but there was some discussion to set it to "None" in the past. Another topic is the buttons sizes, which is independent of this value.

I've created some sub-tasks to discuss definition of some common usage patterns we might want to concentrate first: Desktop, Phone, TV.

In T10462#175716, @dcz-purism wrote:

...
That's not quite right, there are many cases when the usage patterns change, but the output doesn't. Some examples:

• tablet has a keyboard attached,
• convertible switches modes,
• a gamer picks up a controller and moves from the keyboard chair to a couch farther away.

That's true. So the Wayland event message must be sent also on these hot-plug events or when the user does a manual override, what maybe warrants a separate protocol.

In T10502#176600, @nicolasfella wrote:

Window geometry can not be changed

Android has a split screen mode where two windows are tiled. In this case this assumption would not be true.

ping

The problem is not so easy to solve I believe now and needs a more holistic approach. Let's go back to the drawing board.