New SRAL_GetTTSEngines and SRAL_GetAssistiveTechEngines bit mask functions#36
New SRAL_GetTTSEngines and SRAL_GetAssistiveTechEngines bit mask functions#36JRJurman wants to merge 2 commits into
Conversation
JRJurman
left a comment
There was a problem hiding this comment.
Here are the smaller miscellaneous updates that don't have to do with the PR directly, but are still useful to have.
|
|
||
| - name: Configure CMake | ||
| run: cmake . -B build -DCMAKE_OSX_ARCHITECTURES=x86_64 -DBUILD_SHARED_LIBS=ON | ||
| run: cmake . -B build -DCMAKE_OSX_ARCHITECTURES="arm64;x86_64" -DBUILD_SHARED_LIBS=ON |
There was a problem hiding this comment.
noticed that we were missing the Apple Silicone build targets, so including that here as well
| AndroidTextToSpeechEngine | ||
| // AllEngines is a bitmask of all supported engines. | ||
| AllEngines Engine = NVDAEngine | JAWSEngine | ZDSREngine | NarratorEngine | UIAEngine | SAPIEngine | SpeechDispatcherEngine | NSSpeechEngine | VoiceOverEngine | AVSpeechEngine | ||
| AllEngines Engine = NVDAEngine | JAWSEngine | ZDSREngine | NarratorEngine | UIAEngine | SAPIEngine | SpeechDispatcherEngine | NSSpeechEngine | VoiceOverEngine | AVSpeechEngine | AndroidAccessibilityManagerEngine | AndroidTextToSpeechEngine |
There was a problem hiding this comment.
missing engines from the Android PRs 😅
| ANDROID_ACCESSIBILITY_MANAGER = 1 << 11 | ||
| ANDROID_TEXT_TO_SPEECH = 1 << 12 |
There was a problem hiding this comment.
same as above, missing engines from the Android PRs
| } | ||
| bool found = false; | ||
| for (int engine_val = SRAL_ENGINE_NVDA; engine_val <= SRAL_ENGINE_AV_SPEECH; engine_val <<= 1) { | ||
| for (int engine_val = SRAL_ENGINE_NVDA; engine_val <= SRAL_ENGINE_ANDROID_TEXT_TO_SPEECH; engine_val <<= 1) { |
There was a problem hiding this comment.
throughout this file we reference SRAL_ENGINE_AV_SPEECH as the last engine, so we change it to now be SRAL_ENGINE_ANDROID_TEXT_TO_SPEECH (although in retrospect, we probably should have a dedicated variable that semantically references this as last_engine_value, or something like that)
| extern "C" SRAL_API int SRAL_GetTTSEngines(void) { | ||
| return SRAL_ENGINE_SAPI | ||
| | SRAL_ENGINE_SPEECH_DISPATCHER | ||
| | SRAL_ENGINE_NS_SPEECH | ||
| | SRAL_ENGINE_AV_SPEECH | ||
| | SRAL_ENGINE_ANDROID_TEXT_TO_SPEECH; | ||
| } | ||
|
|
||
| extern "C" SRAL_API int SRAL_GetAssistiveTechEngines(void) { | ||
| return SRAL_ENGINE_NVDA | ||
| | SRAL_ENGINE_JAWS | ||
| | SRAL_ENGINE_ZDSR | ||
| | SRAL_ENGINE_NARRATOR | ||
| | SRAL_ENGINE_UIA | ||
| | SRAL_ENGINE_VOICE_OVER | ||
| | SRAL_ENGINE_ANDROID_ACCESSIBILITY_MANAGER; | ||
| } | ||
|
|
There was a problem hiding this comment.
Don't hardcode engines here. Instead, consider making engine feature or Sral::Engine interface method with engine category (either TTS or screen reader).
|
I can give you another idea on how to do this: enum SRAL_EngineCategory {
SRAL_ENGINE_CATEGORY_UNKNOWN = 0,
SRAL_ENGINE_CATEGORY_SCREEN_READER,
SRAL_ENGINE_CATEGORY_TEXT_TO_SPEECH_ENGINE,
SRAL_ENGINE_CATEGORY_ACCESSIBILITY_PROVIDER
};Then, for example, create this function SRAL_API SRAL_EngineCategory SRAL_GetEngineCategory(int engine);The static function Then declare and implement, for example Then, for example, engines like NVDA, JAWS, VoiceOver etc. will be categorized as screen readers, SAPI, Speech Dispatcher, AV etc. as TTS, UIA and Android AM will be a11y providers. Sorry for the mess, but I'm not working on this project right now and I won't be for a while. But I don't want to ignore or discard people's contributions. Thank you for that. |
|
No worries! I appreciate the ideas and feedback here. I'll probably spend some time either this weekend or later next week working through some options to see which comes out most elegant. But yeah, no need to rush on reviews when that is up, and I appreciate all the reviews and ideas you've given ❤️ |
Summary
Previously, it was not possible to exclusively route to Assistive Technology engines (Screen Readers, Braille devices) without already knowing the specific engine values. For most games / applications, we'd almost certainly want to default to ALWAYS reading out to those devices, with an in-app option to also read out to the operating system's Text to Speech engine (always reading out to these could be noisy for most non-blind consumers).
This PR introduces two new functions,
SRAL_GetTTSEnginesandSRAL_GetAssistiveTechEngines, which should make it easier for developers to specifically toggle which engines should be enabled.Snippet from the README:
Note
I'm not especially tied to either of these function names - I'm happy to update these if there are alternatives we think would be better
As part of this change, we also needed to update how the
speech_engine_updatefunction chooses an engine, and allow it to be null even if we previously had one selected. These changes are tested inSRALExample.c(see Testing below).Additionally, there are some small updates throughout (updating the build artifacts, updating the binding files to match latest engine list). If we'd rather those be in a separate PR, I'm happy to split this up. See the PR comments below for more details.
Testing
I validated this via the tests in
SRALExample.c. I've added checks that the engine masks are disjoint (they don't contain any overlapping engines). There's also a check that when we exclude all engines that the current engine is properly set as None.I also updated the macOS example app
SRALCocoaExample.mlocally, but I hesitated on including those changes here (since I think defaulting to TTS is probably better than having that behind a checkbox). If that would feel valuable, I'm happy to include that as part of this PR.