Open ZornsLemma opened 4 years ago
Ah, I think I've got it - if I minimise the b-em window while b-em is paused then immediately un-minimise it, I get this behaviour reliably.
What you have said makes perfect sense. Traditionally, applications that only update their window contents fairly slowly would response to "expose" events where the windowing system advises the application that something else has drawn over part of its window and now the application needs to redraw that.
B-Em doesn't work that way - because of the way it emulates the BBC micro video system it is redrawing its window frequently anyway except, of course, when it is paused. So the solution would be for B-Em to respond to expose events when paused and may be as simple as calling the video_doblit function.
On the other hand, I can't reproduce it, at least with my normal desktop. I can cover the B-Em window, either fully or partially, I can minimise it and restore it, and it still retains its contents. I am running Arch (rolling release) and GNOME 3.36 in Wayland.
Thanks for getting back to me, it's a shame you can't reproduce it but I guess that's often the way with these video problems. I've tried to implement your suggestion on a new branch here: https://github.com/ZornsLemma/b-em/tree/pause-redraw In the five minutes of testing it's had so far it seems to work, but I don't know if it will cause other problems or fail to fix things - I'll try to keep using this and see how it goes...
PS I don't check to see if b-em is paused before doing anything in response to an expose event, but experimentally it seems the expose event isn't generated most of the time anyway, so I don't think this is a problem. It might be better to check for being paused anyway, I am not sure - the Allegro/b-em video model isn't something I'm that familiar with.
Looking at what you;ve done I can't forsee a problem and it runs OK on my system.
The hazards I could think of are that video_doblit may not be re-entrant, that the bitmap used in video_poll should not be written to while it is being blitted to the real screen and that Allegro sometimes seems to care which thread does something but I don't think any of these issues would arise.
Normally, video_doblit is called from video_poll which is called from polltime in the 6502 emulation. That is called by m6502_exec which is called from main_timer which is called from the event dispatch loop that you have added expose events to so we can be sure control is not in any of those functions when you call video_doblit in response to an expose event.
Now that I had more time I set about trying to recreate the issue on the unpatched version of b-em. My usual desktop is GNOME on Wayland. Trying GNOME on X11 didn't reproduce it so it seems the difference is not X11 vs. Wayland. Then I tried Cinnamon desktop - still no luck, but I could reproduce it on MATE.
What I suspect is the difference is whether the desktop is composited. Assuming I understand this correctly, this essentially inserts another level of indirection between applications and the actual screen where the window manager/compositor can apply further transformations to the contents of a window before it reaches the real screen. At a guess that also means the compositor can redraw a window onto the main screen by re-blitting the bitmap it got the application to draw into instead of the real screen and therefore does not need to rely on the application redrawing.
Using MATE as the test environment, your change fixes the issue you observed. There is a related issue, though. When the window is maximised when paused it doesn't expand to fill the available space. The bit that is filled changes scaling but the bit that was not previously drawn doesn't get filled in. I am not sure if I am explaining that very well but see if it does the same for you.
Thanks for taking a look at this and going to such lengths to reproduce it! I'm glad you were able to see it happening. I am using MATE, for what it's worth; sorry for not mentioning that before, I sort of forget I installed it.
You're right, I can see the maximised behaviour you describe. If I use Master Turbo mode (which has a nice long line in the startup banner) with *CONFIGURE MODE 7, when I maximise the window the startup banner gets cropped off after "TUBE 65C". Resizing the window by dragging the window frame does seem to work though, and this also tweaks the area which gets used for the maximised window.
If you have a fix in mind please let me know, otherwise I will have a poke at this later and see if I can spot anything.
The maximise issue seems to be another case of double-buffering strangeness. If I change the video_doblit2 function to call the real video_doblit twice:
void video_doblit2()
{
video_doblit(crtc_mode, crtc[4]);
video_doblit(crtc_mode, crtc[4]);
}
It starts to intermittently do the right thing. Conceptually, in double-buffering there should be a front buffer (visible) and a back buffer (being drawn into) and a flip operation should swap them. With that conceptual model it should only be necessary to draw static things twice, once for each buffer, then they should always be visible. But with Allegro this doesn't seem to be that simple as you found with the LEDs. I don't know if this is because al_flip_buffer sometimes ends up flipping twice so the 2nd draw operation ends up drawing into the same buffer as the first one or if it sometimes has more than two buffers.
I can't see an easy solution as Allegro doesn't seem to give any hint about this. The fact that one has to specifically request expose events means the designer(s) clearly had it in mind that display updates would be continuous which is typically the case for games, which is what I think Allegro is primarily aimed at.
We could work around this by calling video_doblit on a timer when b-em is paused. For me, though, one of the reasons I pause b-em is if I am working on something else and know I won't be using it for a while and want the CPU for something else so having it remain busy when nominally idle doesn't fit with that. I suppose if the timed event was not too frequent that may work, though.
Like you, I pause b-em when I want the CPU use to go down (and I have been toying with trying to implement a "pause when not visible" option like that in BeebEm) but as you say it may be fine if the timed event is not too frequent.
I've had a crack at implementing this: https://github.com/ZornsLemma/b-em/tree/pause-redraw-2
It superficially works (extremely lightly tested) but I notice that in mode 7, if you do "REPEAT:PRINT TIME:UNTIL FALSE" and then pause it, it seems to freeze the display in a "mixed interlace" sort of way. It also seems a little delayed in responding to the pause option being selected. On my machine top shows the CPU use drop from 30% when running at normal speed to 7% when paused, which is better than nothing but is a bit disappointing.
I don't know. I hope I've just made a really bad job of implementing this but it doesn't feel as workable an approach as I'd hoped.
On the plus side it does seem to fix the maximise problem at least. I'll keep using this version over the next few days and see how I get on with it anyway.
On the 7% CPU usage when paused, I compared this with my keypad version, which the LEDs branch with the keypad patch and doesn't redraw the screen when paused and the CPU usage there doesn't seem to be much different. So although B-Em is paused it looks like Allegro is doing something in the background.
It may be sound-related. With the Music 5000, B-Em is responding to events so each time the audio subsystem runs out of something to play it sends and event and the Music 5000 generates something and there is no intelligence to stop that of the device is idle because the real hardware doesn't have and idle state either.
We can't see that from within the B-Em code for other sounds - it looks like we push the sounds when we have something to play but there may still be events in the background and Allegro maybe pushing silence to the hardware whenever we don't send it anything. Not that I have investigated this, just speculating.
One thing I noticed about your change is that you've changed from a fixed interval for the timer, with the amount of work being done in that interval hopefully varying according to the speed setting, to a variable timer interval, hopefully with the amount of work done per event fixed. I thought that was worth a test to see how the speeds worked out. I ran CLOCKSP but it's no good looking at the reported speed as that uses a 6522 which slows and speeds up with the processor so I timed it on my phone:
25% 3m17
50% 1m38
100% 0m49.3s
150% 0m33s
200% 0m25s
So this seems to be working fine.
One thing I did not mention is that I wasn't sure how well adjusting the time interval between timer events would be at faster speeds because we don't know what resolution the hardware timer is behind it or whether the OS puts any restrictions on top of that, but the test above seems fine on my PC.
Good thinking on the Music 5000 - if I disable it from the menus, the CPU use when paused (very roughly; my system isn't idle at the moment) halves, down to maybe 3%.
I wasn't aware I had changed to a variable timer interval! Except when paused, I would have expected the behaviour to be identical. What have I missed? I'm glad it seems to be working all the same, but I think my understanding of the code might be even worse than I thought it was. ;-)
And after behaving all day, right now my b-em window (with this change) has done the no-redraw-when-paused thing. I have no idea why, but I wanted to mention it...
On the time interval, forget what I said earlier. I spotted a change in main_init where the timer had previously been set to a fixed value of 1.0 / 50.0 and you changed it to refer to the table of speeds. What I didn't spot was that there was no similar change to main_setspeed because that had previously referred to the table of speeds so it was already using a different timer value for each speed and, in my last post, I was talking out of my hat.
That setting in main_init to the fixed speed probably looked like a bug but there is a strange inconsistency in that the tube speed is saved to the config file but the main speed is not so it always starts at 1x, hence why that has not bit us previously.
On the no re-draw while paused, how did you pause it? It looks like there are two separate pause mechanisms - selecting a speed of paused via the speed menu and the Page Down key. The latter has a separate toggle variable - it does not change emu_speed because the key acts as a toggle and needs to carry on at the old speed immediately after being unpaused.
As the logic around pausing and resuming gets more complex, though, it may be better if that simply saved the old speed value and called main_setspeed and then, when unpaused, called main_setspeed with the saved value. Then the logic is together in one place.
I always use the pause menu - I wasn't really aware of the Page Down option. I can't rule out the possibility I accidentally pressed Page Down without realising it when that misbehaviour occurred though.
With regard to the fixed speed in main_init, I didn't actually think it was inconsistent, I just saw the opportunity to replace the hard-coded 1.0/50.0 value with the same value from the emuspeed array. It didn't occur to me to wonder why this wasn't respecting the config file. :-)
I will have a look at tweaking the pause logic as you describe.
OK, I've pushed a change which seems to work - I don't think it fixes all the issues (the slightly sluggish response to pausing, the fact mode 7 and maybe other modes can pause with the screen in a "mid-update" state, the higher-than-ideal CPU use) but it does rationalise the pause handling so the menu and Page Down act together, including updating the Pause menu when Page Down is used.
There's also a separate commit on this branch for an unrelated change to stop re-selecting the current model from the model menu losing the tick. If you're happy with this, would you please consider cherry-picking this commit elsewhere so it doesn't get lost if this pause work goes nowhere?
Incidentally even without this recent change, I seem to get the odd "." typed in to the emulated machine when pressing Page Down. I haven't tried to investigate this yet, does it happen for you (with any recent-ish build, not just this branch)?
I also just noticed that making the window full-screen while paused doesn't rescale it.
Ok, I have cherry-picked the model change n-op commit to stardot/master.
I have not managed to reproduce the extra '.' on hitting PageDown but can see the failure to expand to full-screen when paused.
Regarding the mode 7 display being paused in a half-drawn state, as far as I can see this is the result of interlace. With interlace enabled, the 6845 emulation, in video.c, draws into the even lines of bitmap 'b' on one pass and the odd lines the next pass but whenever we blit that bitmap onto the visible screen we always do all of it. That means when the screen is moving, at any instant, one field of the interlaced frame can be ahead of the other.
This won't be an exact match for how the picture would have been on an old CRT as that would depend on the persistence of the phosphor and we don`t emulate a CRT - there is no fading applied to the older of the two fields.
As to why mode 7 - on the real hardware it is impossible to disable interface on mode 7 and B-em copies that. In video.c there is a small integer variable, crtc_mode which is 0 for mode 7, 1 for high frequency modes (0-3) and 1 for low-frequency modes (4-6). With that in mind check:
void video_set_disptype(enum vid_disptype dtype)
{
vid_dtype_user = dtype;
set_intern_dtype(dtype);
}
then
static void set_intern_dtype(enum vid_disptype dtype)
{
if (crtc_mode == 0 && (crtc[8] & 1))
dtype = VDT_INTERLACE;
else if (dtype == VDT_INTERLACE && !(crtc[8] & 1))
dtype = VDT_SCALE;
vid_dtype_intern = dtype;
}
Thanks for taking that commit to master.
I was vaguely aware of this interlace behaviour, but I thought it causing the strange visual appearance on pause was an artefact of this new pause approach. However, I just tried on master (not the latest, but fairly recent) and the same thing happens there with the REPEAT:P.TIME:UN.FA. test, so I guess it's no worse.
Can you please try the "page down gives spurious '.' character" test again in Master 128 mode? I can't reproduce it on an emulated model B but I can in Master 128 mode, which is what I normally use. I might guess the problem is something to do with the numeric keypad "." key on the Master, although I haven't investigated this further yet.
Ok, with Master 128 selected I reliably get the '.' both on your branch and on one of mine much more closely tied to master (and without these redraw during pause changes). I'll check out the keymapping on the master.
Yes, it looks like the PageDown key is mapped to the '.' on the Master keypad as well as having the pause function so on a Master it does both. Here's the complete keyboard map for that keypad:
Key Code Allegro
--- ---- -------
+ 3a ALLEGRO_KEY_PAD_PLUS
- 3b ALLEGRO_KEY_PAD_MINUS
/ 4a ALLEGRO_KEY_PAD_SLASH
* 5b ALLEGRO_KEY_PAD_ASTERISK
7 1b ALLEGRO_KEY_PAD_7
8 2a ALLEGRO_KEY_PAD_8
9 2b ALLEGRO_KEY_PAD_9
# 5a (not mapped)
4 7a ALLEGRO_KEY_PAD_4
5 7b ALLEGRO_KEY_PAD_5
6 1a ALLEGRO_KEY_PAD_6
DEL 4b (not mapped)
1 6b ALLEGRO_KEY_PAD_1
2 7c ALLEGRO_KEY_PAD_2
3 6c ALLEGRO_KEY_PAD_3
, 5c ALLEGRO_KEY_HOME
0 6a ALLEGRO_KEY_PAD_0
. 4c ALLEGRO_KEY_PGDN
Rtn 3c ALLEGRO_KEY_PAD_ENTER
so this keypad is a bit of a problem as it has more keys than the numeric keypad on a PC keyboard so no mapping of it will ever make equal sense to everyone. It would be good, therefore, if people could choose their own mapping but, at the moment, the user defined key-mapping maps one Allegro code to another Allegro code with the default mapping being identity, then that is mapped separately to a BBC row/col code.
That means where there is no Allegro code assigned to a key, as is the case with # and Del, the user-defined key-mapping mechanism cannot be used to choose one. This could be bodged by using an Allegro keycode that is not present on a PC keyboard for the unmapped positions - maybe that's what we should also do with '.' as an interim.
But I think this really needs to be re-worked so perhaps it's time to integrate your logical keyboard work which also has a bearing on key-mapping. I was trying to do one thing at a time, so LEDs first, logical keyboard second, but perhaps the LEDs work is mature now, could be merged, and the logical keyboard could be next.
it may be worth checking out https://github.com/stardot/b-em/tree/sf/autopause
I've been giving this a try over the last few days. On the whole it works really well - I left a b-em window open but ignored as I used the machine for other things and it clocked up about 20 minutes of emulated runtime.
For what little it's probably worth - I don't have any easy recipes to reproduce this, and it is on a VirtualBox guest (running Linux) - I have seen the b-em window exhibit the "correct window contents are not repainted and some random other content appears there" behaviour discussed above (not that commonly; mostly it does repaint fine), and I think I have seen the window frozen but still showing an (unchanging) "MHz" display in the title bar instead of the "(auto-paused)" message.
Hi,
I don't know if this happens on Windows, but on Linux (Ubuntu 18.04.4 LTS) if I choose Speed->Paused and leave the window obscured for a while, the next time it comes to the foreground it shows a semi-random chunk of some other window instead of the emulated screen. (This doesn't happen immediately, unfortunately.) It would be nice if the window continued to show the emulated screen whatever happens.
Sorry this is a little bit vague, but it does happen pretty reliably for me if the window is left paused for a while.
Cheers.
Steve