[VIC-20] CRAM_PTR incorrect at startup

[0cjs]

As described in detail in this Retrocomputing StackExchange answer, at the start of main() on the VIC-20, when loading/running the PRG with VICE's -autostartprgmode 1 option, CRAM_PTR is incorrectly set and any printing using cputc() will not be visible (because the colour data is being set in the wrong place) until something is done that would reset the cursor position, such as clearing the screen, doing a gotoxy() or printing a newline. My vic20cc65 repo contains code and scripts to easily demonstrate this.

(I've reproduced this using VICE 3.0.0 on Debian and ROM images from a source I can't remember now; as you can see from the post another user has reproduced this too, most likely with a different system.)

I'm not sure exactly what the true source of the problem is. If the bug is in VICE it should obviously be fixed there, but that seems less likely to me than a bug in the VIC ROM or a misunderstanding on the part of vic20.lib on how CRAM_PTR is supposed to work. (That said, I'm no expert on this, so I could well be wrong.)

I've looked through libsrc/vic20/crt0.s and I don't see anything obviously wrong there, unless it's actually supposed to be checking or resetting CRAM_PTR; right now it doesn't appear to touch it at all, so when main() is run it should be exactly what it was set to by the VIC ROMs.

So, does this need a fix in vic20.lib? If so, what should the nature of the fix be?

I'd be very pleased to code up the fix and submit a PR if the fix should be done here and someone can give me a quick summary of what approach to take. (I need the coding practice and, hey, it's Hacktoberfest. :-))

[oliverschmidt]

First of all thanks for the detailed problem description :-)

For now I'll classify this issue as question. I hope the Subject Matter Experts will pick it up and there will be a conclusion on how to proceed...

[0cjs]

Ah! Looking through this further, I see we have a libsrc/vic20/kplot.s that replaces the Kernal PLOT function at $FFF0, because "[t]he kernal function does not set the pointer to the color RAM correctly."

