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

Add const qualifier to SurfaceCapabilities lists #256

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

eliemichel
Copy link
Collaborator

The end user of GetCapabilities should not be allowed to mutate capabilities as far as I understand it.

The end user of GetCapabilities should not be allowed to mutate capabilities as far as I understand it.
@Kangz
Copy link
Collaborator

Kangz commented Jan 3, 2024

I'm not sure: the structure returned and its memory is owned by the user of the API so they can mutate it if they'd like.

@kainino0x kainino0x added !discuss Needs discussion (at meeting or online) presentation Presenting images to surfaces like windows and canvases labels Jan 3, 2024
@eliemichel
Copy link
Collaborator Author

True, though imho non-const pointers somehow suggest that the user could mutate the actual capabilities by mutating the fields of the WGPUSurfaceCapabilities struct.

And I can hardly imagine a use case where one would need to mutate these... but on the other hand why not if we do not need to forbid it.

@kainino0x
Copy link
Collaborator

kainino0x commented Jan 5, 2024

Jan 4 meeting:

  • KN: Wondering if these could ever actually be owned by some other object and not allocated just for the WGPUSurfaceCapabilities object?
  • CF: Could have refcounting underneath, then it could be shared owned.
  • CF: Is const_cast undefined behavior in C++?
    • KN: In this case I’m pretty certain it would be.
    • (So it would be OK to return a const pointer to internal data.)
  • KN: Think they should be const in case of the refcounting situation or something.
    • CF: Actually doing this requires some tricks so FreeMembers can get at the refcount from just the SurfaceCapabilities struct. But possible.
  • Let’s make them const

@kainino0x kainino0x added the has resolution Issue is resolved, just needs to be done label Jan 5, 2024
@kainino0x kainino0x merged commit 3ffeaec into webgpu-native:main Jan 5, 2024
1 check passed
@kainino0x kainino0x removed the !discuss Needs discussion (at meeting or online) label Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has resolution Issue is resolved, just needs to be done presentation Presenting images to surfaces like windows and canvases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants