On 11/30/2012 02:29 PM, Kenneth Graunke wrote:
On 11/29/2012 03:44 PM, Carl Worth wrote:
Previously, we were only supporting line-continuation backslash
characters
within lines of pre-processor directives, (as per the specification).
With
OpenGL 4.3, line continuations are now supported anywhere within a
shader.
OpenGL 4.2, actually.
And GLES3. :)
I'm also not sure if we decided we can get away with changing this
behavior. I'd really like to, as the new rules are much more sensible,
consistent, and efficient...
I think this should, at the very least, be the default behavior. We
know this will break Savage2. The alternate behavior (to enable via
driconf) should be either ignore line continuation altogether or ignore
line continuation in comments. The former will be easier, but I'm not
sure if it will cause other problems in that game.
While changing this, also fix a bug where the preprocessor was
ignoring line
continuation characters when a line ended in multiple backslash
characters.
The new code is also more efficient than the old. Previously, we would
perform
a ralloc copy at each newline. We now perform copies only at each
occurrence
of a line-continuation.
---
src/glsl/glcpp/pp.c | 108
++++++++++++++++++++-------------------------------
1 file changed, 42 insertions(+), 66 deletions(-)
diff --git a/src/glsl/glcpp/pp.c b/src/glsl/glcpp/pp.c
index 11b2941..1135570 100644
--- a/src/glsl/glcpp/pp.c
+++ b/src/glsl/glcpp/pp.c
@@ -70,82 +70,58 @@ glcpp_warning (YYLTYPE *locp, glcpp_parser_t
*parser, const char *fmt, ...)
&parser->info_log_length, "\n");
}
-/* Searches backwards for '^ *#' from a given starting point. */
-static int
-in_directive(const char *shader, const char *ptr)
-{
- assert(ptr >= shader);
-
- /* Search backwards for '#'. If we find a \n first, it doesn't
count */
- for (; ptr >= shader && *ptr != '#'; ptr--) {
- if (*ptr == '\n')
- return 0;
- }
- if (ptr >= shader) {
- /* Found '#'...look for spaces preceded by a newline */
- for (ptr--; ptr >= shader && isblank(*ptr); ptr--);
- // FIXME: I don't think the '\n' case can happen
- if (ptr < shader || *ptr == '\n')
- return 1;
- }
- return 0;
-}
-
-/* Remove any line continuation characters in preprocessing directives.
- * However, ignore any in GLSL code, as "There is no line continuation
- * character" (1.30 page 9) in GLSL.
+/* Remove any line continuation characters in the shader, (whether in
+ * preprocessing directives or in GLSL code).
*/
static char *
remove_line_continuations(glcpp_parser_t *ctx, const char *shader)
{
- int in_continued_line = 0;
- int extra_newlines = 0;
char *clean = ralloc_strdup(ctx, "");
- const char *search_start = shader;
- const char *newline;
- while ((newline = strchr(search_start, '\n')) != NULL) {
- const char *backslash = NULL;
-
- /* # of characters preceding the newline. */
- int n = newline - shader;
-
- /* Find the preceding '\', if it exists */
- if (n >= 1 && newline[-1] == '\\')
- backslash = newline - 1;
- else if (n >= 2 && newline[-1] == '\r' && newline[-2] == '\\')
- backslash = newline - 2;
-
- /* Double backslashes don't count (the backslash is escaped) */
- if (backslash != NULL && backslash[-1] == '\\') {
- backslash = NULL;
- }
-
- if (backslash != NULL) {
- /* We found a line continuation, but do we care? */
- if (!in_continued_line) {
- if (in_directive(shader, backslash)) {
- in_continued_line = 1;
- extra_newlines = 0;
- }
- }
- if (in_continued_line) {
- /* Copy everything before the \ */
- ralloc_strncat(&clean, shader, backslash - shader);
+ const char *backslash, *newline;
+ int collapsed_newlines = 0;
+
+ while (1) {
while (true) ?
+ backslash = strchr(shader, '\\');
+
+ /* If we have previously collapsed any
+ * line-continuations, then we want to insert
+ * additional newlines at the next occurrence of a
+ * newline character to avoid changing any line
+ * numbers. */
Please move the */ to its own line. I'd also wrap the lines later
(78-ish?), but that's just me...
+ if (collapsed_newlines) {
+ newline = strchr(shader, '\n');
+ if (newline &&
+ (backslash == NULL || newline < backslash))
+ {
+ ralloc_strncat(&clean, shader,
+ newline - shader + 1);
+ while (collapsed_newlines--)
+ ralloc_strcat(&clean, "\n");
shader = newline + 1;
- extra_newlines++;
}
- } else if (in_continued_line) {
- /* Copy everything up to and including the \n */
- ralloc_strncat(&clean, shader, newline - shader + 1);
- shader = newline + 1;
- /* Output extra newlines to make line numbers match */
- for (; extra_newlines > 0; extra_newlines--)
- ralloc_strcat(&clean, "\n");
- in_continued_line = 0;
}
- search_start = newline + 1;
+
+ if (backslash == NULL)
+ break;
+
+ /* At each line continuation, (backslash followed by a
+ * newline), copy all preceding text to the output,
+ * then advance the shader pointer to the character
+ * after the newline. */
Please un-cuddle the comments here.
+ if (backslash[1] == '\n' ||
+ (backslash[1] == '\r' && backslash[2] == '\n'))
+ {
+ collapsed_newlines++;
+ ralloc_strncat(&clean, shader, backslash - shader);
+ if (backslash[1] == '\n')
+ shader = backslash + 2;
+ else
+ shader = backslash + 3;
+ }
}
+
ralloc_strcat(&clean, shader);
+
return clean;
}
Other than than, looks good to me...thanks Carl!
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
Someday we may want to use ralloc's rewrite_tail functions to avoid the
extra strlen() overhead. But that can be done separately.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
