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

Merge instances of set viewport/scissor at the API level #230

Merged
merged 1 commit into from
Dec 23, 2024
Merged

Conversation

kvark
Copy link
Owner

@kvark kvark commented Dec 23, 2024

Following up to #227

Reducing code and API duplication

Merge the API invocations into RenderEncoder.
Move the depth range inside the viewport.
@kvark kvark enabled auto-merge (rebase) December 23, 2024 06:06
@kvark kvark merged commit e30587a into main Dec 23, 2024
6 checks passed
@kvark kvark deleted the viewport branch December 23, 2024 06:11
@@ -295,11 +296,15 @@ pub struct ComputeCommandEncoder<'a> {
device: &'a Device,
update_data: &'a mut Vec<u8>,
}
//Note: we aren't merging this with `ComputeCommandEncoder`
// because the destructors are different, and they can't be specialized
// https://github.com/rust-lang/rust/issues/46893
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know about this one

Copy link
Owner Author

Choose a reason for hiding this comment

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

Quite annoying, honestly! It breaks the API symmetry between standalone types and generic specializations.
I've spent some time experimenting with it and had to throw a lot of stuff out, due to this seemingly minor issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Very surprising it isn't supported :/

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.

2 participants