v86 icon indicating copy to clipboard operation
v86 copied to clipboard

Add VGA graphical text mode

Open chschnell opened this issue 1 year ago • 5 comments

Added alternate text mode "graphical text mode" that uses internal VGA fonts and settings in text mode with the canvas that is otherwise used in graphical mode.

Added new configuration option screen_options to V86 constructor, which is needed for two screen-related options:

  • use_graphical_text: Boolean, enable VGAScreen's graphical text mode if true
  • disable_autoscale: Boolean, disable ScreenAdapter's auto-scale feature if true

VGAScreen's current behaviour is unmodified unless use_graphical_text is set to true, this PR is intended to be a non-breaking change.

Added new bus event screen-settings-changed, it reports details about the current video mode settings when graphical text mode is enabled, event arguments:

  • in text mode: [false, txt_width, text_height, font_width, font_height]
  • in graphical mode: [true, gfx_width, gfx_height, virtual_width, virtual_height]

Added new VGAScreen function grab_text_content() that returns an array of strings containing the current text screen content (in CP437).

Supported VGA text mode features:

  • monochrome and 16 colour modes
  • 40 and 80 columns
  • 25, 28 and 50 rows
  • font height 1-32 pixel
  • font width 8, 9 or 16 pixel
  • cursor emulation
  • blinking text
  • font page A/B (512 character mode)
  • line graphics enable (continuous horizontal line drawing characters)

Usage example for new optional setting screen_options:

window.emulator = new V86({
    wasm_path: "v86.wasm",
    memory_size: 16 * 1024 * 1024,
    vga_memory_size: 2 * 1024 * 1024,
    screen_container: document.getElementById("screen_container"),
    screen_options: {
        use_graphical_text: true,
        disable_autoscale:  true
    },
    bios: { 
        url: "seabios.bin" 
    },
    vga_bios: { 
        url: "vgabios.bin" 
    },
    fda: { 
        url: "freedos13.img" 
    },
    autostart: true, 
});

Open issues:

  • split screen (looking for example)
  • 43 rows is refused by both MS-DOS and FreeDOS
  • Monochrome text-mode is refused by MS-DOS

This PR should resolve #1098 (more details there).

chschnell avatar Aug 14 '24 10:08 chschnell

Thanks!

I've left a few stylistic comments. However, looking at the bigger picture, many of the changes in vga.js look similar to this:

-    this.bus.send("screen-update-cursor-scanline", [start, end, visible]);
+    if(this.graphical_text)
+    {
+        this.graphical_text.set_cursor_attr(start, end, visible);
+    }
+    else
+    {
+        this.bus.send("screen-update-cursor-scanline", [start, end, visible]);
+    }

Which indicates that it might be better to merge src/vga_text.js into src/browser/screen.js. It would also be nice to reuse the screen-put-char machinery.

You're welcome, and thank you very much for taking the time for a close look at all this, I really appreciate that.

I agree that the new code should be merged into existing classes, it is structured that way (minimal changes to existing code) to best oversee how.

Here's where I ended up: In graphical text mode, the entire DIV-based text implementation in ScreenAdapter is currently dead on purpose as I did not see any use for it, the code is 100% "living off the land" in VGAScreen, specifically without the 2nd text buffer ScreenAdapter.text_mode_data[] and all the code that goes with it.

Are there other bus listeners besides ScreenAdapter for any of the 4 text-related bus events screen-put-char, screen-update-cursor, screen-set-size-text and screen-update-cursor-scanline that need to be taken into consideration? Could you please describe a bit more to me what behaviour you would like to see restored?

Currently, the only essential function of ScreenAdapter in graphical text mode is that it controls the rendering loop at ~60fps, faithfully believing it is in graphical mode (hehe).

I haven't made up my mind, though I currently think GraphicalText (in vga_text.js) is much closer to VGAScreen than ScreenAdapter. Maybe GraphicalText could be split across VGAScreen and ScreenAdapter since its function does sit somewhere in between the two. The rendering should happen in VGAScreen, do you agree? I need to think about this, not sure where to draw the line.

By the way, even though it is not used in graphical text mode, the HTML DIV element is still required because ScreenAdapter still assumes its existence, this could be easily changed as ScreenAdapter also knows about graphical text mode in its constructor through the screen_options constructor argument. Should I?

chschnell avatar Aug 21 '24 18:08 chschnell

