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

Refactor public MVKDevice content into MVKDeviceTrackingMixin functions. #2221

Merged

Conversation

billhollings
Copy link
Contributor

This is a non-functional code-maintenance change.

Previously, MVKDevice contained a significant amount of publicly exposed internal content. This patch adds functions to MVKDeviceTrackingMixin to better encapsulate, consolidate & streamline access to this content.

  • Make MVKDeviceTrackingMixin a friend of MVKDevice & MVKPhysicalDevice.
  • Hide public MVKDevice content behind MVKDeviceTrackingMixin functions.
  • Remove similar MVKDevice content pointers from MVKCommandEncoder.
  • MVKDevice remove getPhysicalDevice(), getPixelFormats() & getMTLDevice(), to focus access through MVKDeviceTrackingMixin.
  • Move performance tracking functions to MVKDeviceTrackingMixin to remove need to reference MVKDevice multiple times when marking performance values.
  • Subclass MVKQueueSubmission, MVKMetalCompiler, MVKShaderLibrary, and MVKShaderLibraryCache from MVKBaseDeviceObject to make use of these changes.

Note to reviewers:

  • This is a non-functional change and passes the same CTS tests as previously.
  • The primary design changes of interest are in the declarations of MVKDeviceTrackingMixin and MVKDevice in MVKDevice.h, and to MVKCommandEncoder in MVKCommandBuffer.h.
  • The remaining changes are simple find-and-replace syntax changes to delegate to MVKDeviceTrackingMixin instead of reaching into MVKDevice.

Copy link
Collaborator

@cdavis5e cdavis5e left a comment

Choose a reason for hiding this comment

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

Sorry about that. Took me a while to get through this massive refactoring.

MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm Show resolved Hide resolved
This is a non-functional code-maintenance change.

Previously, MVKDevice contained a significant amount of publicly exposed
internal content. This patch adds functions to MVKDeviceTrackingMixin to
better encapsulate, consolidate & streamline access to this content.

- Make MVKDeviceTrackingMixin a friend of MVKDevice & MVKPhysicalDevice.
- Hide public MVKDevice content behind MVKDeviceTrackingMixin functions.
- Remove similar MVKDevice content pointers from MVKCommandEncoder.
- MVKDevice remove getPhysicalDevice(), getPixelFormats() & getMTLDevice(),
  to focus access through MVKDeviceTrackingMixin.
- Move performance tracking functions to MVKDeviceTrackingMixin to remove
  need to reference MVKDevice multiple times when marking performance values.
- Subclass MVKQueueSubmission, MVKMetalCompiler, MVKShaderLibrary, and
  MVKShaderLibraryCache from MVKBaseDeviceObject to make use of these changes.
@billhollings billhollings force-pushed the refactor-device-public-content branch from 407c3df to e1baea9 Compare May 2, 2024 15:06
@billhollings
Copy link
Contributor Author

Sorry about that. Took me a while to get through this massive refactoring.

No apology needed. And thanks.

I wrote the Note to reviewers above so you could skip all the search & replace noise. But I appreciate your epic diligence and accuracy once again! 👍🏻

@billhollings billhollings merged commit e361c2a into KhronosGroup:main May 2, 2024
6 checks passed
@billhollings billhollings deleted the refactor-device-public-content branch May 2, 2024 19:14
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