mfld-fr / emu86

Intel IA16 emulator for embedded development
35 stars 6 forks source link

Add scroll down ELKS/PC ROM BIOS, initial MDA support #64

Closed ghaerr closed 3 years ago

ghaerr commented 3 years ago

ELKS vi uses ROM BIOS scroll up/down; this PR implements that and enhances BIOS scrolling to work in all cases. con_scrollup API changed to con_scroll with direction parameter. Added to PC and ELKS ROM BIOS. Fixes various other issues with attributes passed via BIOS functions. More cleanup of con-sdl.c.

Adds initial MDA support with special attribute handling. In the future could be dynamic, this version enable by setting VID_MODE in mem-io-elks.h.

mfld-fr commented 3 years ago

OK for the scrolling, but I dislike the MDA attributes support with a static #if / #else switch as proposed, as there is already a dynamic switch between video modes 3 & 7 at the BIOS level. Moreover, the conversion from MDA attributes to RGB colors is not accurate, even without the blinking and the underlining (see https://github.com/mfld-fr/emu86/pull/58#discussion_r628938986 or https://www.seasip.info/VintagePC/mda.html)

With a fixed 80x25 character matrix in both modes, it seems to me that switching from MDA attributes to EGA attributes (and vice-versa) dynamically is not a big deal. Could you please come back with an improved PR that would be more consistent with the current BIOS, i.e. where the character attribute passed by the BIOS would be converted to foreground & background RGB values based on the current video mode ?

ghaerr commented 3 years ago

I dislike the MDA attributes support with a static #if / #else switch

Well, I did say "initial MDA support"... 😄

Last commit dynamically handles EGA and MDA depending on current video mode.

the conversion from MDA attributes to RGB colors is not accurate

I think it was, but decided to rewrite it more simply. The new implementation only uses one RGB conversion table, when in MDA video mode maps attributes properly to BLACK, GREEN or LTGREEN, using indices into the table.

This PR now properly handles EGA and MDA automatically, tested on ELKS using vi on BIOS Console, which is dependent on bios scrolling and EGA/MDA screen attributes.

There is still some more cleanup and code rearrangement to do, concerning exactly where code should live for some video defines and shared BIOS stuff, but that can be discussed in separate PR.

ghaerr commented 3 years ago

@mfld-fr : Try as I might, I can't seem to squash the last "Fix initial video mode", I used git rebase -i origin/scroll~4 scroll, the same mechanism I used to squash the last commit you requested (which worked). This time, I get the following error after rebase:

Gregs-MBP2:emu86 greg$ git push
warning: redirecting to https://github.com/ghaerr/emu86.git/
To http://github.com/ghaerr/emu86.git
 ! [rejected]        scroll -> scroll (non-fast-forward)
error: failed to push some refs to 'http://github.com/ghaerr/emu86.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
Gregs-MBP2:emu86 greg$ git status
On branch scroll
Your branch and 'origin/scroll' have diverged,
and have 5 and 7 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

This has happened twice, I updated my GitHub master branch with other commits from your repo, pushed, then tried again by deleting my local branch, fetching origin scroll, and still the same problem.

I have little experience with git, I am sorry for the trouble. I'm afraid if I continue, I may destroy the entire PR. The current PR (branch scroll) is correct; just containing the extra non-squashed commit.

ghaerr commented 3 years ago

Yep, did a git push origin +scroll and completely screwed things up - 3 previous commits were added to this PR. I have no idea how to fix this without screwing things up worse.

Attached is patch of what PR should look like. Let me know if you would like me to close this and open a new PR. scroll.patch.zip

mfld-fr commented 3 years ago

So bad... let me try to pick the changes from your patch file...

mfld-fr commented 3 years ago

Changes pushed to the next branch.