Closed mras0 closed 2 years ago
Interesting test case!
I've converted it to an ADF to make it easier to run it on real machines: bplcon-new.adf.zip
On my A500 8A it looks as described on the EAB thread:
If I interpret the image correctly, I guess the part marked with the yellow arrow is the part that is "missing" in the area marked with the blue arrow.
In the upper part, vAmiga seems to do the right thing.
What I've learned from the test case so far is that the following line in Denise::beginOfLine
is definitely wrong:
// Prepare the bitplane shift registers
for (isize i = 0; i < 6; i++) shiftReg[i] = 0;
When this line is removed and the shift registers are only cleared when they were used, it looks like this:
It's better, but still not right.
I probably nailed it:
Here is a summary of what I had to change in vAmiga to make it work:
Denise::beginOfLine(isize vpos)
:
Don’t clear the shift registers.
Denise::drawOdd(Pixel offset)
:
Denise::drawEven(Pixel offset)
:
Denise::drawBoth(Pixel offset)
:
Only clear the shift registers that have been pumped out.
Denise::setBPLxDAT(u16 value)
:
Only feed the BLTDAT registers of enabled bitplanes into the pipeline. The BPU value is determined by the Agnus view on these bits, not the Denise view. The test program is timing-sensitive here.
Denise::updateShiftRegisters()
:
Always move all pipeline registers to the shift registers, no matter how many bitplanes are enabled.
I have created a separate branch for the new code: https://github.com/dirkwhoffmann/vAMIGA/tree/issue609
Right now, the code is not on the dev branch, because
The test case written by EAB user "losso" turns out to be a very good one.
Here is a combination where vAmiga still does it wrong:
It is supposed to look like this:
To ease debugging, I've added the ability to wipe out certain bitplanes. The functionality is accessible via the debug console:
The parameter is a bit mask. E.g, a value of 3 will wipe out biplane 1 and 2.
This is how the test case looks like with plane 5 disabled:
Conclusion: If BPLCON1::PF1 equals 0x09, vAmiga still fills shift register 5, but the real machine does not.
Weird, for the $9 case, your vAmiga screenshot matches what losso posted (pf1-screenshots.zip):
And that's also how Toni has implemented it:
Weird, for the $9 case, your vAmiga screenshot matches what losso posted (pf1-screenshots.zip
Interesting. I'll recheck this on my real machine...
I've rerun the test on my A500 and it looked exactly the same to what I've posted above. I've also checked on another A500 (the one which we call MMSE (Marble Madness Special Edition), because it became the victim of an epic retrobright fail). On this machine, it looks similar to what losso has posted:
Both are rev. 8A A500's, but I haven't checked the exact Agnus or Denise revision numbers.
@mras0: Could you do me a favour and run this combination with the latest UAE build?
Using A500 ECS Agnus quickstart.
Using A500 OCS (most common) quickstart.
I think they look the same, but Toni apparently found a subtle difference between ECS/AGA and OCS.
Very interesting that it can give different results on Amigas that should otherwise be very similar.
EDIT: Missed that the difference was only implemented in subpixel mode. Trying to figure out how to get screenshots of that. EDIT2: Doesn't make a difference for these cases as far as I can tell.
Difference is between OCS Denise and ECS Denise. There are some internal differences in shifter pipeline (that some specific tests show) but more about that later...
Difference is between OCS Denise and ECS Denise
Interesting stuff. I'm curious about the details.
I'm still struggling with a fundamental flaw in my Denise implementation. There is a specific thing which I don't understand. The following picture shows a test case using a DDF window starting at $38. The test iterates BPLCON1 through $11, $22, $33,... On my A500, the image looks as expected:
Now, when I change DDSTRT to $3A, it looks like this:
What I don't understand is how DDSTRT interacts with BPLCON1. Why does the upper part of the image jump to the right?
I guess you use BPL1DAT position as BPLCON1 "reference" position. It does sound logical but it is wrong. Correct is Denise internal horizontal counter. (=if BPL1DAT position = DDFSTRT position is "unaligned" with Denise horizontal counter -> offset)
Logic something like "if ((horizontal counter & mask[7=hires,15=lores]) == BPLCON1[odd/even]) and BPLxDAT_temp has been updated (=BPL1DAT was written since last comparison match) and plane is active: copy BPLxDAT_temp to shift register. Clear "has been updated" flag.
This logic has low level differences on OCS vs ECS Denise vs AGA, for example OCS Denise seem to have parallel shifters if hires (most likely because OCS chip technology couldn't do full 14MHz), one for odd and one for even hires pixels (and most likely it is not full 2x8 but only 2x4 and copy is done twice per BPL word, to save logic gates). ECS probably was fast enough to support hires with single shifter: lores and hires were most likely merged, shres most likely uses dual pipeline. AGA probably got rid of dual stuff completely. This is speculation only but at least OCS Denise "dual" behavior is quite obvious, have lores single bitplane with all bits set to one, switch to hires mid-scanline and you'll notice interesting 2x4 pixel pattern (make sure BPLCON1=0 to make it very clear). This pattern does not appear if ECS Denise or AGA.
EDIT: "BPLxDAT was written since.." changed to "BPL1DAT was written since.."
@tonioni Thanks a lot for the explanation. I get the idea, but I am still not sure if I understand all pieces. In the following, I distinguish four events:
aa : Read BPL1DAT from memory (DMA)
bb : Transfer BPL1DAT, ..., BPL5DAT to temp registers
cc : Transfer temp registers to shift registers (if bb happened before)
dd : Start shift registers
Let's examine the following four cases:
In my current understanding, the following happens. Is this correct or am I still missing something?
1. DDFSTRT = 38
BPLCON1 = 00
<---- Fetch unit -----> <---- Fetch unit -----> <---- Fetch unit ----->
37 38 39 3A 3B 3C 3D 3E 3F 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F
aa aa aa
bb bb bb
cc cc cc cc cc cc
dd dd
2. DDFSTRT = 38
BPLCON1 = 44
<---- Fetch unit -----> <---- Fetch unit -----> <---- Fetch unit ----->
37 38 39 3A 3B 3C 3D 3E 3F 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F
aa aa aa
bb bb bb
cc cc cc cc cc cc
dd dd
3. DDFSTRT = 3A
BPLCON1 = 00
<---- Fetch unit -----> <---- Fetch unit -----> <---- Fetch unit ----->
37 38 39 3A 3B 3C 3D 3E 3F 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F
aa aa aa
bb bb bb
cc cc cc cc cc cc
dd dd
4. DDFSTRT = 3A
BPLCON1 = 44
<---- Fetch unit -----> <---- Fetch unit -----> <---- Fetch unit ----->
37 38 39 3A 3B 3C 3D 3E 3F 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F
aa aa aa
bb bb bb
cc cc cc cc cc cc
dd dd
Assuming ECS Agnus (=DDFSTRT 3A is valid, bit 1 exists in DDFSTRT). Fetch start can be unaligned, Agnus does not care, if DDFSTRT is 3A, it will start from 3A.
After working on remote servers for a while, I am back at improving emulation accuracy. With my latest code changes, I think I got it right. All value combinations that didn't work previously seem to work fine now. I.e., combination C9 / 5 gives the correct result now:
Super low priority, but may be of interest.
@mras0: Thanks again for starting this thread. Losso's test case revealed a big bug in vAmiga. In hindsight the priority was huge (which was impossible to know at the beginning of course).
No problemo, always happy to create extra work for others ;)
When I have some time, I'll update the vAmigaSDL port to the latest version, and maybe take a stab at adding recorder support for Windows if you're open to incorporating a patch/pull request.
P.S. you should just close the issue when you feel like it, I can of course help you check, but you're still in a better position to know when it's done.
take a stab at adding recorder support for Windows if you're open to incorporating a patch/pull request.
Yes, cool. I'd be happy to merge pull requests in.
you should just close the issue when you feel like it,
You mean this issue? When an issue is solved, I usually put a label on it (V1.1 in this case) and close all labeled issues altogether when a new version is releases. This makes it easy for me to remember all changes to the previous release (I am usually too lazy to keep separate release notes).
take a stab at adding recorder support for Windows if you're open to incorporating a patch/pull request.
Yes, cool. I'd be happy to merge pull requests in.
👍
you should just close the issue when you feel like it,
You mean this issue? When an issue is solved, I usually put a label on it (V1.1 in this case) and close all labeled issues altogether when a new version is releases. This makes it easy for me to remember all changes to the previous release (I am usually too lazy to keep separate release notes).
Great, yes, I meant this issue. Was just reminded by the "close with comment" button that I was the reporter :)
Fixed in v1.1b8
Super low priority, but may be of interest. Thread on EAB. You might not want to start work on it until Toni does the debugging :)
It should look like this:
(EDIT, since it's easier to compare) Looks like this in 1.1 Beta 5 (web edition):
But I think it looks like this in latest vAmiga (5445aaf907a6c162eaf233584e92da9f17cfcf9a, big disclaimer follows):
Screenshot is from my very, very early attempt at getting an up to date vAmiga running locally under WSL2 (for various reasons I couldn't get it to compile easily under Windows), so it might work with correct settings.
I'm using (in addition to KS1.3):
But something else might be out of order, so take it with a grain of salt.