davidedmundson updated this revision to Diff 30800.
davidedmundson added a comment.
Changed to a version that only caches names as requested.
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D7481?vs=18588=30800
BRANCH
master
REVISION DETAIL
davidedmundson added inline comments.
INLINE COMMENTS
> broulik wrote in xwindowsystemeventbatcher.cpp:35
> Why emit a window change just before you emit a removal? Or is that what
> `KWindowSystem` usually does and we rely on that?
In a version where we cache all properties, I didn't want to
broulik added a comment.
Monthly ping :)
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc: ngraham, cfeck, broulik, hein, graesslin, plasma-devel, ragreen, ZrenBot,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas,
broulik added a comment.
Pingeliping.
INLINE COMMENTS
> xwindowsystemeventbatcher.cpp:34
> +connect(KWindowSystem::self(), ::windowRemoved, this,
> [this](WId wid) {
> +if (m_cache.contains(wid)) {
> +emit windowChanged(wid, m_cache[wid].properties,
>
broulik added a comment.
Ping.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc: ngraham, cfeck, broulik, hein, graesslin, plasma-devel, ZrenBot, progwolff,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol,
ngraham edited the summary of this revision.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc: ngraham, cfeck, broulik, hein, graesslin, plasma-devel, ZrenBot, progwolff,
lesliezhai, ali-mohamed, jensreuterberg, abetts,
ngraham added a comment.
@davidedmundson, what are our next steps here?
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc: ngraham, cfeck, broulik, hein, graesslin, plasma-devel, ZrenBot, progwolff,
lesliezhai,
hein added a comment.
David, ping?
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc: ngraham, cfeck, broulik, hein, graesslin, plasma-devel, ZrenBot, progwolff,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas,
hein added a comment.
Yes it is, pending rework.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc: ngraham, cfeck, broulik, hein, graesslin, plasma-devel, ZrenBot, progwolff,
lesliezhai, ali-mohamed, jensreuterberg,
cfeck added a comment.
Is there a consensus if this is still needed?
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc: cfeck, broulik, hein, graesslin, plasma-devel, ZrenBot, progwolff,
lesliezhai, ali-mohamed,
hein added a comment.
I had a feeling it was like that, WM::Name always coming from KWin seemed
weird somehow.
Ok, so we need the name props and selectively icon.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc:
graesslin added a comment.
In https://phabricator.kde.org/D7481#139276, @hein wrote:
> Sounds good, then we really only care about the title and icon.
>
> Let's do the title in KWin, then, AIUI? But we still need this patch for
the icon. But we could now limit it to windows that are
hein added a comment.
Sounds good, then we really only care about the title and icon.
Let's do the title in KWin, then, AIUI? But we still need this patch for the
icon. But we could now limit it to windows that are actually on the fallback
path that does anything in response to icon
graesslin added a comment.
Window class is not allowed to change by ICCCM. For desktop file we could
decide that we don't monitor changes as it is our own property.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc:
hein added a comment.
I'm not sure we only care about the title.
First of, icons: libtm usually ignores NET::WMIcon changes. It only looks at
them for windows which hit the fallback path of using the WM icon. Of course a
malicious window might be particularly likely to fall into that
graesslin added a comment.
@hein, @davidedmundson if the main concern is too often update of title I
suggest we implement limiting of updates in KWin. So the batch would happen in
KWin, taken away stress from KWin and indirectly also from Plasma.
Against updating icons it won't help,
graesslin added a comment.
> The reason the old lib did this and the reason I'm aware of for wanting
this is to prevent the TM being ddos'd by a rogue window updating its title
thousands of times per second. How are we protected from this on Wayland
currently?
By ensuring KWin does a
graesslin added a comment.
In https://phabricator.kde.org/D7481#138822, @broulik wrote:
> > Theproblem here is Firefox: that one should be fixed.
>
> I can also cause trouble with VLC if I have a repeating playlist with a
broken file in it, it will continuously cycle between "$title"
hein added a comment.
In https://phabricator.kde.org/D7481#138817, @davidedmundson wrote:
> We don't want it for Wayland.
>
> Surfaces are double buffered so all synced. We don't fully do that on the
XDG side, but most of that is static anyway
Can you explain that more? The
broulik added a comment.
> Theproblem here is Firefox: that one should be fixed.
I can also cause trouble with VLC if I have a repeating playlist with a
broken file in it, it will continuously cycle between "$title" and "VLC" and
cause Plasma to freeze. We can't fix all apps…
davidedmundson added a comment.
We don't want it for Wayland.
Surfaces are double buffered so all synced. We don't fully do that on the XDG
side, but most of that is static anyway
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To:
hein added a comment.
I'm in general for this. It's listed in the TODO file as one of the remaining
open tasks for the lib, as the old lib did this but it got dropped along the
way.
However, I would like it to be implemented as a proxy pass (e.g. in
WindowTasksModel) so it can be
graesslin added a comment.
In general I'm against this as we would also need the same in KWin and every
other user. Theproblem here is Firefox: that one should be fixed.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7481
To: davidedmundson, #plasma
Cc:
davidedmundson created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
REVISION SUMMARY
In a log from someone talking about high CPU we can see get multiple X
events for the same window as multiple events, but directly
24 matches
Mail list logo
