Skip to content

ef9345: fix insert and cursor rendering logic #13631

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fabio-d
Copy link
Contributor

@fabio-d fabio-d commented Apr 28, 2025

Hello, this PR rewrites how the "insert" attribute is processed to solve the issue discussed in jfdelnero/minitel#3 (comment) and briefly mentioned in #13200.

TL;DR: MAME's current handling of the background color is wrong for the TS9347 video chip variant, and the "insert" attribute is handled wrong on EF9345 too. In addition, the hardware cursor should blink twice as fast as flashing text, instead of the same rate.

Longer recap from that discussion:

  • EF9345/TS9347 have a per-character attribute called "insert" that, in conjunction with the PAT register, can be used to force colors to black in some conditions.
  • The minitel2's demov1 bios rendered correctly in MAME, but ft_bv4 and ft_bv9 display very noticeable glitches in many Videotex pages, such as those in the screenshots here:

    instead of
  • It turned out that MAME currently contains wrong code that always forces the background of all characters rendered by the minitel2 machine to black, regardless of insert, PAT or the character's background color.
    if (m_variant == EF9345_MODE::TYPE_TS9347)
    {
    c0 = 0;
    }
  • Just removing those lines made ft_bv4 and ft_bv9 render correctly but broke demov1, because MAME's current support for "insert" (on which demov1 relies in order to set black backgrounds) is wrong/incomplete
    if ((m_pat & 0x30) == 0x30)
    insert = 0; //active area mark
    if (insert == 0)
    c1 += 8; //foreground color
    if ((m_pat & 0x30) == 0x00)
    insert = 1; //insert mode
    if (insert == 0)
    c0 += 8; //background color

@jfdelnero and I initially made some experiments over the Internet on a real Minitel 2 to better understand how the PAT register works. I've then created a custom "development" board hosting real EF9345/TS9347 chips I bought on eBay. Thanks to this board I've written automatic tests to exhaustively capture the output of all the combinations of PAT and insert attribute values and, in addition, the interactions with the hardware cursor too.

This PR:

  • Removes the wrong "background is always black" code block.
  • Properly implements the insert attribute under all the possible PAT values (pixel-perfect validated against the output of the real chip).
  • Fixes the timing of the hardware cursor which, when positioned on flashing text, shows that it alternates between 4 phases instead of 2:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant