On Monday, March 7, 2016 5:23:26 PM PST Ian Romanick wrote: > On 03/04/2016 07:33 PM, Kenneth Graunke wrote: > > We resolved the implicit version directive when processing control lines, > > such as #ifdef, to ensure any built-in macros exist. However, we failed > > to resolve it when handling ordinary text. > > > > For example, > > > > int x = __VERSION__; > > > > should resolve __VERSION__ to 110, but since we never resolved the implicit > > version, none of the built-in macros exist, so it was left as is. > > > > This also meant we allowed the following shader to slop through: > > > > 123 > > #version 120 > > > > Nothing would cause the implicit version to take effect, so when we saw > > the #version directive, we thought everything was peachy. > > > > This patch makes the lexer's per-token action resolve the implicit > > version on the first non-space/newline/hash token that isn't part of > > a #version directive, fulfilling the GLSL language spec: > > > > "The #version directive must occur in a shader before anything else, > > except for comments and white space." > > > > Because we emit #version as HASH_TOKEN then VERSION_TOKEN, we have to > > allow HASH_TOKEN to slop through as well, so we don't resolve the > > implicit version as soon as we see the # character. However, this is > > fine, because the parser's HASH_TOKEN NEWLINE rule does resolve the > > version, disallowing cases like: > > > > # > > #version 120 > > > > This patch also adds the above shaders as new glcpp tests. > > Does any of this interfere with various workarounds that we have to > allow mixed #version lines, #extension lines, and code? Assuming all > that still works, this series is > > Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
Good call. I verified that Unigine Heaven's shaders fail to compile normally, but work with allow_glsl_extension_directive_midshader=true, even after my patch series. The app itself also continues running fine. Thanks for the review!
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
