Review (part 1)
libkpipewire
ScreencastingRequest
-
"ScreencastingSingleton::self()->::requestInterface(this);" is quite messy.
We don't need the async handling of the registry if we use QWaylandClientExtension. It'll kill a load of code too.
Then we get
ScreencastingRequest::create()
{
if (!ScreencastingSingleton::self()->isValid() return;
ScreencastingSingleton::self()->createWindowStream(...)
}
-
m_stream is unused.
If it is used it needs to be a QPointer as ScreencastingStream are self deleting
-
Q_EMIT closeRunningStreams(); connect(this, &ScreencastingRequest::closeRunningStreams, stream, &QObject::deleteLater);
Use of a signal to invoke methods on ourselves is weird.
-
KWayland::Client::Output *m_output = nullptr; is unused
stream->setObjectName(m_uuid);
This is set multiple times, you're overriding the object name set in screencasting->createWindowStream
KPipewireDeclarativePlugin
qmlRegisterUncreatableType<Screencasting>(uri, 0, 1, "Screencasting", "Use ScreencastingItem");
-
you mean PipeWireSourceItem ?
-
PipeWireSourceItem what happens if we're not running with EGL? Software renderer or alike.
-
please run through clang-format.
PipeWireSourceItem
ItemSceneChange
says we don't need to do this manually.
-
why do we do the path in m_needsRecreateTexture? It's not about recreating the texture, but about the node that holds the texture. We're rebuilding the texture every time by the looks of it.
We can call QSGImageNode::setTexture multiple times without recreating that.
-
Why are we toggling setEnabled ?
-
There's a codepath to draw a blue square
static EGLImage createImage(EGLDisplay display, const QVector &planes, uint32_t format, const QSize &size)
-
We would crash if planes is empty.
Edited by Aleix Pol Gonzalez