<table><tr><td style="">romangg added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D18824">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18824#inline-104868">View Inline</a><span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:349</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> xdgoutput_interface.h
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> eglstream_controller_interface.h
</div><div style="padding: 0 8px; margin: 0 4px; ">)
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">alphabetical sorted</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18824#inline-104872">View Inline</a><span style="color: #4b4d51; font-weight: bold;">display.cpp:529</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span> <span style="color: #aa4000">else</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qCWarning</span><span class="p">(</span><span class="n">KWAYLAND_SERVER</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Unable to load libnvidia-egl-wayland.so.1"</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">There should be always a non-null object returned. Instead check somewhere before the create call of EglStreamControllerInterface if the library can be loaded. Also this would put the logic and the QLibrary include in the class file and not in display.cpp, what I would prefer.</p>
<p style="padding: 0; margin: 8px;">One could overwrite the <tt style="background: #ebebeb; font-size: 13px;">Global::create</tt> method in the <tt style="background: #ebebeb; font-size: 13px;">EglStreamControllerInterface</tt> child class. From there then call QLibrary::resolve and only if it succeeds the parent create method. Afterwards compositor needs to check <tt style="background: #ebebeb; font-size: 13px;">Global::isValid</tt>.</p>
<p style="padding: 0; margin: 8px;">The control flow would then be in the compositor:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">auto *e = display->createEglStreamControllerInterface();
// other initial setup
e->create();
if (!e->isValid()) {
// error handling
}</pre></div>
<p style="padding: 0; margin: 8px;">It's unfortunate that the create method is not virtual in Global. We can change this in KF6.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18824#inline-104869">View Inline</a><span style="color: #4b4d51; font-weight: bold;">display.h:310</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * @return the created EGL Stream controller, or nullptr on failure</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * @since 5.55</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">5.56</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18824#inline-104886">View Inline</a><span style="color: #4b4d51; font-weight: bold;">eglstream_controller_interface.cpp:53</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_UNUSED</span><span class="p">(</span><span class="n">client</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_UNUSED</span><span class="p">(</span><span class="n">attribs</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Private</span> <span style="color: #aa2211">*</span><span class="n">p</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">static_cast</span><span style="color: #aa2211"><</span><span class="n">Private</span> <span style="color: #aa2211">*></span><span class="p">(</span><span class="n">wl_resource_get_user_data</span><span class="p">(</span><span class="n">resource</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Why is attribs unused in this implementation?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18824#inline-104885">View Inline</a><span style="color: #4b4d51; font-weight: bold;">eglstream_controller_interface.cpp:54</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_UNUSED</span><span class="p">(</span><span class="n">attribs</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Private</span> <span style="color: #aa2211">*</span><span class="n">p</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">static_cast</span><span style="color: #aa2211"><</span><span class="n">Private</span> <span style="color: #aa2211">*></span><span class="p">(</span><span class="n">wl_resource_get_user_data</span><span class="p">(</span><span class="n">resource</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">emit</span> <span class="n">p</span><span style="color: #aa2211">-></span><span class="n">m_controller</span><span style="color: #aa2211">-></span><span class="n">streamConsumerAttached</span><span class="p">(</span><span class="n">SurfaceInterface</span><span style="color: #aa2211">::</span><span class="n">get</span><span class="p">(</span><span class="n">surface</span><span class="p">),</span> <span class="p">(</span><span style="color: #aa4000">void</span> <span style="color: #aa2211">*</span><span class="p">)</span><span class="n">eglStream</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Use <tt style="background: #ebebeb; font-size: 13px;">Global::Private::cast</tt>.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18824#inline-104881">View Inline</a><span style="color: #4b4d51; font-weight: bold;">eglstream_controller_interface.cpp:58</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">EglStreamControllerInterface</span><span style="color: #aa2211">::</span><span class="n">Private</span><span style="color: #aa2211">::</span><span class="n">Private</span><span class="p">(</span><span class="n">EglStreamControllerInterface</span> <span style="color: #aa2211">*</span><span class="n">controller</span><span class="p">,</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Display</span> <span style="color: #aa2211">*</span><span class="n">display</span><span class="p">,</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Rename <tt style="background: #ebebeb; font-size: 13px;">controller</tt> and <tt style="background: #ebebeb; font-size: 13px;">m_controller</tt> below to <tt style="background: #ebebeb; font-size: 13px;">q</tt> for enhanced pimpl street cred.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18824#inline-104879">View Inline</a><span style="color: #4b4d51; font-weight: bold;">eglstream_controller_interface.h:28</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#include</span> <span class="cpf">"surface_interface.h"</span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">namespace</span> <span class="n">KWayland</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Cleanup includes</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R127 KWayland</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18824">https://phabricator.kde.org/D18824</a></div></div><br /><div><strong>To: </strong>ekurzinger, romangg, davidedmundson, zzag, KWin<br /><strong>Cc: </strong>kde-frameworks-devel, michaelh, ngraham, bruns<br /></div>