<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/D12388">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/D12388#inline-62843">View Inline</a><span style="color: #4b4d51; font-weight: bold;">zzag</span> wrote in <span style="color: #4b4d51; font-weight: bold;">outputconfiguration.cpp:184</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Maybe <tt style="background: #ebebeb; font-size: 13px;">static_cast</tt>?</p>

<p style="padding: 0; margin: 8px;">I haven't used wl_array but can't you allocate big enough contiguous chunk of memory and call memcpy, e.g.</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="cpp" 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);"><span class="n">wl_array</span> <span class="n">wlRed</span><span class="p">;</span>

<span class="n">wl_array_init</span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">wlRed</span><span class="p">);</span>
<span style="color: #aa4000">auto</span><span style="color: #aa2211">*</span> <span class="n">redDest</span> <span style="color: #aa2211">=</span> <span class="n">wl_array_add</span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">wlRed</span><span class="p">,</span> <span style="color: #aa4000">sizeof</span><span class="p">(</span><span style="color: #aa4000">uint16_t</span><span class="p">)</span> <span style="color: #aa2211">*</span> <span class="n">red</span><span class="p">.</span><span class="n">count</span><span class="p">());</span>
<span class="n">memcpy</span><span class="p">(</span><span class="n">redDest</span><span class="p">,</span> <span class="n">red</span><span class="p">.</span><span class="n">data</span><span class="p">(),</span> <span style="color: #aa4000">sizeof</span><span class="p">(</span><span style="color: #aa4000">uint16_t</span><span class="p">)</span> <span style="color: #aa2211">*</span> <span class="n">red</span><span class="p">.</span><span class="n">count</span><span class="p">());</span></pre></div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Good point about allocating directly all of the memory. I looked up the wl_array definition and should be no problem.</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/D12388#inline-62835">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cfeck</span> wrote in <span style="color: #4b4d51; font-weight: bold;">outputconfiguration.h:211</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">The documentation could state how many elements need to be in the vectors. Ideally, every component could have any number of elements, and if there are too few, the other elements are interpolated.</p>

<p style="padding: 0; margin: 8px;">But it is probably simpler to just state "The number of elements in each vector must be 256 or 1024, depending on the depth of the framebuffer (24 bits or 30 bits)."</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Number of elements depends on the OutputDevice technical possibilities (i.e. for example as you said 256 for 8bit color ramps and 1024 for 10bit ones and every other value between 1 and 16bit ramp size).</p>

<p style="padding: 0; margin: 8px;">Will add a comment to the doc.</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/D12388#inline-62839">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cfeck</span> wrote in <span style="color: #4b4d51; font-weight: bold;">outputconfiguration_interface.cpp:217</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">X11 uses 16 bit values even for 24 bit screens, because the actual values that are sent to the DAC can have higher precision than the screen pixmap. I doubt that Wayland has reduced the precision, so I don't think checking values here is right.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I forget to remove this comment. This is not an issue since we can "stretch" the image over the whole uint16_t value domain linearly such that image 1 is associated with the largest uint16_t value. By that we have well defined arguments in any case.</p>

<p style="padding: 0; margin: 8px;">The array size though is checked before that in the checkArg call by comparing it with the size set first by the compositor.</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/D12388">https://phabricator.kde.org/D12388</a></div></div><br /><div><strong>To: </strong>romangg, Frameworks<br /><strong>Cc: </strong>zzag, cfeck, michaelh, bruns<br /></div>