Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

shaders: report line numbers error accurately - #297

Open
ubitux wants to merge 1 commit into
NopeForge:mainfrom
ubitux:line-nums
Open

shaders: report line numbers error accurately#297
ubitux wants to merge 1 commit into
NopeForge:mainfrom
ubitux:line-nums

Conversation

@ubitux

@ubitux ubitux commented Jun 21, 2025

Copy link
Copy Markdown
Contributor

No description provided.

params->label ? params->label : "", s_with_numbers);
ngli_free(s_with_numbers);
}
LOG(ERROR, "failed to compile shader \"%s\"", params->label ? params->label : "");

@mbouron mbouron Jun 22, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested a bit the PR, and for now, I think it makes debugging harder. While it works for simple cases, it does not scale well, especially since we are injecting the ressources / inputs / outputs / helpers, ie: there's no other way to visualize the whole assembled shader, which is of great help for debugging. This is even more true for builtin shaders assembled from different glsl helpers and/or when nopegl is distributed across multiple devices with different kind of gpus (for example, one shader can fail on a specific hw / driver combinaison and works nicely on the other devices). Just having the error reported without the full context would make debugging a nightmare.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another issue I see, is that it would not work with builtins such as ngl_texvideo() is they use more than one line. This is currently the case so not an issue. But I had the idea at some point to use multiple line to make the final shader more readable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a debug option we could add a flag to keep dumping the whole thing for the project developers.

For users though the current experience as-is is much better experience. They don't have access to includes so and the line reset appears right before their input, which means the lines are aligned with their input, always (unless they have their own include system, in which case they can also use line directives in their own preprocessor).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another issue I see, is that it would not work with builtins such as ngl_texvideo() is they use more than one line. This is currently the case so not an issue. But I had the idea at some point to use multiple line to make the final shader more readable.

We could still trick it with something like that:

#line 34
tex = texture(
   foo,
   bar
);
#line 35

And if we use a function declared before the user input it's a non-issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants