Is VsetScreen() broken?

[mikro]

It was my understanding that Falcon's VsetMode(mode) + VsetScreen(-1, screen, -1, -1) is basically equivalent to VsetScreen(-1, screen, 3, mode).

However it would seem that something super-fishy is going on when I enable the #define? New video base address it not only not set but when started from 640x480@1bpp it quite severely crashes? (reproducible in Hatari, too) I can even disable the for loop and it still crashes, so it's not like I write somewhere where I shouldn't (not sure how that would be possible but anyway)

Resolutions other than 640x480@1bpp and/or when using NVDI lead to various strange effects, even different on Hatari and real hardware. In Hatari I'd often see the video base address set but the exit resolution corrupted, on real hardware I'd see blank screen with nothing (and corrupted exit resolution). Works perfectly if I comment out the #define.

Code: Select all

#include <mint/falcon.h>
#include <stdint.h>
#include <stdio.h>

//#define ONLY_VSETSCREEN

int main()
{
        const uint16_t mode = BPS16 | COL40 | VGA | PAL | VERTFLAG;
        const size_t size = VgetSize(mode);

        uint16_t* scr = (uint16_t*)Mxalloc(size + 15, MX_STRAM);
        if (scr == NULL)
                return 1;

        uint16_t* scr_aligned = (uint16_t*)(((uintptr_t)scr + 15) & 0xfffffff0);

        uint16_t old_mode = VsetMode(VM_INQUIRE);
        void* old_scr = Physbase();

#ifdef ONLY_VSETSCREEN
        VsetScreen(SCR_NOCHANGE, scr_aligned, 3, mode);
#else
        VsetMode(mode);
        VsetScreen(SCR_NOCHANGE, scr_aligned, SCR_NOCHANGE, SCR_NOCHANGE);
#endif

        for (size_t i = 0; i < size / 2; i++) {
                *scr_aligned++ = 0x0fff;
        }

        getchar();

#ifdef ONLY_VSETSCREEN
        VsetScreen(SCR_NOCHANGE, old_scr, 3, old_mode);
#else
        VsetMode(old_mode);
        VsetScreen(SCR_NOCHANGE, old_scr, SCR_NOCHANGE, SCR_NOCHANGE);
#endif

        return 0;
}

Anyone any idea / insight what's wrong with that code?

[dml]

I have a vague memory that VsetScreen may try to clear the screen (VT52 init side effect?) on a mode change, so if you're not pointing at a big enough buffer it can corrupt. And exactly what happens depends on some random things like NVDI, screen expanders... etc.

When using these I generally tried to make sure the log/phys points at the larger buffer on a mode change, then update them if necessary. If the buffer gets written by anything it won't then overrun. I think the behaviour of VsetMode is not the same as VsetScreen also w.r.t. the VT52 stuff.

Not sure what happens if you change mode and don't set both log+phys together. That might also be an issue, especially if a clear event occurs.

This might be wrong - haven't been near this stuff in a long while.

[mikro]

Yes, I'm aware of the VT52 reinit but I did not expect it to be crashing. :)

I think you are right -- if one doesn't change *both* addresses, TOS is trying to clear both of them with the size of new video mode, i.e. 640x480@1bp logbase = 38400 bytes gets overwritten by 320x240@16bpp = 153600 bytes for physbase.

What a treat. OK, I guess I'll stick with the first version then.

[shoggoth]

Well yeah. Vsetscreen() is broken sort of.

Use VsetMode() to set the screen resolution, use VsetScreen(-1,physbase,-1,-1) to set physbase. Anything else will give you trouble, because VsetScreen() will reinitialise the VDI and load screen drivers (drawing privitives for chunky/planar). Atari designed it that way.

[shoggoth]

Some snippets from TOS, VsetMode:

Code: Select all

/* ... because VsetMode isn't supposed to muck with line A at all  */

And VsetScreen:

Code: Select all

if (rez >= 0)
    {
        mode2sdef(modecode, &sdef);
	dev_init(&sdef);
    }

'modes2def' generates a screen driver based on the modecode, and dev_init installs it. Apart from changing the VDI drawing primitives on the fly (thus severely confusing anything that use it), it also reinitialises the console @ logbase, which may not fit within the VDI screen at all given what resolution you're switching to. Anything running in the background will go fruitcake. It's been this way since 1992.

