-
-
Notifications
You must be signed in to change notification settings - Fork 22
fix: Console.log in drawindexed #1771
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes console.log functionality for the drawIndexed
method in render pipelines, ensuring that logging works correctly when drawing indexed geometry.
- Adds logging support to
drawIndexed
method by integrating log resource handling - Adds a new "Draw indexed" test case to verify console.log functionality with indexed drawing
- Refactors shader definitions and removes unused test case to improve test organization
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/typegpu/src/core/pipeline/renderPipeline.ts | Integrates log resources handling into the drawIndexed method |
apps/typegpu-docs/src/examples/tests/log-test/index.ts | Adds "Draw indexed" test case and refactors shader definitions |
packages/typegpu/tests/examples/individual/log-test.test.ts | Updates expected test count and adds corresponding shader code for the new test |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pkg.pr.new packages
benchmark commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
'Too much data': { | ||
onButtonClick: () => { | ||
try { | ||
prepareDispatch(root, () => { | ||
'kernel'; | ||
console.log(d.mat4x4f(), d.mat4x4f(), 1); | ||
}).dispatch(); | ||
} catch (err) { | ||
console.log(err); | ||
} | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for deleting this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not need to use the GPU, so no need to keep it in an example. The same test already appears in unit tests.
No description provided.