(In asminc/cbm_kernal.inc, however, PLOT is still set to $FFF0 if __VIC20__ is defined. I'm not sure exactly how the replacement overrides this, if indeed it does.)

My guess at this point about what's going on is:

1. PLOT from kplot.s is used.
2. However, it's not called until plot in libsrc/vic20/cputc.s is called.
3. That in turn is not called by _cputc in that file until a CR is printed.
4. Thus, when you first print, the bad CRAM_PTR setting left behind by the VIC Kernal at startup is used until you print a CR, causing your output to be invisible.

My guess at a good solution would be that plot in libsrc/vic20/cputc.s should be called at initialization, possibly via the __CONSTRUCTOR_TABLE__ thing that I don't understand yet. That routine loads CURS_X and CURS_Y, which I have verified are correct, and sets the other pointers to the correct values based on those.

[mrdudz]

I wonder if it wouldnt be a good idea to define conio so that we guarantee that when conio is being used, the program will start with screen cleared, and the text color being different to the background color (this will take care of initializing the mentioned pointers too). Simply because conio does not support scrolling, which (on some targets) means a single call to any of the text output functions results in undefined behaviour (or even a crash).

[0cjs]

Ok, further research seems to indicate that a good solution for this would indeed be to add a .CONSTRUCTOR for the VIC-20 conio routines. This would call the bugfixed PLOT in libsrc/vic20/kplot.s either via plot in libsrc/vic20/cputc.s or by doing the same thing that it does.* This will fix up the bogus CRAM_PTR at entry with no other effects. This should also have quite minimal (hopefully almost no) impact on memory usage since it will be in the ONCE segment, which I assume from the name is released for other uses (the heap?) after all constructors have run. (And to be clear, this should be just for VIC-20; other platforms don't need this and should not take the admittedly trivial hit.)

BTW, just read through libsrc/runtime/condes.s. That's very cool code.

---

*It clears the carry and loads CURS_X into the Y register and CURS_Y into the X register. Welcome to Commodore-land. However, at least it gets points for tail call optimization.

[0cjs]

@mrdudz Well, on the CBM machines, or any machine really, it's certainly not necessary that we do that, and my immediate thought is that it might be better to preserve the flexibility of being able to, if you're careful, e.g., print just a little bit without blowing away the screen contents or changing the current colour, or even read those screen contents.

That said, it may be more helpful to those not familiar with conio* to do that than it hurts those who understand it better and have other needs. But that's a change across all platforms, not just VIC-20, that definitely warrants its own issue. If there are other platforms that go wrong on the first write when it's not off the screen, though, I'd agree that they also need a constructor or whatever that fixes that.

---

*Until I read this and tested it just now, I didn't know it doesn't scroll! Or how it gets weird when you do things that would normally scroll. I now suspect that the poster of the original question should have been using stdio instead of conio; I suspect that he assumed as I did that it would work like the VIC-20 Kernal routines which do scroll on output.

[mrdudz]

if you're careful, e.g., print just a little bit without blowing away the screen contents or changing the current colour, or even read those screen contents.

my point is that you (the program) do not "know" where the cursor is when you start printing, unless you explicitly set the cursor position first (the user may have typed "RUN" in the bottom line of the screen, for example). the same is true for the text- and background color (although less likely that the user would use an unreadable combination, of course).

[0cjs]

@mrdudz

...you (the program) do not "know" where the cursor is when you start printing, unless you explicitly set the cursor position first...

If you don't know without setting the cursor position, it's because you decided not to call wherex() and wherey() to find out, and that is hardly the library's fault.

I do understand the point you're making, or at least I understand how there are reasonable arguments that can be made that conio should initialize the screen in some way. But I also see good arguments against that, and other possible solutions to the underlying issues that make you want to clear the screen at startup when conio is being used. (A really basic argument against it would be that the screen would be cleared even if you decide, after examining the conditions at startup, that you won't use conio.)

If conio is defined to clear the screen and set colours, you're basically saying that conio can no longer be linked in to certain types of programs. That's probably a worthwhile discussion to have, but it is certainly not one for this issue, which is focused on something quite different, so if you really want this to be considered you should start a new issue. (If you click the ⋯ menu on a comment you'll find a "Reference in new issue" option that will link this discussion to it.) At any rate, I will certainly not be proposing changing the screen in any way in this issue and any PRs resulting from it. Here I'm looking for a bugfix, not a breaking API change.

[mrdudz]

If you don't know without setting the cursor position, it's because you decided not to call wherex() and wherey() to find out, and that is hardly the library's fault.

That means every program must do this, and also react accordingly. Thats likely more overhead than just clearing the screen (especially if you, as you say, include other output as an alternative).

Here I'm looking for a bugfix, not a breaking API change.

My proposal is changing undefined behaviour into defined behaviour - thats not a breaking API change at all. (It is not guaranteed the screen will NOT be cleared on startup either - and some targets (must) do it)

[0cjs]

That means every program must do this, and also react accordingly.

No, only programs that use conio. Not just link it in, but decide to use it at runtime as well.

My proposal is changing undefined behaviour into defined behaviour....

You are proposing a change that could break programs that rely on the current behaviour of conio which, though perhaps not defined in documentation, is well-defined by the code itself for at least some platforms.

At any rate, can you please, please, take this to an appropriate forum (like a new issue in this repo) for "I want to change how an API works"? Like I said, I'm not saying you don't have a point worth discussion, but I am very definitely saying that if you restrict your discussion to this issue you will definitely not get the change you're suggesting.

[mrdudz]

i dont care if i "will get" the change i am suggesting. i am suggesting a possible fix for the problem described. but sure, i will stop. just one thing: the point of conio is portability across all(!) supported targets. a program relying on undefined behaviour is broken, even if it happens to work on some target. and i am not proposing to change the API at all - i am suggesting to define how it works more clearly, in a way which happens to fix this bug, and also makes the API inhibt less surprises for portable programs. and using very little overhead at that, for any non trivial program anyway.

[greg-king5]

I think that the condition of the screen when a program starts should be labelled "platform-defined behaviour". I agree that a portable program shouldn't rely on that behaviour. But, that's the responsibility of the portable program, not the system library.

conio is supposed to be simple, small, and fast. It shouldn't change the screen, or move the cursor -- except when told to do it by the program. It should give us the flexibility to exploit the previous contents of screens on systems that don't erase them when starting a program.

(I remember that we had this discussion about TGI, a long time ago. We decided that TGI shouldn't pre-clear the graphics screen. It should let us re-use old pictures -- if it is possible.)

[greg-king5]

I looked at the VIC-20 firmware. I concluded that the behavior of its PLOT function isn't broken -- strictly speaking. What happens is that the color pointer is updated by the code that uses that pointer. That is, each time a character is printed on the screen, the pointer is updated just before the screen code and color are poked into RAM. Also, the pointer is updated before the cursor is "blinked" over a character on the screen. That method is inefficient. The color pointer's value changes only after the screen pointer's value changes (when the cursor moves to another row). Therefore, most of the updates do nothing but waste time.

The Commodore 64's ROMs were copied from the VIC-20's ROM. Therefore, its Kernal had that inefficiency. But, the firmware was improved in a later revision. After that change, the color pointer is updated only when the screen pointer is updated.

"kplot.s" uses the new method; and, cputc() assumes it. But, when a program starts, the color pointer still has the old method's value. Therefore, we need a constructor that does the update that normally would be done by the next call to Kernal's CHROUT function.

Both cgetc() and cputc() need it. The constructor can be put into "conio.s". "cgetc.s" and "cputc.s" must .forceimport that constructor. That "actor" will be very simple:

.constructor initcram

initcram := $EAB2

It will add two bytes to programs. (The advantage of the ONCE segment hasn't been implemented for the VIC-20, yet.)

[0cjs]

This is now fixed by #965. Many thanks to @greg-king5 for his extensive help with this.

[oliverschmidt]

Thanks to both of you from my side.