Skip to content

Conversation

@MonsterDruide1
Copy link
Contributor

@MonsterDruide1 MonsterDruide1 commented May 6, 2025

Required for implementing GpuMemAllocator in SMO. Apparently, the arguments of GPUMemBlockBase have been wrong here - the function is invoked after the sead::Heap*-specific operator new is called, but it does not take the heap itself, according to symbols in SMO. I could not find any reference to this in BotW.

This causes no mismatches or compiler errors in BotW and SMO.


This change is Reviewable

Copy link
Contributor

@Fuzzy2319 Fuzzy2319 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @MonsterDruide1)


include/common/aglGPUMemBlock.h line 55 at r3 (raw file):

class GPUMemBlockT : public GPUMemBlockBase {
public:
    ~GPUMemBlockT() { ; }

Shouldn't this be marked as override?

Copy link
Contributor Author

@MonsterDruide1 MonsterDruide1 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @Fuzzy2319 and @MonsterDruide1)


include/common/aglGPUMemBlock.h line 55 at r3 (raw file):

Previously, Fuzzy2319 wrote…

Shouldn't this be marked as override?

Ah, good catch. Done.

Copy link
Contributor

@Fuzzy2319 Fuzzy2319 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MonsterDruide1)

@MonsterDruide1 MonsterDruide1 merged commit 40a550b into open-ead:master May 18, 2025
2 checks passed
@MonsterDruide1 MonsterDruide1 deleted the gpu-mem-addr-block branch May 18, 2025 09:07
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