Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(webgl): shader debug #2066

Merged
merged 1 commit into from
Apr 5, 2024
Merged

fix(webgl): shader debug #2066

merged 1 commit into from
Apr 5, 2024

Conversation

Pessimistress
Copy link
Collaborator

Change List

  • Correct typo

Overall I am not sure what this code path is supposed to do. According to

if (log.level === 0) {
this.compilationStatus = 'pending';
return;
}

If log.level = 0, then compilationStatus will never be 'error'
If log.level = 1, then the shader instance would have called debugShader() already.

@ibgreen
Copy link
Collaborator

ibgreen commented Apr 4, 2024

Overall I am not sure what this code path is supposed to do

Yes, I was experimenting with different approaches to controlling errors and what is in there is probably not the final word

  • Checking shader compilation status is a driver roundtrip and significantly slows down the app.
  • So the recommendation is to not check shader status unless program linking fails.
  • Of course when debugging it is nice if the error is thrown from the shader code, arguably a bit easier to pinpoint the failure.
  • I also implemented asynchronous shader compilation (unfortunately currently turned off by deck.gl as it was causing a small regression) and that also affects error checking flow.

@Pessimistress
Copy link
Collaborator Author

If it's intentional not to check shader status, should we remove the status check and just call debugShader after linking fails?

@ibgreen
Copy link
Collaborator

ibgreen commented Apr 5, 2024

If it's intentional not to check shader status, should we remove the status check and just call debugShader after linking fails?

Yes sure, that could be an option. As already mentioned, the reason it is still there is that , when debugging it is nice if the error is thrown synchronously from the shader code, this makes faster to debug the failure as you can break on exceptions and see in the stack trace exactly when the shader/model etc is created.

@ibgreen ibgreen merged commit def29f4 into master Apr 5, 2024
2 checks passed
@ibgreen ibgreen deleted the Pessimistress-patch-1 branch April 5, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants