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

Combine fields sharing the same length-field into a single builder function #445

Open
gadunga opened this issue May 28, 2021 · 4 comments
Open

Comments

@gadunga
Copy link

gadunga commented May 28, 2021

So right now resolve_attachments takes in a slice of AttachmentReference, and then sets color_attachment_count to the len of the input slice. However according to the specification for VkSubpassDescription it states:

pResolveAttachments is NULL or a pointer to an array of colorAttachmentCount VkAttachmentReference structures defining the resolve attachments for this subpass and their layouts.

This seems to imply that the behavior of the builder function is incorrect in overwriting color_attachment_count. At the very least we should probably change it so that it doesn't do that. However I believe in the long run the correct behavior would be to verify that the len of the input slice matches color_attachment_count and produce a result of some kind.

@gadunga
Copy link
Author

gadunga commented May 28, 2021

To be clear I'm willing to address this. Just let me know what course of action I should take. Thanks!

@MarijnS95
Copy link
Collaborator

Arrays are an often returning and discussed subject, because of their issues with optionality (or rather, lack of such annotations in the bindings), ambiguity for zero-length slices (such pointers should not be dereferenced) and the issue here: length fields shared between multiple pointers. For these cases I've been thinking about generating single functions that accept all involved slices at once, and asserts they're either identical in size or "null" (modeled as None or &[], haven't decided yet). The current separation is a bit too error-prone IMO.

To be clear I'm willing to address this. Just let me know what course of action I should take. Thanks!

Concrete answer: let's use this issue to discuss the appropriate way forward. I think single function for all slices could work, but others may have different solutions or find the drawbacks too severe.

@Ralith
Copy link
Collaborator

Ralith commented May 30, 2021

I've been thinking about generating single functions that accept all involved slices at once, and asserts they're either identical in size or "null"

I like this idea a lot, personally!

@MarijnS95
Copy link
Collaborator

@Ralith @MaikKlein Shall we rename this issue to something like "Combine fields sharing the same length-field into a single builder function" (or something more generic yet irrespective of SubpassDescriptionBuilder) and use it as a tracking issue to investigate such approaches?

@MarijnS95 MarijnS95 changed the title resolve_attachments in SubpassDescriptionBuilder overwrites color_attachment_count Combine fields sharing the same length-field into a single builder function Jun 17, 2021
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

No branches or pull requests

3 participants