[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwin
Subject:    D5249: [RFC] New effect plugin - projector (keystone) correction
From:       Martin_Gräßlin <noreply () phabricator ! kde ! org>
Date:       2017-03-29 19:44:40
Message-ID: 20170329194437.20908.39021.988657FF () phabricator ! kde ! org
[Download RAW message or body]

graesslin added a comment.


  I think the effect system is the wrong place for it - at least if we want to add it \
to KWin directly. The transformation should be done directly in the Scene.

INLINE COMMENTS

> projector.cpp:101-126
> +    if (m_cursorTexture && m_cursorVisible) {
> +        QPoint p = effects->cursorPos();
> +        for (const ScreenData &screenData : m_screenData) {
> +            if ((screenData.number != -1) && (screenData.rect.contains(p))) {
> +                p = translatePoint(p.x() - screenData.rect.left(), p.y() - \
> screenData.rect.top(), +                                   \
> screenData.transMatrix).toPoint(); +                p.rx() += \
> screenData.rect.left();

This is something which we should not do. That has major impact on the performance of \
the overall system. Cursors are on an own layer and thus cursor movement does not \
require a repaint.

Also on X11 we are not able to perfectly track the cursor. On Wayland, though we have \
full control over it.

> projector.cpp:261-263
> +    // load the cursor-theme image from the Xcursor-library
> +    xcb_xfixes_get_cursor_image_cookie_t keks = \
> xcb_xfixes_get_cursor_image_unchecked(xcbConnection()); +    \
> xcb_xfixes_get_cursor_image_reply_t *ximg = \
> xcb_xfixes_get_cursor_image_reply(xcbConnection(), keks, 0);

unrelated to the fact that I think we shouldn't do cursor manipulation: we do have \
access to the cursor image in the effects API.

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D5249

To: nowicki, #plasma, graesslin
Cc: kwin, plasma-devel, #kwin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, \
abetts, sebas, apol


[Attachment #3 (text/html)]

<table><tr><td style="">graesslin added a comment.
</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/D5249" rel="noreferrer">View \
Revision</a></tr></table><br /><div><div><p>I think the effect system is the wrong \
place for it - at least if we want to add it to KWin directly. The transformation \
should be done directly in the Scene.</p></div></div><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/D5249#inline-21612" rel="noreferrer">View \
Inline</a><span style="color: #4b4d51; font-weight: \
bold;">projector.cpp:101-126</span></div> <div style="font: 11px/15px \
&quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, 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: \
#aa4000">if</span> <span class="p">(</span><span class="n">m_cursorTexture</span> \
<span style="color: #aa2211">&amp;&amp;</span> <span \
class="n">m_cursorVisible</span><span class="p">)</span> <span class="p">{</span> \
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, \
.6);">        <span class="n">QPoint</span> <span class="n">p</span> <span \
style="color: #aa2211">=</span> <span class="n">effects</span><span style="color: \
#aa2211">-&gt;</span><span class="n">cursorPos</span><span class="p">();</span> \
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, \
.6);">        <span style="color: #aa4000">for</span> <span class="p">(</span><span \
style="color: #aa4000">const</span> <span class="n">ScreenData</span> <span \
style="color: #aa2211">&amp;</span><span style="color: #a0a000">screenData</span> \
<span class="p">:</span> <span class="n">m_screenData</span><span class="p">)</span> \
<span class="p">{</span> </div><div style="padding: 0 8px; margin: 0 4px; background: \
rgba(151, 234, 151, .6);">            <span style="color: #aa4000">if</span> <span \
class="p">((</span><span class="n">screenData</span><span class="p">.</span><span \
class="n">number</span> <span style="color: #aa2211">!=</span> <span style="color: \
#aa2211">-</span><span style="color: #601200">1</span><span class="p">)</span> <span \
style="color: #aa2211">&amp;&amp;</span> <span class="p">(</span><span \
class="n">screenData</span><span class="p">.</span><span class="n">rect</span><span \
class="p">.</span><span class="n">contains</span><span class="p">(</span><span \
class="n">p</span><span class="p">)))</span> <span class="p">{</span> </div><div \
style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">          \
<span class="n">p</span> <span style="color: #aa2211">=</span> <span \
class="n">translatePoint</span><span class="p">(</span><span class="n">p</span><span \
class="p">.</span><span class="n">x</span><span class="p">()</span> <span \
style="color: #aa2211">-</span> <span class="n">screenData</span><span \
class="p">.</span><span class="n">rect</span><span class="p">.</span><span \
class="n">left</span><span class="p">(),</span> <span class="n">p</span><span \
class="p">.</span><span class="n">y</span><span class="p">()</span> <span \
style="color: #aa2211">-</span> <span class="n">screenData</span><span \
class="p">.</span><span class="n">rect</span><span class="p">.</span><span \
class="n">top</span><span class="p">(),</span> </div><div style="padding: 0 8px; \
margin: 0 4px; background: rgba(151, 234, 151, .6);">                                 \
<span class="n">screenData</span><span class="p">.</span><span \
class="n">transMatrix</span><span class="p">).</span><span \
class="n">toPoint</span><span class="p">();</span> </div><div style="padding: 0 8px; \
margin: 0 4px; background: rgba(151, 234, 151, .6);">                <span \
class="n">p</span><span class="p">.</span><span class="n">rx</span><span \
class="p">()</span> <span style="color: #aa2211">+=</span> <span \
class="n">screenData</span><span class="p">.</span><span class="n">rect</span><span \
class="p">.</span><span class="n">left</span><span class="p">();</span> \
</div></div></div> <div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; \
margin: 8px;">This is something which we should not do. That has major impact on the \
performance of the overall system. Cursors are on an own layer and thus cursor \
movement does not require a repaint.</p>

<p style="padding: 0; margin: 8px;">Also on X11 we are not able to perfectly track \
the cursor. On Wayland, though we have full control over it.</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/D5249#inline-21613" rel="noreferrer">View \
Inline</a><span style="color: #4b4d51; font-weight: \
bold;">projector.cpp:261-263</span></div> <div style="font: 11px/15px \
&quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, 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">// load the cursor-theme image from the Xcursor-library</span> </div><div \
style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span \
class="n">xcb_xfixes_get_cursor_image_cookie_t</span> <span class="n">keks</span> \
<span style="color: #aa2211">=</span> <span \
class="n">xcb_xfixes_get_cursor_image_unchecked</span><span class="p">(</span><span \
class="n">xcbConnection</span><span class="p">());</span> </div><div style="padding: \
0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span \
class="n">xcb_xfixes_get_cursor_image_reply_t</span> <span style="color: \
#aa2211">*</span><span class="n">ximg</span> <span style="color: #aa2211">=</span> \
<span class="n">xcb_xfixes_get_cursor_image_reply</span><span class="p">(</span><span \
class="n">xcbConnection</span><span class="p">(),</span> <span \
class="n">keks</span><span class="p">,</span> <span style="color: \
#601200">0</span><span class="p">);</span> </div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: \
8px;">unrelated to the fact that I think we shouldn&#039;t do cursor manipulation: we \
do have access to the cursor image in the effects \
API.</p></div></div></div></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D5249" \
rel="noreferrer">https://phabricator.kde.org/D5249</a></div></div><br \
/><div><strong>To: </strong>nowicki, Plasma, graesslin<br /><strong>Cc: \
</strong>kwin, plasma-devel, KWin, progwolff, lesliezhai, ali-mohamed, \
jensreuterberg, abetts, sebas, apol<br /></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic