Add options to split framebuffers/z-buffer and better alloc assert option

[Aegiker]

No description provided.

[Yanis002]

sorry for the late review, looks good to me but I'll wait few days just in case someone with more knowledge than me wants to comment this

works on my machine™

[Yanis002]

merging this tomorrow unless there's last-minute comments

[Thar0]

I think the address assignments for at least the z-buffer should be moved to the spec. That way you don't need to use a macro like GET_ZBUFFER and can continue to use gZBuffer everywhere, which is a much less intrusive change.

[Thar0]

I don't think we should be returning a pointer to the z-buffer in a function that returns framebuffer pointers

I think this should be moved to the spec and do all the necessary address assignments there:

beginseg
    name "buffers"
    flags NOLOAD
    include "$(BUILD_DIR)/src/buffers/gfxbuffers.o"
    include "$(BUILD_DIR)/src/buffers/audio_heap.o"
#if !ENABLE_SPLIT_ZBUFFER
    include "$(BUILD_DIR)/src/buffers/zbuffer.o"
#endif
endseg

#if ENABLE_SPLIT_ZBUFFER
beginseg
    name "zbuffer"
    flags NOLOAD
#if ENABLE_SPLIT_FRAMEBUFFERS
    address 0x80500000 /* Leave room for the split framebuffers in 2 banks */
#else
    address 0x80600000 /* Both framebuffers in 1 bank */
#endif
    include "$(BUILD_DIR)/src/buffers/zbuffer.o"
endseg
#endif

[Thar0]

This is going to break without warning if anyone tries to use it with only 4MB

[ariahiro64]

looking at this we actually kinda worked for 2 months getting 4mb mode working and im unsure if it still works

[Aegiker]

ok, made the changes. if someone who can build this more quickly wouldn't mind stress testing it I would appreciate that

[Thar0]

I get the feeling we're still missing lots of cases here, e.g. it looks like 4MB mode is still going to break without warning. I thought about all the possible cases that would be useful:

In 4MB mode we don't have enough memory to do particularly meaningful splits, we can't split the z-buffer and both framebuffers with only 4MB since it would complicate the memory map too much, and we can't reserve entire banks for anything.

4MB AND !zbuffer AND !framebuffers
        vanilla layout:
                zbuffer follows code segments
                framebuffers at end of RAM

4MB AND !zbuffer AND framebuffers
        MM layout:
                zbuffer follows code segments
                fb0 at the start of RAM
                fb1 at the end of RAM

4MB AND zbuffer AND !framebuffers
        modified MM layout:
                zbuffer at the start of RAM
                framebuffers at end of RAM

4MB AND zbuffer AND framebuffers
        error, can't split both with only 4MB

In 8MB mode we have a lot more flexibility. We can choose to burn lots of memory to achieve the maximum possible RDP buffer performance, or we can be a little more conservative of the memory. This should be up to the user to configure based on their needs: the option I called "ramsave" below will reclaim memory at the cost of performance.

8MB AND !zbuffer AND !framebuffers
        vanilla layout, except end of RAM has moved

8MB AND !zbuffer AND framebuffers AND ramsave
        MM layout:
                zbuffer follows code segments
                fb0 at the start of RAM
                fb1 at the end of RAM

8MB AND !zbuffer AND framebuffers AND !ramsave
        vanilla layout, except last two framebuffers occupy all of the last two MB of RAM for performance

8MB AND zbuffer AND !framebuffers
        vanilla layout, except last MB is reserved for zbuffer for performance

8MB AND zbuffer AND framebuffers AND ramsave
        MM layout (fb0 at start of RAM, fb1 at end of 7MB), and last MB is fully reserved for the zbuffer

8MB AND zbuffer AND framebuffers AND !ramsave
        last 3 MB are reserved for framebuffers and zbuffer for performance

idk if we want to be this flexible, but if the goal is to be a useful base for lots of different uses it seems advantageous.

[Aegiker]

Interesting, though I think it's already flexible enough as is. You can already split the Z-buffer without splitting the framebuffer, which I think is generous. I disagree that 4MB would crash without warning, because I wrote a warning above the option and tried to make it clear what would happen if you did. If you're using 4MB you probably have bigger problems anyway, and it's not an argument to have here but I don't think extended support for 4MB is a good idea in general. If you wanted to implement this it certainly wouldn't hurt, but even though it's possible I don't think the proposal is terribly useful, and I wouldn't have any interest in implementing this. I don't think that should prevent the sufficient functionality here from being merged though, because it works as intended.

[Thar0]

I disagree that 4MB would crash without warning, because I wrote a warning [in a comment]

imo it's not enough to just say so in a comment. We can do better and make the build fail with an error message.

I don't think extended support for 4MB is a good idea in general

Personally I have no interest in 4MB, I'd never use it. But if the goal is to reach lots of users it's not really ok to just ignore 4MB, unless we specifically define what our boundaries are with respect to supporting it. If we disagree on what these boundaries are then that probably needs to be worked out to decide how to proceed

[Aegiker]

I don't know if there is an established method for making the build fail with an error message, that'd be fine. 4MB support is... fine, but I think that extended support for things like this is an unnecessary hindrance. Options are cool, and you know I try to defend the underdog when the discussion is about console compatibility. Still, too many options creates bloat, and not everything that could be one should be one. 4MB users have the option to not use this, so it does not take away the choice to use 4MB. 8MB exists for a reason, and should be allowed to be used to its fullest potential. 4MB is not the best option with regards to performance and I don't think I should have to go out of my way to support extra diminished versions of this just because they could exist, especially if it denies users access to what is already available because it got stuck in tape. This applies to everything. If full support for both is the standard, the debug heap should be fixed too instead of overriding SYS_CFB_END, but it's not realistic to expect everything to work and I don't think that anyone does.

[ariahiro64]

I don't know if there is an established method for making the build fail with an error message, that'd be fine. 4MB support is... fine, but I think that extended support for things like this is an unnecessary hindrance. Options are cool, and you know I try to defend the underdog when the discussion is about console compatibility. Still, too many options creates bloat, and not everything that could be one should be one. 4MB users have the option to not use this, so it does not take away the choice to use 4MB. 8MB exists for a reason, and should be allowed to be used to its fullest potential. 4MB is not the best option with regards to performance and I don't think I should have to go out of my way to support extra diminished versions of this just because they could exist, especially if it denies users access to what is already available because it got stuck in tape. This applies to everything. If full support for both is the standard, the debug heap should be fixed too instead of overriding SYS_CFB_END, but it's not realistic to expect everything to work and I don't think that anyone does.

essentially the main requirement camera debug was completely disabled. the rest wasnt a hard requirement. i think that this define should just denable 4mb mode entirely. as what your trying to do kills it anyway. thats the simplest solution and the easiest.

[Yanis002]

let me know when this is ready to merge