<table><tr><td style="">romangg 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/D26311">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D26311#585860" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D26311#585860</a>, <a href="https://phabricator.kde.org/p/davidedmundson/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@davidedmundson</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>Good start, the general approach is ok, but it needs a little tweak.</p>
<hr class="remarkup-hr" />
<p>I don't like OutputDeviceInterface::setLogicalSize.</p>
<p>Currently we have a pixel size and a scale, which gives us the logical size.<br />
By adding an explicitly setLogicalSize we have this 3 way binding loop that is very confusing.</p>
<p>Having an explicitly set logical size with a scale right now would break that glViewport, and it's semantically confusing as to what the compositor view size is.</p></div>
</blockquote>
<p>What do you mean with "compositor view size"? The size of the here introduced view geometry?</p>
<p>If the logical size is set explicitly the scale and mode size become irrelevant to what area the output covers in compositor space. I don't find that too confusing per se. But maybe the name should be changed to <tt style="background: #ebebeb; font-size: 13px;">setLogicalSizeOverride</tt> or something like that?</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;">
<hr class="remarkup-hr" />
<p>I think all of this can be remedied if we change the output device method to be<br />
OutputDevice::setVirtualMode(QSize)</p>
<p>Which acts as a clip rect for where we render the plane as you have here, then have geometry() can use that instead of pixelSize.</p>
<p>Then scale/positioning etc. will all just work intuitively without extra semantics, egl_gbm_backend.cpp wouldn't need changing.</p></blockquote>
<p>Interesting approach. I assume you would like to change the protocol request from a <tt style="background: #ebebeb; font-size: 13px;">logical_size</tt> setter to a <tt style="background: #ebebeb; font-size: 13px;">virtual_mode</tt> one as well, right?</p>
<p>When using this for replicating one output source onto another (target) the calculation becomes quite difficult I believe though. The client needs to factor in:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">Current logical size of source output</li>
<li class="remarkup-list-item">Current mode of target output</li>
<li class="remarkup-list-item">Current scale of target output</li>
<li class="remarkup-list-item">Current transform of target output</li>
</ul>
<hr class="remarkup-hr" />
<p>For example let's assume we have two outputs:</p>
<ol class="remarkup-list">
<li class="remarkup-list-item">Ultrabook 13'' source output: 1920x1200, scale 1.2, rotation landscape</li>
<li class="remarkup-list-item">TV 50'' target output: 4K, scale 1, rotation portrait</li>
</ol>
<p>For the setVirtualMode argument we need to find a value such that what is displayed on the source output is also displayed on the target output while respecting the target rotation and scale. But there is another problem: the scale of the target is dependent on the source logical size and target resolution and must be changed as well then.</p>
<p>Logical size of source is 1600x1000. Current mode of target is 3860x2160, but with the rotation what we need to compare the logical size of the source against is actually 2160x3860. To fit it in the virtual mode would be 2160x1350 (by <tt style="background: #ebebeb; font-size: 13px;">2160/1600*1000 = 1350</tt>). The scaling factor is 1.35. To have sharp images we need an integer scale factor 2 on one of the outputs. Output (1) has this so we do not need to set it on (2).</p>
<p><em>Now user changes the scale factor of output (1) to 1.</em></p>
<p>Logical size of source is now 1920x1200. Virtual mode would still be 2160 x 1350 (by <tt style="background: #ebebeb; font-size: 13px;">2160/1920*1200 = 1350</tt>). The scaling factor is 1.125. For sharp images still an integer scale factor of 2 is needed, so either output (1) or (2) needs to be set to this. Since a change of scale in (1) would alter the depiction overall we need to change it on (2). This would need to be done either via a scale override independent of what is set in OutputDevice or we send this new scale factor via the protocol to KScreen. Should it then ignore this scale factor change?</p>
<p><em>Now user changes rotation of output (2) to landscape.</em></p>
<p>Now client needs to fit logical size 1920x1200 into 3860x2160. Virtual mode is now 3456 x 2160 (by <tt style="background: #ebebeb; font-size: 13px;">2160/1200*1920 = 3456</tt>). Scale factor is 1.8. Like before integer scale factor of output (2) needs to be set to this.</p>
<hr class="remarkup-hr" />
<p>Let's compare with setting the logical size:</p>
<p>Client sends logical size of 1600x1000 for output (2).</p>
<p><em>Now user changes the scale factor of output (1) to 1.</em></p>
<p>Client sends logical size of 1920x1200 for output (2).</p>
<p><em>Now user changes rotation of output (2) to landscape.</em></p>
<p>Client sends logical size of 1200x1920 for output (2).</p>
<hr class="remarkup-hr" />
<p>In the use case of output replication the API is simpler because semantically it makes more sense to directly set what area <em>should be replicated</em> and not what the end result on the output hardware must look like for that to happen. On a conceptual level weston and mutter do something similar by differentiating between the two separate structs outputs and heads.</p>
<p>Note that above steps I outlined are missing in current patches (setting the wl_output scale factor accordingly when logical size is set and updating logical size on replicas when source output resolution and/or scale is changed).</p></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/D26311">https://phabricator.kde.org/D26311</a></div></div><br /><div><strong>To: </strong>romangg, KWin, davidedmundson<br /><strong>Cc: </strong>davidedmundson, kwin, LeGast00n, The-Feren-OS-Dev, sbergeron, jraleigh, zachus, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart<br /></div>