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

List:       kwin
Subject:    Re: Review Request 115369: allow users to disable GL/X extensions & disable GLX_ARB_create_context o
From:       Fredrik_Höglund <fredrik () kde ! org>
Date:       2014-01-30 15:58:47
Message-ID: 20140130155847.2191.73917 () probe ! kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


> On Jan. 29, 2014, 12:43 a.m., Fredrik Höglund wrote:
> > I honestly don't see how this is going to help you with either problem.
> > 
> > Bug 327310 is a threading bug, and it happens because kwin is directly
> > or indirectly making Xlib calls from multiple threads at the same time.
> > That this happens to bite you in glXCreateContextAttribs() is just a
> > coincidence.  It's a case of "Doctor, it hurts when I do this".
> > 
> > Bug 328518 almost certainly happens because copying data from the old
> > index buffer to the new in IndexBuffer::accommodate() fails.  And this
> > has nothing to do with GLX_MESA_copy_sub_buffer.  glCopyBufferSubData()
> > is provided by GL_ARB_copy_buffer.  The solution is to either map both
> > buffers and copy the data on the CPU, or just discard the old buffer
> > and recreate the data in the new buffer.
> > 
> > Also with Mesa: MESA_EXTENSION_OVERRIDE="+/-<extension_name>"
> > 
> 
> Thomas Lübking wrote:
> I hope it to help "me" preventing users from running into disfunctional GL \
> compositing. 
> See bug #327310 comment #4 (XInitThreads solution failure) as well as bug #326352 \
> comment #14 and bug #329908 comment #9 (fbconfig error after trying a core profile \
> once, success for GL2 only after reboot - while bug #326352 comment #29 adds more \
> confusion) 
> Something's really weird here, so the answer you'll usually get from your doctor \
> is: "Then don't do that" - it's not satisfying, but better than kwin GL compositing \
> "broken" on a growing amount of systems (as xserver >= 1.15 being distributed) It \
> also happens too often and repeatable to be just coincidental. 
> -
> GLX_MESA_copy_sub_buffer is indeed a red herring - i saw the "weird" mesa extension \
> in the catalyst driver and glCopyBufferSubData as a requirement to use indexed \
> quads. 
> -
> overriding the mesa extension failed (see bug #329478 comment #21)

That calling XInitThreads() didn't fix it doesn't mean that it's not a threading bug.
The "xcb_xlib_threads_sequence_lost" assert in the backtrace tells you that it is.

"This" in the sentence "Doctor it hurts when I do this" is making X calls from \
multiple threads when you know that Xlibs is not thread-safe.

As for the fbconfig error, it's an unrelated issue that happens to be easy to fix.


- Fredrik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115369/#review48506
-----------------------------------------------------------


On Jan. 28, 2014, 10:19 p.m., Thomas Lübking wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115369/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2014, 10:19 p.m.)
> 
> 
> Review request for kwin and Martin Gräßlin.
> 
> 
> Bugs: 327310 and 328518
> http://bugs.kde.org/show_bug.cgi?id=327310
> http://bugs.kde.org/show_bug.cgi?id=328518
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> 2 commits, the other bug is about catalyst and (probably) GLX_MESA_copy_sub_buffer
> In general it will allow us to have users disable extensions for testing and allow \
> us to remove the specific solution for the buffer_age extension. 
> In the light of the many reports on MESA GLX and Xorg 1.15 or just intel \
> pre-sandybridge I also suggest to just disable that by default on those platforms \
> and for the moment (w/o being happy with this "solution") 
> 
> Diffs
> -----
> 
> kwin/libkwineffects/kwinglutils.cpp a6cd8c3 
> kwin/libkwineffects/kwinglutils_funcs.cpp cb5391e 
> 
> Diff: https://git.reviewboard.kde.org/r/115369/diff/
> 
> 
> Testing
> -------
> 
> ensured that i can control GLX_ARB_create_context support in either direction
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
> 


