<div dir="ltr">On 13 September 2013 11:25, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">We now set the ARB_shader_bit_encoding flag for versions that support<br>
this functionality, so we don't need to double check it here.<br>
<br>
Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
Cc: Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>><br>
---<br>
src/glsl/builtin_functions.cpp | 3 +--<br>
1 file changed, 1 insertion(+), 2 deletions(-)<br>
<br>
diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp<br>
index c468bd5..b020a7c 100644<br>
--- a/src/glsl/builtin_functions.cpp<br>
+++ b/src/glsl/builtin_functions.cpp<br>
@@ -182,8 +182,7 @@ shader_texture_lod_and_rect(const _mesa_glsl_parse_state *state)<br>
static bool<br>
shader_bit_encoding(const _mesa_glsl_parse_state *state)<br>
{<br>
- return state->is_version(330, 300) ||<br>
- state->ARB_shader_bit_encoding_enable ||<br>
+ return state->ARB_shader_bit_encoding_enable ||<br>
state->ARB_gpu_shader5_enable;<br>
}<br>
<span class=""><font color="#888888"><br>
--<br>
1.8.3.4<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div><div class="gmail_extra">I'm not a huge fan of this approach. We're currently inconsistent in Mesa as to how we handle GLSL extensions that were adopted into later versions of GLSL (or backports from later versions of GLSL). For these extensions:<br>
<br>ARB_draw_instanced<br>ARB_fragment_coord_conventions<br>ARB_gpu_shader5<br>ARB_shader_texture_lod<br>ARB_shading_language_420pack<br>ARB_shading_language_packing<br>ARB_texture_cube_map_array<br>ARB_texture_multisample<br>
OES_texture_3D<br><br></div><div class="gmail_extra">we use what I'll call the "enable means explicitly enabled" style, which means we only set the "_enable" flag if the shader contains text like "#extension ARB_foo: enable"; if the extension's functionality is provided by the given version of GLSL, we fold that into the if-test that enables the functionality--e.g.:<br>
<br> if (state->ARB_draw_instanced_enable || state->is_version(140, 300))<br> add_system_value(SYSTEM_VALUE_INSTANCE_ID, int_t, "gl_InstanceID");<br><br></div><div class="gmail_extra">But for these extensions:<br>
<br>ARB_explicit_attrib_location<br>ARB_texture_rectangle<br><br></div><div class="gmail_extra">we use what I'll call the "enable means available" style, which means that we set the "_enable" flag when processing the version directive (or in the _mesa_glsl_parse_state constructor), to indicate that the functionality is available, whether or not the user explicitly requested it or not.<br>
<br></div><div class="gmail_extra">(Note: for ARB_uniform_buffer_object it looks like we do some of each style!)<br></div><div class="gmail_extra"><br><br></div><div class="gmail_extra">Personally I'd prefer to see us consistently adopt the "enable means explicitly enabled" style (to me, the "enable means available" style feels like we're fibbing to ourselves). An additional advantage of the "enable means explicitly enabled" style is that it allows us to handle cases (such as in ARB_draw_instanced) where the extension differs slightly from the functionality that was eventually folded into GLSL. Another advantage is that if a client-supplied shader does something silly like "#extension ARB_draw_instanced: disable" in a GLSL 1.40+ shader, we won't accidentally disable built-in functionality (although in practice this is extremely unlikely to ever crop up).<br>
</div><div class="gmail_extra"><br></div><div class="gmail_extra">If it becomes too onerous to add the "|| state->is_version(...)" checks all over the place we can always make some inline functions, e.g.:<br>
<br></div><div class="gmail_extra">struct _mesa_glsl_parse_state {<br> ...<br></div><div class="gmail_extra"> bool is_draw_instanced_available() const<br> {<br></div><div class="gmail_extra"> return this->ARB_draw_instanced_enable || this->is_version(140, 300);<br>
}<br> ...<br>};<br></div></div>