Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upReadable shader error messages #2622
Merged
+22
−6
Conversation
|
Nice! @bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Apr 5, 2018
Readable shader error messages Working with shaders, I find `#line` navigation experience to be rather poor: 1. first, I take an `apitrace` capture 2. load the capture, go to the last `glShaderSource` 3. copy out the input into a new file 4. for each `#line` directive, try to match the reported error number ... This PR removes line overrides. Instead, it prints the relevant piece to the console, e.g.: ``` Loading shaders... Failed to compile shader: cs_clip_image 0:1152(17): error: syntax error, unexpected NEW_IDENTIFIER, expecting ',' or ';' | ClipScrollNode scroll_node = fetch_clip_scroll_node(cmi.scroll_node_id); > DImageMaskData mask = fetch_mask_data(cmi.clip_data_address); | RectWithSize local_rect = mask.local_rect; ``` This information instantly gives me an idea of what's wrong (note: unlike the original driver message), and makes all the listed steps unnecessary for 99% of cases. And when the steps are necessary, we don't have to check for each `#line` occurrence since the driver-reported line number is no longer ambiguous. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2622) <!-- Reviewable:end -->
|
|
bors-servo
added a commit
that referenced
this pull request
Apr 6, 2018
Separate primitive shaders from clip shaders Includes #2622 Note: this is not done because I'm bored. The problem Szeged team has bumped into is that `aDataX` attributes end up being used in the shaders they don't relate to (e.g. clip shaders). This isn't affecting GLSL but it does affect GLSL -> SPIRV -> XXX shader pipelines. The solution I came up with involves moving parts of `prim_shared` into separate shared modules, and turning `clip_shared` into a catch-all header of clip shades. Downsides: more shader modules (but not programs) to navigate between. Any suggestions on alternative solutions are welcome ;) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2623) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
kvark commentedApr 5, 2018
•
edited by larsbergstrom
Working with shaders, I find
#linenavigation experience to be rather poor:apitracecaptureglShaderSource#linedirective, try to match the reported error number ...This PR removes line overrides. Instead, it prints the relevant piece to the console, e.g.:
This information instantly gives me an idea of what's wrong (note: unlike the original driver message), and makes all the listed steps unnecessary for 99% of cases. And when the steps are necessary, we don't have to check for each
#lineoccurrence since the driver-reported line number is no longer ambiguous.This change is