[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/115369/">https://git.reviewboard.kde.org/r/115369/</a>
  </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <p style="margin-top: 0;">On January 29th, 2014, 12:43 a.m. UTC, <b>Fredrik \
Höglund</b> wrote:</p>  <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">  <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">I honestly don&#39;t see how this is going to help you with either \
problem.

Bug 327310 is a threading bug, and it happens because kwin is directly
or indirectly making Xlib calls from multiple threads at the same time.
That this happens to bite you in glXCreateContextAttribs() is just a
coincidence.  It&#39;s a case of &quot;Doctor, it hurts when I do this&quot;.

Bug 328518 almost certainly happens because copying data from the old
index buffer to the new in IndexBuffer::accommodate() fails.  And this
has nothing to do with GLX_MESA_copy_sub_buffer.  glCopyBufferSubData()
is provided by GL_ARB_copy_buffer.  The solution is to either map both
buffers and copy the data on the CPU, or just discard the old buffer
and recreate the data in the new buffer.

Also with Mesa: MESA_EXTENSION_OVERRIDE=&quot;+/-&lt;extension_name&gt;&quot;
</pre>
 </blockquote>




 <p>On January 29th, 2014, 1:26 a.m. UTC, <b>Thomas Lübking</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I hope it to help \
&quot;me&quot; preventing users from running into disfunctional GL compositing.

See bug #327310 comment #4 (XInitThreads solution failure) as well as bug #326352 \
comment #14 and bug #329908 comment #9 (fbconfig error after trying a core profile \
once, success for GL2 only after reboot - while bug #326352 comment #29 adds more \
confusion)

Something&#39;s really weird here, so the answer you&#39;ll usually get from your \
doctor is: &quot;Then don&#39;t do that&quot; - it&#39;s not satisfying, but better \
than kwin GL compositing &quot;broken&quot; on a growing amount of systems (as \
xserver &gt;= 1.15 being distributed) It also happens too often and repeatable to be \
just coincidental.

-
GLX_MESA_copy_sub_buffer is indeed a red herring - i saw the &quot;weird&quot; mesa \
extension in the catalyst driver and glCopyBufferSubData as a requirement to use \
indexed quads.

-
overriding the mesa extension failed (see bug #329478 comment #21)</pre>
 </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">That calling \
XInitThreads() didn&#39;t fix it doesn&#39;t mean that it&#39;s not a threading bug. \
The &quot;xcb_xlib_threads_sequence_lost&quot; assert in the backtrace tells you that \
it is.

&quot;This&quot; in the sentence &quot;Doctor it hurts when I do this&quot; is making \
X calls from multiple threads when you know that Xlibs is not thread-safe.

As for the fbconfig error, it&#39;s an unrelated issue that happens to be easy to \
fix. </pre>
<br />










<p>- Fredrik</p>


<br />
<p>On January 28th, 2014, 10:19 p.m. UTC, Thomas Lübking wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for kwin and Martin Gräßlin.</div>
<div>By Thomas Lübking.</div>


<p style="color: grey;"><i>Updated Jan. 28, 2014, 10:19 p.m.</i></p>







<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=327310">327310</a>, 

 <a href="http://bugs.kde.org/show_bug.cgi?id=328518">328518</a>


</div>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">2 commits, the other bug is about catalyst and (probably) \
GLX_MESA_copy_sub_buffer In general it will allow us to have users disable extensions \
for testing and allow us to remove the specific solution for the buffer_age \
extension.

In the light of the many reports on MESA GLX and Xorg 1.15 or just intel \
pre-sandybridge I also suggest to just disable that by default on those platforms and \
for the moment (w/o being happy with this &quot;solution&quot;)</pre>  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">ensured that i can control GLX_ARB_create_context support in either \
direction</pre>  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kwin/libkwineffects/kwinglutils.cpp <span style="color: \
grey">(a6cd8c3)</span></li>

 <li>kwin/libkwineffects/kwinglutils_funcs.cpp <span style="color: \
grey">(cb5391e)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/115369/diff/" style="margin-left: \
3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>



_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin


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

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