On 08/09/2013 04:38 PM, Ian Romanick wrote:
From: Ian Romanick <ian.d.roman...@intel.com>
Previously we would emit a warning for empty declarations like
float;
We would also emit the same warning for things like
highp float;
However, this second case is most likely the application trying to set the
default precision. We should instead generate an error.
Fixes piglit precision-05.vert.
Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
Cc: "9.2" <mesa-sta...@lists.freedesktop.org>
---
src/glsl/ast_to_hir.cpp | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 49804b7..9d2ffff 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2697,6 +2697,10 @@ ast_declarator_list::hir(exec_list *instructions,
* name of a known structure type. This is both invalid and weird.
* Emit an error.
*
+ * - The program text contained something like 'mediump float;'
+ * when the programmer probably meant 'precision mediump
+ * float;' Emit an error.
+ *
* Note that if decl_type is NULL and there is a structure involved,
* there must have been some sort of error with the structure. In this
* case we assume that an error was already generated on this line of
@@ -2705,7 +2709,10 @@ ast_declarator_list::hir(exec_list *instructions,
*/
assert(this->type->specifier->structure == NULL || decl_type != NULL
|| state->error);
- if (this->type->specifier->structure == NULL) {
+ if (this->type->qualifier.precision != ast_precision_none) {
+ _mesa_glsl_error(&loc, state,
+ "set default precision using `precision' keyword");
I like how this message suggests what to do. I don't like that it
doesn't describe the actual problem.
Perhaps something like:
"empty declaration; perhaps you meant `precision highp %s'?",
this->type->specifier->type_name
or:
"empty declaration; to set the default precision, use `precision highp
%s;'", this->type->specifier->type_name
+ } else if (this->type->specifier->structure == NULL) {
if (decl_type != NULL) {
_mesa_glsl_warning(&loc, state, "empty declaration");
} else {
@@ -2714,12 +2721,6 @@ ast_declarator_list::hir(exec_list *instructions,
type_name);
}
}
-
- if (this->type->qualifier.precision != ast_precision_none &&
- this->type->specifier->structure != NULL) {
- _mesa_glsl_error(&loc, state, "precision qualifiers can't be applied "
- "to structures");
- }
}
foreach_list_typed (ast_declaration, decl, link, &this->declarations) {
Although stupid, declarations like "highp float;" are actually permitted
by the letter of the spec. nVidia's driver also accepts them. So I'm a
bit uneasy about disallowing them.
We should check AMD. If it disallows them, I'm fine with disallowing
them as well. If it accepts them, I think we should too (sadly).
I definitely approve of generating a better message, though.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
