I do wonder if the check in change_mode to stop the call if the VDP thinks it’s already in the requested mode is actually necessary? Could there be value in removing the check completely and just forcing a mode change, even if it’s to the same mode?

Treating the mode change (even to the same mode) as being an opportunity to refresh/reset the display state would also solve the following issues:

If legacy mode support (added in PR #68) is enabled, it does not currently recognise a mode change if the requested mode is the same number as the currently selected mode. Forcing the mode change would resolve this.

The mode change could could also disable paged scrolling (as per issue #72).

So I can see some advantages in dropping the check, but are there any disadvantages?

My initial gut feel on reading your comment @julianregel was that I kinda like the skipping of the resolution change, as it does feel unnecessary to rip-down and bring back up the underlying fab-gl stuff. This has however made me think further on this issue tho.

As you point out, swapping to legacy modes is indeed not currently compatible with the code. That would indeed be fixed by removing the "is this the same mode" check and always refreshing the screen mode.

As it stands, #72 has actually already been fixed in the most recent commit on the main branch.

What the current code saves by skipping the change_resolution call for the same mode is not really all that much IMHO - it avoids some calls to the VGAController that shouldn't do anything, primarily its the ripping down and re-creating of the Canvas object. Pretty much the rest of the state gets reset outside of that check anyway. We may even be able to remove the cls call.

In conclusion - yep - let's remove the check. 😁 Having it there isn't really gaining us much of anything.

@julianregel done - this version should fix the legacy modes issue you mentioned too

Whilst this fixes the issues being discussed, there is still a bug, which is related to legacy modes.

For me, new mode 0 (legacy mode 3) fails with an insufficient memory error.

The following sequence breaks things for me:

VDU 23,0,193,1 : REM legacy modes
MODE 0
VDU 23,0,193,0 : REM new modes
MODE 0

If I do that then I get a report of error 2 on restoring mode, and I'm left with the machine in an unusable state.

I think that this may not be a new bug per-se... as it might be possible to reproduce that via a different sequence of modes and swapping to/from legacy.

I wonder if the simplest solution to that bug may not just be to remove support for the "legacy" screen modes?

The upside of the legacy mode support is that entering VDU 23,0,193,1 before trying to run code that used a legacy mode will let it work without needing to change that code. However one could argue that there is not that much legacy code out there and it's a fairly easy fix to change which screen mode legacy code is trying to use.

Just worked out that removing the CLS leaves sprites behind on mode change...

I have therefore restored the CLS call, and made a further change_mode fallback call in the event of a second mode change error to swap to mode 1. As mode 1 seems to be safe, whether running in legacy or new modes, that should give a proper fix for the "unusable state" error I mentioned above. This fixes all issues in my testing.

I wanted to ensure that the new modes were logically ordered, but there were comments in the Facebook group that some were unhappy with the idea of losing the existing modes (and numbering) as a result of the new modes and that it would break code. Hence legacy modes.

I'm not sure how much software would be impacted in reality, but it was a "quick win" to add the support and I did get a message from someone expressing thanks for that feature.

I guess for most use cases, switching to legacy modes would be a one-way operation in order to run a specific piece of existing software. While it’s nice - and theoretically straightforward - to switch back to new modes, the alternative could be to reset the Agon after going into legacy modes.

So one option might be to support switching into legacy modes, but either not support switching back, or leaving it as an undocumented feature (as it may be unreliable)? Not necessarily advocating that approach, but throwing it out there.

I wonder if this bug has the same underlying issue with available memory that you’ve already called out and that getting to the bottom of that will resolve this issue as well?

the lack of available memory is definitely a major underlying issue at play here. According to the Arduino build tools, global variables account for about 42kb of memory usage - at run-time tho I'm sure things are somewhat different.

The Arduino tools are reporting for the code I'm running that my globals usage is at about 43kb....

My logging code tells me that with my codebase I have about 130kb free internal RAM in mode 1. after attempting to change into mode 0 it reports around 74kb free, but that's a duff mode and the fallback swaps to previous successful mode, going back to 130kb free. my logging is a simple thing:

	debug_log("free mem: %d\n\r", heap_caps_get_free_size(MALLOC_CAP_INTERNAL));

My feeling is that there is some scope for memory saving, but it will likely need some fairly significant efforts to re-engineer how chunks of the VDP codebase work. Some better encapsulation of functionality may be able to reduce memory usage, and let it be slightly easier to move some data into SPI-RAM.

As things are left with this PR, the issues I'd noticed around switching to/from legacy modes are essentially fixed, as it will revert back to mode 1. My idea of removing support for them can be ignored. 😁

I did some further analysis on this issue, and made some notes against #75 - #75 (comment)

As noted in that comment, there are significant issues with the memory usage related to how screen modes are handled. Even before the new screen modes were added, some screen modes would effectively only be accessible once. Legacy mode 3 was a good example of that - on a fresh boot the mode could be used, but change to mode 2 first and you'd be out of luck.

That comment links to a PR on the Console8 VDP codebase which provides a solution to the underlying memory issues, and indeed all of the issues around screen modes. It achieves this by taking a somewhat different approach to memory management of the VGA controllers. In my testing, this solution gives significantly more available free internal memory on the VDP without needing to move any memory for screen handling into PSRAM.