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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃Ъ Clean up Metal implementation #227

Merged
merged 1 commit into from
Jun 21, 2024
Merged

馃Ъ Clean up Metal implementation #227

merged 1 commit into from
Jun 21, 2024

Conversation

FlannyH
Copy link
Contributor

@FlannyH FlannyH commented May 22, 2024

  • Make all the #[derive()]s consistent with the Dx12 implementation
  • Improve the formatting slightly by adding newlines
  • Add debug label for memory blocks
  • Remove unused structs

@FlannyH FlannyH marked this pull request as ready for review May 22, 2024 12:22
@FlannyH FlannyH requested a review from MarijnS95 June 5, 2024 11:46
Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

Other cleanups you could do (that's not visible in the diff):

  • Remove functions that always panic via todo!();
  • Change heap_types.iter() in Allocator::new() to into_iter(), so that you can remove heap_descriptor.clone();
  • Remove _size from MemoryBlock if it's never read and not public?

src/metal/mod.rs Show resolved Hide resolved
src/metal/mod.rs Outdated Show resolved Hide resolved
src/metal/mod.rs Outdated Show resolved Hide resolved
@MarijnS95 MarijnS95 enabled auto-merge (squash) June 13, 2024 11:23
@MarijnS95 MarijnS95 disabled auto-merge June 13, 2024 11:23
@MarijnS95 MarijnS95 enabled auto-merge (squash) June 13, 2024 11:24
commit b1407d5
Author: Marijn Suijten <[email protected]>
Date:   Wed Jun 12 14:34:30 2024 +0200

    metal: Use our own `MemoryLocation` for `MTLHeap` (`MemoryBlock`) labels

commit 91a839d
Author: Marijn Suijten <[email protected]>
Date:   Wed Jun 12 14:32:38 2024 +0200

    metal: Delete strange copy-paste functions that always panic via `todo!()`

    Since we're not implementing `trait`s here (every backend just "looks"
    the same but is different because of heavy API coupling), we shouldn't
    have a bunch of dead `todo!()` unimplemented functions publicly exposed.

commit 8182a39
Merge: c812c08 95ba6d0
Author: Marijn Suijten <[email protected]>
Date:   Wed Jun 12 12:47:19 2024 +0200

    Merge remote-tracking branch 'origin/main' into metal-clean

commit c812c08
Author: Marijn Suijten <[email protected]>
Date:   Wed Jun 12 10:15:56 2024 +0200

    Remove clone

commit 0a943ea
Author: Lily Haverlag <[email protected]>
Date:   Wed May 22 12:19:04 2024 +0200

    Add white lines

commit 465713f
Author: Lily Haverlag <[email protected]>
Date:   Wed May 22 12:18:36 2024 +0200

    Remove unused empty structs

commit d809208
Author: Lily Haverlag <[email protected]>
Date:   Wed May 22 12:18:26 2024 +0200

    Improve debug label for memory blocks

commit 33e8ae0
Author: Lily Haverlag <[email protected]>
Date:   Wed May 22 12:18:13 2024 +0200

    Add debug derives
@MarijnS95 MarijnS95 merged commit 49d1ace into main Jun 21, 2024
20 of 26 checks passed
@MarijnS95 MarijnS95 deleted the metal-clean branch June 21, 2024 09:00
@MarijnS95
Copy link
Member

Really curious why automerge pulled in the PR even before the CI run completed. If failed after all.

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.

None yet

3 participants