Are there other bus listeners besides ScreenAdapter for any of the 4 text-related bus events screen-put-char, screen-update-cursor, screen-set-size-text and screen-update-cursor-scanline that need to be taken into consideration? Could you please describe a bit more to me what behaviour you would like to see restored? ... I haven't made up my mind, though I currently think GraphicalText (in vga_text.js) is much closer to VGAScreen than ScreenAdapter. Maybe GraphicalText could be split across VGAScreen and ScreenAdapter since its function does sit somewhere in between the two. The rendering should happen in VGAScreen, do you agree? I need to think about this, not sure where to draw the line.

Looking at this again, you're right that the line is blurry. I think we can merge this in its current design, and I'll think about potential refactoring later (see the next paragraph).

Regarding bus, this is a leftover from a very old version of v86 that supported web workers (with the idea that "bus" was the boundary between web worker and main thread). It's now mostly used as an event handler for the public V86 class. I'm thinking of removing it and replacing it with more direct calls.

By the way, even though it is not used in graphical text mode, the HTML DIV element is still required because ScreenAdapter still assumes its existence, this could be easily changed as ScreenAdapter also knows about graphical text mode in its constructor through the screen_options constructor argument. Should I?

No, an invisible div doesn't harm anyone (and code that switches between modes might assume its existence).

copy avatar Aug 24 '24 18:08 copy

Are there other bus listeners besides ScreenAdapter for any of the 4 text-related bus events screen-put-char, screen-update-cursor, screen-set-size-text and screen-update-cursor-scanline that need to be taken into consideration? Could you please describe a bit more to me what behaviour you would like to see restored? ... I haven't made up my mind, though I currently think GraphicalText (in vga_text.js) is much closer to VGAScreen than ScreenAdapter. Maybe GraphicalText could be split across VGAScreen and ScreenAdapter since its function does sit somewhere in between the two. The rendering should happen in VGAScreen, do you agree? I need to think about this, not sure where to draw the line.

Looking at this again, you're right that the line is blurry. I think we can merge this in its current design, and I'll think about potential refactoring later (see the next paragraph).

I will finish this PR today, there is only one last issue.

It just occurred to me that I haven't thought about V86's persistent state[] at all, GraphicalText is currently completely ignorant about save/restore state. Later today I will look into this and it is very likely that I will push one last patch by the evening to fix this in case it is broken (which I believe it is).

Whether or not, I'm determined to finish by this evening because I'll need the next few weeks for other things.

Can you give me a quick sketch about what needs to be considered when saving/restoring state?

chschnell avatar Aug 25 '24 08:08 chschnell

Yesterday I took a very detailled look at this PR's changeset to ensure everyhing was ok (which it wasn't), and also took notes on the design for an overview.

Here are the current relations between VGAScreen, ScreenAdapter and GraphicalText:

+---------------+         +---------------+         +---------------+
| ScreenAdapter |<= BUS =>|   VGAScreen   |<= API =>| GraphicalText |
+---------------+         +---------------+         +---------------+