So - to have a multitasking-friendly, private, screen:

uint16_t modecode_old = VsetMode(modecode_new);
void* physbase_old = Physbase();
VsetScreen(-1,physbase_new,-1,-1);
...
/* do stuff */
...
VsetScreen(-1,-physbase_old,-1,-1);
VsetMode(modecode_old);

EDIT: IIRC VsetScreen will behave like VsetMode if REZ is -1; Atari does this in the VDI. This could mean (VsetScreen(-1,yada,-1,mode) might work as intended.

[mikro]

EDIT: IIRC VsetScreen will behave like VsetMode if REZ is -1; Atari does this in the VDI. This could mean (VsetScreen(-1,yada,-1,mode) might work as intended.

Unfortunately, it works as expected -- 'mode' is then ignored. So yes, it's not crashing the TOS anymore but one has to call VsetMode anyway.

[shoggoth]

Question is - did Atari break something, or do we have the wrong expectations. Looking at Thorstens reconstructed TOS1.x-sources, even TOS1.x reinitialises a bunch of global LineA variables and possibly also clears the screen, but the impact seems very limited in that case. On the Falcon, it has to do a bit more since the VDI has been developed further to support device drivers, the screen sizes differ, things explode. But perhaps Atari didn't break anything, it's just that Setscreen() is unsuitable when creating private screens in a multitasking environment.

Snippet from Thorstens GitHub:

/* --- set logical location: */
tst.l 4(a7) /* if(logloc < 0) then ignore it */
bmi.s nosetl
move.l 4(a7),ABSW(_v_bas_ad)
nosetl:
/* --- set physical location: */
tst.l 8(a7) /* if(physloc < 0) then ignore it */
bmi.s nosetp
move.b 9(a7),ABSW(v_bas_h)
move.b 10(a7),ABSW(v_bas_m)
#if TOSVERSION >= 0x106
move.b 11(a7),ABSW(v_bas_l)
#endif
nosetp:
/* --- set screen resolution (clears the screen, clobbers the cursor): */
tst.w 12(a7) /* if(rez < 0) then ignore it */
bmi.s nosetr
move.b 13(a7),ABSW(_sshiftmod) /* set software shadow */
bsr _vsync
move.b ABSW(_sshiftmod),ABSW(v_shf_mod) /* set hardware location */
clr.w ABSW(_vblsem) /* disable vblank processing */
jsr esc_init /* re-initialize glass tty routines */
move.w #1,ABSW(_vblsem) /* re-enable vblanks */

[ThorstenOtto]

I think the problem is that the VSetScreen call that Mikro uses *only* changes the physical address, but not the logical address. The logical address is that what VDI and also the vt52 emulation uses, and bad things will happen if for example the mouse cursor is drawn in an area that is not large enough for the current resolution.

[shoggoth]

I think the problem is that the VSetScreen call that Mikro uses *only* changes the physical address, but not the logical address. The logical address is that what VDI and also the vt52 emulation uses, and bad things will happen if for example the mouse cursor is drawn in an area that is not large enough for the current resolution.

If he changes only the physical address, the mouse/vt52 stays within the logical screen space. If you want a private screen, that’s what you have to do. And changing the logical screen resolution can only be done safely by the AES anyway. So I think changing the physical address + VsetMode is the only clean way to achieve what he’s doing.

[mikro]

Of course, it is what is it. I'm just surprised that they put so little effort to validation. I mean, if my usage is illegal, how hard would be to return "bro, you can't change only one pointer when calling with rez != -1? Or at least document it. There are two pointers, both of them can have three different states (no change, alloc, pointer) so naturally, people will try to see what happens.

And the fact that VsetScreen does indeed return a (pretty useful) return value even if they didn't document it anywhere is just another sad story.

[shoggoth]

Yeah. I suspect a lot of it has to do with things being a ”last minute wrapup”. There has gone a lot of thought into the TOS4 VDI for example, but they more or less hid it from the world to get something released.

[shoggoth]

And I don’t think your usage is invalid - it’s just not suitable for your purpose. It’s perfectly ok for a VDI/AES or an auto folder thing, but not for a multitasking environment (which too was a last minute thing).

[ThorstenOtto]

If he changes only the physical address, the mouse/vt52 stays within the logical screen space.

And that's the problem: it doesn't stay in the logical screen space. VSetScreen modifies the line-a variables, and other internal variables for the new resolution. But Logbase() still points to a different buffer that is only valid for the previous resolution.

[shoggoth]

If he changes only the physical address, the mouse/vt52 stays within the logical screen space.

And that's the problem: it doesn't stay in the logical screen space. VSetScreen modifies the line-a variables, and other internal variables for the new resolution. But Logbase() still points to a different buffer that is only valid for the previous resolution.

Yes. Unless REZ == -1, and you only change physbase. In such case it doesn't touch any of that, which makes it possible to create multitasking friendly private screens. I.e. fun stuff.

[sunaiac]

This topic prompted me to try and correct that opendune2 init/deinit problems.

Code was like this (after my first batch of corrections):

Code: Select all

		
		s_workPhysBase = Mxalloc(...);	/* allocate 4 lines more for explosions */
		memset(s_workPhysBase , ...);
		VsetScreen(-1, s_workPhysBase , -1, -1);
		(void)VsetMode(newMode);
		VgetRGB(0, 256, s_paletteBackup);	/* backup palette */

and deinit was like this :

Code: Select all

		VsetRGB(0, 256, s_paletteBackup);
		(void)VsetMode(s_savedMode);
		VsetScreen(-1, Logbase(), -1, -1);
		Mfree(s_savedPScreen);

Symptom : crash on exit.

After reading this topic, I changed it to :

Code: Select all

		s_workPhysBase = Mxalloc(...);	/* allocate 4 lines more for explosions */
		memset((void*)s_workPhysBase, ...);
		VgetRGB(0, 256, s_paletteBackup);	/* backup palette */
		s_savedPhysBase = Physbase();		/* backup physical screen address*/
		(void)VsetMode(newMode);
		VsetScreen(-1, s_workPhysBase, -1, -1);

And

Code: Select all

		VsetScreen(-1, s_savedPhysBase, -1, -1);
		(void)VsetMode(s_savedMode);
		VsetRGB(0, 256, s_paletteBackup);
		Mfree(s_workPhysBase);

Now I don't have a black screen. I have a pumpkin screen. That's the season but not what I want
I tested moving VsetRGB around, and realized that I probably didn't have a crash, just a black screen. Why ? because the code above leaves game correctly but desktop palette is bad. depending on VsetRGB position, palette is actually all black. I confirmed by pressing enter several times on a black screen and game restarted.

So, my question is, what is the recommended order for VsetRGB and VgetRGB relative to the mode setting and screen setting ?
Anyone already had such problems ?
I have not been able to get back the right palette
Yet I'm resetting things in reverse order of my setup (thanks @shoggoth), and the first and last things I do is touch the palette, so I'm lost.

[sunaiac]

Oh, just got an idea, I start from and return to a 16 colors desktop. Game is 256 colors. Maybe I should just try to read and write the right number of colors in case palette for those 16 colors are stored in the top of my 256 entries table but read from the bottom or whatever. Testing that, coming back to you.

[sunaiac]

That works.
Do not try to read and restore more colors than the current palette depth.
Thanks guys, nice talking to you
(rubber duck debug for the win )

doc (https://freemint.github.io/tos.hyp/en/S ... ml#VgetRGB) would have deserved warning about those undefined behaviours
I'm just starting atari dev, I feel sorry for you guys fighting this since forever

[shoggoth]

The platform is full of such things!

[sporniket]

doc (https://freemint.github.io/tos.hyp/en/S ... ml#VgetRGB) would have deserved warning about those undefined behaviours
I'm just starting atari dev, I feel sorry for you guys fighting this since forever

Since the doc project is on github, I created an issue for that.

[sporniket]

doc (https://freemint.github.io/tos.hyp/en/S ... ml#VgetRGB) would have deserved warning about those undefined behaviours
I'm just starting atari dev, I feel sorry for you guys fighting this since forever

Since the doc project is on github, I created an issue for that.

...and after 15 minutes trying to figure how github dev works (note : copy/paste do not works...), there is a pull request

edit1: but, i guess weird text encoding (meaning, NOT utf-8) seems to have ruined the patch ...

edit2: had to fix my patch with local wizardry, it looks ok now, we just have to wait for it to be approved and merged

edit3: after a little formatting fix, it is merged, the documentation is up to date

[sunaiac]

Sorry I thought that was Atari docs and not supposed to change
Next time I'll propose the update myself, my bad