Added a checkbox-based Stops dialog (Audio/Midi -> Stops) https://github.com/GrandOrgue/grandorgue/issues/1816

[oleg68]

Resolves: #1816

This PR adds a GOStopsDialog class for using with screen readers.

It is accessibly with Audio/Midi -> Stops.

[willeke1234]

From the user's point of view, this window is a panel and has nothing to do with audio or MIDI. Shouldn't it be in the Panel menu?

[oleg68]

From the user's point of view, this window is a panel and has nothing to do with audio or MIDI. Shouldn't it be in the Panel menu?

@larspalo @rousseldenis What is your opinion? Should this window be launched with Panel?

Some time ago, there were a discussion of reorganising the main menu structure. I propose put this window anywhere now in this PR and then continue discussing the menu structure in a separate thread.

[oleg68]

From the user's point of view, this window is a panel and has nothing to do with audio or MIDI. Shouldn't it be in the Panel menu?

@larspalo @rousseldenis What is your opinion? Should this window be launched with Panel?

Some time ago, there were a discussion of reorganising the main menu structure. I propose put this window anywhere now in this PR and then continue discussing the menu structure in a separate thread.

he screenreader on my Ubuntu 20.04 system (Orca) fail to provide any relevant feedback in order to use it whithout visual clues.

I'd commit this change as is without waiting for feedback from @lewisalexander2020. He wrote that he did not have any problems with Settings dialogs. Because this new window uses the same controls, I hope he would be able to use it. We can change something after a feedback.

[willeke1234]

If you want, I can test it on macOS with VoiceOver.

[larspalo]

Should this window be launched with Panel?

It might make sense for the user to have it in the Panel menu. But it's of course less important where it's placed at the moment.

Another possible improvement that I could suggest of the current PR would be to honor the switch references on the manuals and list them under each manual if that switch only is referenced on that single manual (instead of lumping all of them together in the global column).

[oleg68]

Another possible improvement that I could suggest of the current PR would be to honor the switch references on the manuals and list them under each manual if that switch only is referenced on that single manual (instead of lumping all of them together in the global column).

I hope that now GO works so. If not, could you give me an example of ODF, where the panel does not respect the switch-manual relationship?

[oleg68]

If you want, I can test it on macOS with VoiceOver.

It would be great!

[larspalo]

I hope that now GO works so.

Yes, it indeed does! My mistake to not check some corner cases where it seemingly doesn't. Sorry.

It does fail with Norrfjärden Church as the same switches are actually listed for both full compass manual versions and original manuals. Thus the switches are no longer unique to a single manual... That's understandable.

It also fail with Bygdsiljum Church, but that's because the package version doesn't include any switches in the manuals whereas in my un-packed working/improvement version I've added them correctly - and when opening that particular version, the switches are indeed listed correctly.

[willeke1234]

It looks like the Stops window contains the send-only MIDI objects. Is this intended?

In the Stops window of the demo organ is a Motor noise checkbox in the Pedal section. The checkbox follows the Motor noise button on the panel but not the other way around.

[willeke1234]

GO crashes (not always) when I opened the Stops window and quit. Crashreport.txt.zip

[oleg68]

GO crashes (not always) when I opened the Stops window and quit. Crashreport.txt.zip

Could you post here more crash dumps. Are they all with the same stacktrace?

[oleg68]

It looks like the Stops window contains the send-only MIDI objects. Is this intended?

In the Stops window of the demo organ is a Motor noise checkbox in the Pedal section. The checkbox follows the Motor noise button on the panel but not the other way around.

The demo organ has a switch network that is not yet compatible with the stop panel. Could you test with another organ?

[willeke1234]

Could you test with another organ?

The compatible organs I tried: Wildervank. The not yet compatible organs I tried: Friesach, Barton, Chapelet, Kassel-Niederzwehren, Kdousov.

Edit: Barton is almost compatible, the toys and bell shouldn't be visible.

[willeke1234]

More crash reports, they look all the same to me. GO crashes when I quit or load another organ. I tried several organs. Crashes.zip

[willeke1234]

The size and scroll bars of the Stops window may be a bit off initially:

[willeke1234]

Voice Over works, although it has its quirks. It pronounces everything in English and Barton's "Great to Solo Oct" is pronounced as "Great to solo october".

[oleg68]

@willeke1234 Could you test the new build https://github.com/oleg68/GrandOrgue-official/actions/runs/9922125693

• I fixed an issue of compatibility with certain organs. The reason was that non-input switches appeared in the Stop Panel
• I could not reproduce the crash. According to the stacktrace, the crash occured in the GOStopsDialog destructor when it called

  r_model.UnRegisterControlChangedHandler(this);

It looks like r_model refers to nullptr, but in this case the crash would occur on the constructor

r_model.RegisterControlChangedHandler(this);

The onle reason why r_model might be zeroed is the destructor called twice. Earlier I experienced such problem especially in MacOs but I don't remember the details.

Does the crash is reproduced with the new build? If so, are you ready to test builds with more debug printing?

[willeke1234]

@oleg68 I think r_model is a dangling pointer. When I reload the organ, GOOrganController::~GOOrganController() is called before the progress window and GOStopsDialog::~GOStopsDialog() is called after the progress window.

[oleg68]

@willeke1234 Does the crash appear when you are just closing the Stop Panel without reloading an organ?

[willeke1234]

@oleg68 GO crashes when the Stops window is open and I choose Quit, Reload, Close or load another organ. It looks like r_model is deallocated before GOStopsDialog::~GOStopsDialog() is called. If I close the Stops window before closing the organ then GO doesn't crash.

[oleg68]

@willeke1234 I tried to fix the crash. Could you test https://github.com/oleg68/GrandOrgue-official/actions/runs/9963122909 ?

[willeke1234]

@oleg68 The crash is gone.

[oleg68]

@willeke1234

It looks like the Stops window contains the send-only MIDI objects. Is this intended?

In the Stops window of the demo organ is a Motor noise checkbox in the Pedal section. The checkbox follows the Motor noise button on the panel but not the other way around.

Is this issue still present with the last build?

[willeke1234]

@oleg68 The motor noise checkbox is no longer there.

[oleg68]

@oleg68 The motor noise checkbox is no longer there.

Because it has StoreInGeneral=N. Only the switches are stored in generals are present on the Stop panel.

[oleg68]

@rousseldenis Could you approve this PR?