In detail (functions that suppress a bus message are annotated with the message's name):

1. VGAScreen uses these public methods of GraphicalText:

   GraphicalText(bus, vga)
   GraphicalText.set_character_map(char_map_select)
   GraphicalText.set_size(rows, cols)                  // "screen-set-size-text"
   GraphicalText.set_cursor_pos(row, col)              // "screen-update-cursor"
   GraphicalText.set_cursor_attr(start, end, visible)  // "screen-update-cursor-scanline"
   GraphicalText.invalidate_row(row)                   // "screen-put-char"
   GraphicalText.invalidate_font_shape()
   GraphicalText.resize_canvas()
   GraphicalText.invalidate_all()
   GraphicalText.render()

2. GraphicalText uses these public members and methods of VGAScreen:

   VGAScreen.vga_memory[]
   VGAScreen.plane2[]
   VGAScreen.offset_register
   VGAScreen.line_compare
   VGAScreen.start_address
   VGAScreen.dac_mask
   VGAScreen.dac_map[]
   VGAScreen.vga256_palette[]
   VGAScreen.clocking_mode
   VGAScreen.attribute_mode
   VGAScreen.max_scan_line
   VGAScreen.scan_line_to_screen_row(VGAScreen.line_compare)

3. ScreenAdapter and VGAScreen interface each other through the bus only.

4. ScreenAdapter and GraphicalText do not interface each other.

I also cobbled a crude hack together where I changed the design to this:

+---------------+         +---------------+         +---------------+
|   VGAScreen   |<= BUS =>| ScreenAdapter |<= API =>| GraphicalText |
+---------------+         +---------------+         +---------------+

This proof-of-concept worked, VGAScreen was left with almost no changes (compared to the current master) and it was as ignorant about GraphicalText as ScreenAdapter currently is in my PR. ScreenAdapter becomes the owner of the private GraphicalText instance (currently VGAScreen is the owner). There are only a few hurdles for this transformation, I guess it would take me a couple of hours to make it right and proper. As before, GraphicalText needs a valid reference to VGAScreen, but that's solvable.

If this design issue comes up in the future I'd be happy to help.

chschnell avatar Aug 25 '24 09:08 chschnell

Can you give me a quick sketch about what needs to be considered when saving/restoring state?

Never mind, I did a few tests and it seems to work as it is, allthough I'm not sure I'm testing right.

I saved the state, reloaded my browser, rebooted using the same image and then restored the state. The screen restored to the exact same state (content and cursor) that I had saved.

Then I changed mode from 25 to 50 rows, trashed the screen and restored state again. Also the second time the screen was restored perfectly including 25 line mode.

I guess it works because I'm working off the VGA state which does get saved/restored properly.

chschnell avatar Aug 25 '24 13:08 chschnell

FYI, I'm reworking the bus interface, and will try to get this merged afterwards.

copy avatar Aug 31 '24 23:08 copy

FYI, I'm reworking the bus interface, and will try to get this merged afterwards.

Ok!

The other day I made a thorough cross-check with qemu, and the text modes look and behave the same as in this PR.

So from my perspective it's complete and ready to be merged.

chschnell avatar Sep 01 '24 07:09 chschnell

just found out yesterday there's another repo named doswasmx, seemingly had solutions to a few issues here. But that is mainly aiming at gaming, I suppose.

bit-lang avatar Sep 02 '24 06:09 bit-lang

just found out yesterday there's another repo named doswasmx, seemingly had solutions to a few issues here. But that is mainly aiming at gaming, I suppose.

If I run a text mode application in DOS Wasm X which intensely dumps to stdout the screen gets quite unresponsive and kinda breaks (I ran nmake over the MS-DOS 4.0 source tree with a Pentium II at 300Mhz), see here:

dos-wasm-x-nmake

It's a bit hard to catch in a screenshot but take a look at the 3rd row from the bottom for what I mean.

A bit worse, if I run the Norton Commander installer in DOS Wasm X I don't see the font-based graphical mouse-cursor emulation (see the 3rd screebshot and text below here), and then Firefox crashes.

I only know a few simulators and thus don't have much of an opinion, but perhaps EM-DOSBox is a better wasm-port of DOSBox than "DOS Wasm X", EM-DOSBox runs all the simulations on archive.org, and those work quite well in my experience.

See also here: The Secret Feature of EM-DOSBOX on Internet Archive.

chschnell avatar Sep 02 '24 15:09 chschnell

just found out yesterday there's another repo named doswasmx, seemingly had solutions to a few issues here. But that is mainly aiming at gaming, I suppose.

If I run a text mode application in DOS Wasm X which intensely dumps to stdout the screen gets quite unresponsive and kinda breaks (I ran nmake over the MS-DOS 4.0 source tree with a Pentium II at 300Mhz), see here:

dos-wasm-x-nmake

It's a bit hard to catch in a screenshot but take a look at the 3rd row from the bottom for what I mean.

A bit worse, if I run the Norton Commander installer in DOS Wasm X I don't see the font-based graphical mouse-cursor emulation (see the 3rd screebshot and text below here), and then Firefox crashes.

I only know a few simulators and thus don't have much of an opinion, but perhaps Em-DOSBox is a better wasm-port of DOSBox than "DOS Wasm X", Em-DOSBox runs all the simulations on archive.org, and those work quite well in my experience.

Indeed Firefox crashes running doswasmx. try chrome. IMHO chrome works way better.

bit-lang avatar Sep 02 '24 15:09 bit-lang

Merged in c758d6da40c79d8ee648e00ff66ee66e44b33501 and f92e6b4b55c3f6e5a327b836375df20fedd45fa4. I meant to push to wip instead of master, but oh well.

There's still more refactoring I'd like to do, in particular:

  • Get rid of the if(this.graphical_text) this.graphical_text.foo() else this.screen.foo(), by merging graphical_text into ScreenAdapter
  • Pass the font data from VGAScreen to GraphicalText (and remove this.vga from GraphicalText)
  • Reusing the screen-put-char logic (e.g. storing the screen contents similar to ScreenAdapter
  • Move the invalidation logic out of redraw
  • Re-enable blinking with a proper timing (at the moment it blinks twice as fast on a 120hz screen)

However I have other things to work on. It would be nice if you could submit some of the refactorings as a PR, but it's good enough to be merged as is.

Could you do a quick test to see if everything is still working? I moved several things around, especially around the font size checks.

copy avatar Sep 22 '24 01:09 copy

Can you undo this merge?

chschnell avatar Sep 22 '24 06:09 chschnell

Else I would like to begin with the state of vga.js in the 2nd-to-last commit c758d6d and just override what was then merged in the last commit f92e6b4. It looks good to me, would that be safe?

I have a solid plan how to implement what you listed in one go, I'm only looking for a better starting position.

Compared to the previous state of vga.js, I only need to add 2 methods ScreenAdapter.set_font_bitmap() and ScreenAdapter.set_font_page() which get called by VGAScreen (let me know if I should expand on them) and make sure that the 6 VGA register-additions I made are good, these are the only changes to vga.js, very few.

For the rest, GraphicalText will get merged into ScreenAdapter, and ScreenAdapter won't need a link to VGAScreen. ScreenAdapter will be the only class with knowledge about graphical text mode. Graphical text mode implementation in ScreenAdapter will reuse as much as possible from classical text mode (all the methods (former bus messages), the character and attribute buffer, cursor attributes, etc.). All rendering (font and screen) will be implemented in ScreenAdapter.

chschnell avatar Sep 22 '24 14:09 chschnell

Can you undo this merge?

I can't, but feel free to include git revert f92e6b4b55c3f6e5a327b836375df20fedd45fa4 in your PR, then I'll merge (or squash) everything in one go.

copy avatar Sep 22 '24 18:09 copy

Ok I have started the 2nd iteration of this.

Please do not modify vga.js or screen.js in the next couple of days.

chschnell avatar Sep 22 '24 20:09 chschnell

Success!

I can see it booting as it's supposed to, so far I've tested: 50 row-mode, 8/9px font width and font pages A/B. Fontraption and Norton Commander look as they should.

So the main part is working, only minor things like the cursor are left to do.

chschnell avatar Sep 22 '24 22:09 chschnell

The redesign is complete, it's ready for review.

All previous features are implemented and tested:

  • 80 and 40 columns
  • 25, 28 and 50 rows
  • 16-color and monochrome mode
  • 8/9px font width
  • 16px font width (double font width)
  • 512-character set mode (font pages A/B)
  • blinking text and cursor emulation (now using clock-based interval timing instead of framecounter-based)
  • switching between graphical text mode and graphical mode (shared canvas)

I also made sure that the "classical" (DIV-based) text mode is still working.

chschnell avatar Sep 23 '24 17:09 chschnell

There is one issue, I'm trying to be as brief as possible but I must give a bit of context.

  • The text attribute byte is divided into two nibbles. The lower four bits are the foreground attributes (mostly color bits, see below), and the higher four are the background attributes.
  • If the screen-wide BLINK_ENABLE bit (bit 4 of the VGA Attribute Mode Control Register 10h) is set, the highest bit in the fourground nibble becomes the per-character BLINK attribute, reducing the available foreground colors from 16 to 8 for all characters on screen.
  • If the screen-wide PAGE_AB_ENABLE bit (Character Map Select Register Index 03h[^1]) is set, the highest bit in the background nibble becomes the per-character FONT_PAGE_SELECT attribute, reducing the available background colors from 16 to 8 for all characters on screen.

So in order for a character to blink two things must to be true at the same time, the same goes for a character that is to be selected from one of the two font pages A/B.

When VGAScreen calls ScreenAdapter.put_char() it uses the current value of the screen-wide bits (BLINK_ENABLE and PAGE_AB_ENABLE) to build a character's attributes and colors, and that is too early, this should be computed later whenever a character is drawn to screen (not when it's stored into a buffer as it is now). The applicaton is free in the order in which it writes characters/attributes to VGA memory and when it modifes the screen-wide bits.

I do not have a good solution for this (yet), maybe you have an idea. This PR does not handle this well yet.

EDIT:

In addition to the unmodifed, raw per-character attribute byte the render function for both text and graphical-text mode needs:

  1. current state of screen-wide BLINK_ENABLE bit
  2. current state of screen-wide PAGE_AB_ENABLE bit
  3. current VGAScreen.vga256_palette[]
  4. current value of VGAScreen.dac_mask
  5. current value of VGAScreen.dac_map

The current values of all these 5 variables must be present in the render function.

And that doesn't fit into the current design. Maybe add a callback argument to function ScreenAdapter.put_char() to retrive these from VGAScreen when needed?

EDIT 2:

Or add 5 new methods to ScreenAdapter which are called by VGAScreen whenever any of these values change:

  • ScreenAdapter.set_blink_enable(bool enable)
  • ScreenAdapter.set_page_ab_enable(bool enable)
  • ScreenAdapter.set_vga256_palette(Int32Array palette[256])
  • ScreenAdapter.set_dac_mask(int mask)
  • ScreenAdapter.set_dac_map(Uint8Array map[16])

Plus the unmodifed per-character attribute byte in ScreenAdapter.put_char().

Not very pretty, but in line with the design and this should solve it.

EDIT 3:

To clarify, in the PR I changed the semantics of the per-character BLINK integer to be a bitset of FLAG_BLINKING and/or FLAG_FONT_PAGE_B.

Currently, FLAG_BLINKING has the value of screen-wide BLINK_ENABLE bit baked into it when it is stored in ScreenAdapter's character buffer, and FLAG_FONT_PAGE_B doesn't, and I'm just uncertain which direction to go.

The most simple solution would be to treat the new FLAG_FONT_PAGE_B like the old FLAG_BLINKING (bake in on put_char()), but I'm not really comfortable with that. Should I just do that for now? Maybe I'm just paying too much attention to detail here.

[^1]: this "bit" is True when CHAR_SET_A_SELECT != CHAR_SET_B_SELECT

chschnell avatar Sep 24 '24 09:09 chschnell

The applicaton is free in the order in which it writes characters/attributes to VGA memory and when it modifes the screen-wide bits.

Presumably the palettes and screen-wide bits are not modified very often, would that be a fair assumption? If so, I'd suggest calling VGAScreen.prototype.complete_redraw when any of them change (which, under the hood, refills the entire screen with put_char calls).

copy avatar Sep 25 '24 17:09 copy

The applicaton is free in the order in which it writes characters/attributes to VGA memory and when it modifes the screen-wide bits.

Presumably the palettes and screen-wide bits are not modified very often, would that be a fair assumption? If so, I'd suggest calling VGAScreen.prototype.complete_redraw when any of them change (which, under the hood, refills the entire screen with put_char calls).

Now that's a really well-balanced proposition, thank you. This will also change the code only minimally.

I'll add another commit tomorrow.

chschnell avatar Sep 25 '24 18:09 chschnell

The second CI check for my last commit has now been stuck at Run api tests: Restoring: filesystem for more than 2 hours, and I'm not sure what to do about it.

Run make api-tests
./tests/api/clean-shutdown.js
Calling stop()
Called stop()
./tests/api/state.js
Saving: async cdrom
Restoring: async cdrom
Done: async cdrom
Saving: sync cdrom
Restoring: sync cdrom
Done: sync cdrom
Saving: filesystem
Restoring: filesystem

In code, it's stuck at tests/api/state.js in a call to emulator.restore_state().

I have not modifed the state[] of VGAScreen in this redesign PR.

chschnell avatar Sep 26 '24 17:09 chschnell

The second CI check for my last commit has now been stuck at Run api tests: Restoring: filesystem for more than 2 hours, and I'm not sure what to do about it.

It's a spurious failure, feel free to ignore it.

copy avatar Sep 27 '24 05:09 copy

@chschnell This is good to merge now, do you mind if I squash?

copy avatar Oct 06 '24 23:10 copy

@chschnell This is good to merge now, do you mind if I squash?

Yes, it's good to go.

chschnell avatar Oct 07 '24 03:10 chschnell

@chschnell Thanks for the contribution!

copy avatar Oct 07 '24 04:10 copy

You're welcome, and I'm really pleased with the outcome.

chschnell avatar Oct 07 '24 07:10 chschnell