| fvogt |
| KWin | |
| Plasma |
The KWindowSystem call which we used doesn't work on Wayland as it's only
implemented in the xcb variant and cannot be made available for Wayland
in an easy way as it is still XLib based.
This change turns the optional XCB-ICCCM dependency in a required one
and thus can use the functionality provided by said library to implement
what KWindowSystem provided.
BUG: 382789
New test case which failed with old code
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| client.cpp | ||
|---|---|---|
| 1447 | Missing a simplified() | |
Built and tested - works fine! I only added two rather nitpicky comments.
| client.cpp | ||
|---|---|---|
| 1442 | Why not directly QString::fromUtf8(reply.name, reply.name_len) here? It would save one unnecessary copy. Alternatively, QByteArray::fromRawData(reply.name, reply.name_len), but that's more verbose. | |
| 1447 | This omits the call to QString::simplified that is done in the old rev and for the info->name() case. Intentional? | |
| client.cpp | ||
|---|---|---|
| 1442 | fromRawData is dangerous as we delete the data with the wipe call again. That's why I didn't use it. QString::fromUtf8 is something I didn't use as QString from ascii is discouraged and can result in compile error depending on the compile flags. So in short: the copy is intended. | |
| client.cpp | ||
|---|---|---|
| 1442 | QString::fromUtf8 is always available and has nothing to do at all with the CAST_(FROM|TO)_ASCII macros. | |
| client.cpp | ||
|---|---|---|
| 1442 | QByteArray has a move constructor, it's not copy. | |
| client.cpp | ||
|---|---|---|
| 1442 | That does not matter in this instance. QString::fromUtf8(QByteArray(reply.name, reply.name_len)); does two copies:
The first copy is unnecessary. | |
| client.cpp | ||
|---|---|---|
| 1442 | I'll keep the second copy nevertheless. After all this mess with Qt 5 transition I don't trust Qt in the area of QString from char array. I rather have here a copy too much, which won't matter, then a potential breakage when going to Qt 6. | |
Still works fine here, but I can't really comment on the sanity of the code changes.
| client.cpp | ||
|---|---|---|
| 1442 | If Qt6 would introduce UTF-8 encoded QString (so that QString::fromUtf8() would no longer need to create a new array), then you can expect all Qt software to break. QString::fromUtf8() is used nearly everywhere. | |
| client.cpp | ||
|---|---|---|
| 1442 | Just to add a little bit of context here: we are discussing two memcopies here in a code path which performs a roundtrip to the X server. This thing is going to be slow and the memcopy does not matter at all. Yes the code is not perfect, but it also doesn't matter. If someone feels like changing the code, feel free. | |
| client.cpp | ||
|---|---|---|
| 1442 | Sure, I don't want to block your code. Just wanted to let you know that you are rejecting Fabian's proposals for emotional reasons, not for technical reasons. | |
Nobody seems to be against it and the code looks good to me + it works fine here. So I don't see a reason to let this laying around for longer.