Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sprites_4.bas causes VDP to shut down after several minutes #66

Closed
jonathan-teh opened this issue May 30, 2023 · 11 comments
Closed

sprites_4.bas causes VDP to shut down after several minutes #66

jonathan-teh opened this issue May 30, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@jonathan-teh
Copy link

I suspect this is a VDP bug but since the reproducer is in BBC BASIC I've raised it here. Straightforward to reproduce:

LOAD "SPRITES_4.BAS"
RUN

For me, after between 1 min 30 s and 5 minutes (across 4 runs), VDP appears to shut down, the monitor no longer detects a signal and shuts off, the keyboard is unresponsive to any input e.g. to toggle caps lock or num lock. Pressing the reset button is the only way back to a usable machine.
sprites_5.bas can also cause a shutdown but takes longer, 12 minutes on the first run, the second run lasted 16 minutes before I stopped it.
MOS 1.03, VDP 1.03, BBC BASIC 1.04 on an Agon Light 2. Has been reproduced on another Agon Light 2 running the same software versions.

@AntonioCarlini
Copy link

I can reproduce this easily with no special effort. Boot into BBC BASIC, "LOAD TESTS\SPRITES_4.BAS", "RUN" and wait.
Reproduced fives times out of five attempts:
Trial A: Unresponsive by 10 mins
Trial B: Screen blanked after 3mins
Trial C: Screen blanked @ 1m09s
Trial D: Screen blanked @ 10s
Trial E: Still running @ 9m30s, but screen blanked by 30m

In Trial A I set it of and only came back after 10m; in trial E I gave up watching after ~10m but when I came back after 30m the screen was blank. For B, C and D I sat and watched with a timer running on my phone.

AL2 with MOS 1.03, VDP 1.03, BBC BASIC 1.04

@lennart-benschop
Copy link
Contributor

I get the same crash on the original Agon Light (one that was assembled by Bernardo himself), with sprites_4.bas, and also with sprites_5.bas

@jonathan-teh
Copy link
Author

Trying to gather more data points, I changed line 960 to MODE 3 (640x480) and it survived 30 minutes until I manually stopped it. Thinking it might have something to do with sprites moving offscreen, I changed these lines:

   40 SW%=320-16
   50 SH%=200-16

but with the usual MODE 2 it crashed after 2 minutes.
I then tried MODE 1 (512x384) and it crashed after 20 minutes.

@jonathan-teh
Copy link
Author

Looking around, a similar bug was fixed for VDP 1.03 RC3 in #28 by 94220de (perhaps a link keyword would make it easier to find) but prior to that it was easier to reproduce with sprites_5.bas rather than sprites_4.bas.
Then perhaps it regressed in VDP 1.03 RC5 with #40 from 1da1509 and ba597c8.

@breakintoprogram breakintoprogram added the bug Something isn't working label Jun 27, 2023
@breakintoprogram breakintoprogram transferred this issue from breakintoprogram/agon-bbc-basic Jun 27, 2023
@breakintoprogram
Copy link
Owner

This seems to be related to animating the sprite (VDU 23, 27, 8). I've removed that from the demo and all is well.

@breakintoprogram
Copy link
Owner

In sprites.h, changing the function nextSpriteFrame to this seems to fix the issue

void nextSpriteFrame() {
	auto sprite = getSprite();
	auto frame = sprite->currentFrame;
	sprite->setFrame(frame < sprite->framesCount -1 ? frame + 1 : 0);
}

which seems to indicate an issue with sprite->nextFrame()

@breakintoprogram
Copy link
Owner

breakintoprogram commented Nov 15, 2023

I think I see what is happening here:

This function in vdp-gl (~line 571 of displayController.h) could for a brief period increment currentFrame to an invalid frame before the if condition is triggered to reset it to 0. If a sprite were to be drawn at that point, then you'd get a guru exception.

void nextFrame() { ++currentFrame; if (currentFrame >= framesCount) currentFrame = 0; }

I've modified the code locally as follows and am testing now.

void nextFrame() { currentFrame = (currentFrame  < framesCount - 1) ? currentFrame + 1 : 0; }

@breakintoprogram
Copy link
Owner

This has now been running for over 90 minutes on my test Agon, whereas before it would last no longer than 5-10 minutes. I've issued a pull request on the vdp-gl repo here avalonbits/vdp-gl#31.
As this is not a bug in agon-vdp, and there will be no changes on my side, I'm going to close this card as 'Won't Do'.

@breakintoprogram breakintoprogram closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2023
@jonathan-teh
Copy link
Author

Thanks for fixing this. Should the fix also be sent upstream to FabGL?

@stevesims
Copy link
Contributor

excellent investigation work @breakintoprogram

I'd never managed to reproduce this error when I'd tried - left sprites_4 running for many many minutes, maybe even hours before with it seemingly quite happy. the unlikely circumstance of the nextFrame method getting interrupted between its two statements would make it an incredibly rare crash indeed. 😁

I've been trying to fix sprites performance to reduce flickering for the past couple of weeks, and have been incidentally trying to look out for this bug. I have seen occasional/rare crashes when testing out my revised sprites code; i had put that down to bugs in my own code, which I couldn't see, but at least some of the crashes was probably this bug 😁

@breakintoprogram
Copy link
Owner

Cool, thanks @stevesims. It's a quick fix. Ideally it should fail gracefully if an invalid sprite frame is selected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Won't Do
Development

No branches or pull requests

5 participants