<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - Savage 2 Edges render white [r600g]" href="https://bugs.freedesktop.org/show_bug.cgi?id=63579#c14">Comment # 14</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - Savage 2 Edges render white [r600g]" href="https://bugs.freedesktop.org/show_bug.cgi?id=63579">bug 63579</a> from <a class="email" href="mailto:idr@freedesktop.org" title="Ian Romanick <idr@freedesktop.org>"> Ian Romanick</a> <pre>(In reply to <a href="show_bug.cgi?id=63579#c12">comment #12</a>) > Well, Khronos meetings don't define the spec, the specification does. And > the specification is pretty clear here. The discussion was at the time of the vote, and I stand by that. I can't make the meeting minutes public, and I'm not sure that's the most important thing here. However, I can explain the rationale behind our position for Mesa's compiler. Since you won't be satisfied by the short version of the story, prepare for a long, twisting tale... The problem is that, following the C model of compilation, line continuation is a separate compilation phase, and it occurs before preprocessing. As a result, it's a shocking amount of work to *correctly* dynamically switch behaviors depending on the contents of the shader. The implementation is also likely to have compilation overhead for every shader. Consider a pathological shader like: // \ /* #version 120 // */ \ #version 420 Shaders similar to this one were discussed by Khronos during OpenGL ES 3.0 and OpenGL 4.2 development. Suffice to say, more time than seems reasonable was spent trying to decide 1. what that shader should do, and 2. how to craft spec language to describe that. This is also why issue 12.19 in the GLSL ES 3.0 spec is resolved, "Line-continuation to be made optional in GLSL ES 1.00." Mesa's current GLSL compiler originally had line continuation, but, after having a problem with this game, we removed it. I can't find a specific commit, so this may have been removed before the preprocessor was merged. We encountered a small number of shaders that wanted continuation in preprocessor lines (#define and #line), and these shaders all compile on both NVIDIA and AMD. We added continuation support for those cases in 2010 (commit bc1097d). Both 3.00 ES and 4.20 require continuation, so we had to add it back for all cases. In doing so, of course, we noticed that the Savage2 shader stopped compiling. We thought about just supporting continuation in ES contexts, but the problem still exists since we expose GL_ARB_ES3_compatibility. We searched our internal database of a few thousand shaders from shipping applications (open- and closed-source). This was the only shader didn't want continuation behavior. There was a very small number of shaders that used continuation. If it weren't for the Savage2 shader, frankly, we wouldn't have implemented the non-continuation behavior at all... and nobody would have ever noticed. There were zero shaders that both wanted continuation and non-continuation behavior, and there were zero applications that contained both a continuation shader and a non-continuation shader. We just couldn't justify either the effort or the added code complexity to support dynamic selection. We also couldn't justify breaking a shipping application that previously worked. The right compromise was to have a workaround for the one case that required the non-continuation behavior, and this game works fine on the i965 driver. The Gallium drivers just need to use the workaround, and <a href="attachment.cgi?id=78052" name="attach_78052" title="[PATCH] gallium: handle drirc disable_glsl_line_continuations option">attachment #78052</a> <a href="attachment.cgi?id=78052&action=edit" title="[PATCH] gallium: handle drirc disable_glsl_line_continuations option">[details]</a> <a href='page.cgi?id=splinter.html&bug=63579&attachment=78052'>[review]</a> should do that. We probably could have done a better job telling the other driver authors that they needed to support this option when it was added. We were overwhelmingly busy trying to get ES3 support finished, so some things like that fell through the cracks. We put the patches on the mailing list (and the commit message in 4b00ece specifically says it fixes Savage2), so it's not like we were being sneaky. We can only do better in the future. The part you didn't mention from section 3.1 of the 1.20 spec is that \ is not listed as part of the character set. If we really want to argue the letter of the spec, the shader should fail to compile because it contains an invalid character. :)</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>