search

openMSX / wxcatapult

23 stars 4 forks source link

Reversing a lot causes audio control panel to have 'obsolete widgets' #35

Closed MBilderbeek closed 8 years ago

MBilderbeek commented 8 years ago

I recently changed that the whole audio control panel gets reinitialized when machine events are received. This happens also when reversing. So lots of stuff is going on then.

When reversing slowly, everything seems to work fine. But if you keep the reverse key pressed, after a while, 'old/obsolete' slider widgets seem to become sticky in the audio control panel (volume sliders etc.).

Strange issue, as the code does seem to remove everything... I don't see how this can happen. Perhaps it's due to the asynchronous nature of the communication? So perhaps replies are still being processed and most importantly, new queries being made on which replies are processed) due to the machine changed event, while the next machine changed event is already coming in?

It's very easy to reproduce as I wrote above.

MBilderbeek commented 8 years ago

Can still be reproduced in heavy case scenarios: take the Boosted MSX2 EN (which has loads of audio channels) and do the same. You'll end up with a way too many channel sliders (I guess because of replies coming in after a new request for audio channels was sent already) or duplicated master volume sliders even while the rest has been destroyed.

I guess the best way to solve this issue would be to ignore all replies of old commands when we are going to do a relaunch (triggered by a machine event) which will requery all the data anyway, making the responses to the old queries useless.

MBilderbeek commented 8 years ago

The issue with the controls has been solved with commit 8a6c62af43b908c4bdfad9e936fe4839a37f70a9 But I noticed that CPU usage of Catapult goes up a lot when using reverse as described above (by keeping PgUp pressed in openMSX). CPU usage stays high even after this.

So, I bisected it and it's introduced in this commit:

730028bcdd68e975070d7bfa1e538be85c5a0f02 is the first bad commit commit 730028bcdd68e975070d7bfa1e538be85c5a0f02 Author: Manuel Bilderbeek Manuel.Bilderbeek@gmail.com Date: Fri Jul 24 21:35:19 2015 +0200

Update (reinit) parts of UI when hardware changes

Although it's doing a lot of work, Catapult can still quickly enough reinit
itself when stuff changes in the hardware. So if anything changes (e.g. someone
does a console or menu command in openMSX), Catapult will stay in sync.

:040000 040000 516ad7f90ec5b4dc72bf0fcbfde46fba48958812 9fc49582f73506b7cf12283d0f35548c62293ec4 M src

However, I don't understand (yet) why this causes the high CPU usage. Please help :)

MBilderbeek commented 8 years ago

Checked a bit more, I can 'fix' the issue if I comment out these lines from openMSXController.cpp:

        } else if (data->updateType == CatapultXMLParser::UPDATE_HARDWARE) {
            ExecuteStart(m_relaunch); // reinit stuff now */
        } else if (data->updateType == CatapultXMLParser::UPDATE_CONNECTOR) {
            ExecuteStart(m_relaunch); // reinit stuff now */

(UPDATE_EXTENSION can be kept in.)

Perhaps Catapult is getting into some communication loop, even though I thought it wasn't. EDIT: no, just checked, there really is no abnormal communication going on while Catapult is eating 100% in seemingly idle state. Just the query for the FPS, that's all.

MBilderbeek commented 8 years ago

Closing this issue, as the original problem is solved. Please see issue #36 for followup.