Comment # 14
on bug 63579
from Ian Romanick
(In reply to comment #12)
> 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 attachment #78052 [details] [review] 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. :)