sarah-walker-pcem / arculator

Arculator
http://b-em.bbcmicro.com/arculator
GNU General Public License v2.0
56 stars 23 forks source link

CPU dropdown gets huge on "8MHz RAM" models (A3000 or lesser) - only #41

Open IJeffray opened 10 months ago

IJeffray commented 10 months ago

image

For A3000 or 'lesser' models, selecting a CPU type causes the box to embiggen and stay that way until the window is closed.

Repeated with Arculator 2.2 on Windows 10.

Sophira commented 10 months ago

I've been able to also reproduce this on Windows 10 with Arculator 2.2. It does not occur on Linux with Arculator 2.2, or on Arculator 2.1 on either OS.

Steps I used to reproduce:

  1. Start Arculator.
  2. Create a machine, name it "Test" (probably doesn't matter).
  3. Choose "Archimedes 440/1", click OK.
  4. Click on the CPU dropdown, and select anything other than "ARM2" (the default).
  5. The CPU dropdown gets huge, as per the image in the original post.

Notably, this does not happen if you just use the keyboard to select a CPU by tabbing to the dropdown and using the up/down keys to select a CPU; you have to actually expand the dropdown to show all the choices. (Though using the keyboard to select a choice after expanding it will still trigger the glitch.)

Sophira commented 10 months ago

So I've been spending some time bisecting this - which took longer than expected as many of the commits I needed to test did not build under Windows and I had to search out the correct patches from the rest of the code, so apologies for the delay! But with these problems patched out, it turns out that the problem first started appearing in commit 10bb3bbe, which added options for more overclocked ARM3s.

As far as I can tell, though, all this commit seems to do is just that - add more options! Which likely means that the problem was actually always there and it just didn't manifest itself when there weren't as many options in the CPU dropdown.

For anybody who wants to try to debug this further, the patches I needed to apply in order to get this commit to compile and run on Windows (using autotools, since I don't know what the other method of compiling at the time would have been) were:

Sophira commented 10 months ago

I've been looking further into this. The problem seems to specifically occur on the ninth cbox->Append call (src/wx-config.cc:637). If I change the for loop to go to nr_elems(cpu_names) - 1 instead, the problem doesn't occur - but then of course the last choice doesn't show up, either.

This makes it very clear as to why the problem only occurs with some machine types - the newer machines don't have an "ARM2" option available, so there's one less option in the CPU dropdown. Since the issue only seems to occur on the ninth Append call, it turns out this is enough to stop the problem from occurring when configuring those machines.

While I haven't yet managed to solve this issue, I did stumble into another bug while attempting to debug this further. (Which I had actually already fixed a while ago locally while working on #26 but hadn't yet submitted into a non-draft pull request.) It seems that wx-resources.cc is not regenerated on compile due to a typo in the Makefile.am file. I'll make a pull request [edit: #42] that fixes the typo and removes wx-resources.cc from the distribution, but unfortunately it doesn't automatically fix this bug like I was hoping it would do.