jcsteh / osara

OSARA: Open Source Accessibility for the REAPER Application
GNU General Public License v2.0
121 stars 45 forks source link

Report the cursor position after dismissing the Jump to Time dialog. #1045

Closed jcsteh closed 1 month ago

jcsteh commented 1 month ago

Fixes #1041.

@ScottChesworth, is this what you're after?

AppVeyorBot commented 1 month ago

Build succeeded! Build osara pr1045-1383,1e86f40a completed (commit https://github.com/jcsteh/osara/commit/1e86f40aae by @jcsteh) Downloads:

Windows - [installer/osara_pr1045-1383,1e86f40a.exe](https://ci.appveyor.com/api/buildjobs/fq31wcyiht3md4ol/artifacts/installer/osara_pr1045-1383,1e86f40a.exe) - [locale/osara_pr1045-1383,1e86f40a.pot](https://ci.appveyor.com/api/buildjobs/fq31wcyiht3md4ol/artifacts/locale/osara_pr1045-1383,1e86f40a.pot)
Mac - [installer/osara_pr1045-1383,1e86f40a.dmg](https://ci.appveyor.com/api/buildjobs/ig6e25qytgl926d3/artifacts/installer/osara_pr1045-1383,1e86f40a.dmg)
ScottChesworth commented 1 month ago

Hot diggity dawg this is excellent! I'll get it checked on Mac before merging. Thanks!

pitermach commented 1 month ago

Just checked on Mac OS 14.4.1, and the message does speak, however most of the time gets interrupted by VoiceOver reading the focus moving back to the main Reaper Window, usually announcing the "new project" button.

On 5 Apr 2024, at 15:10, ScottChesworth @.***> wrote:

Hot diggity dawg this is excellent! I'll get it checked on Mac before merging. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/jcsteh/osara/pull/1045#issuecomment-2039768779, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAABTMKJLIMU56J4UKJBL3Y32PEVAVCNFSM6AAAAABFY3ITASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZZG43DQNZXHE. You are receiving this because you are subscribed to this thread.

ScottChesworth commented 1 month ago

Is there a low effort way to experiment with a short delay before the report @jcsteh? Only way I know to do that is sleep for an amount of ms, I'm guessing you'll have a more elegant way?

jcsteh commented 1 month ago

Sleeping would block the entire UI thread, so that's not an option. We can set a timer instead. However, I think this is risky and brittle and it leads me to question whether we should drop this idea. A timer is likely to be unreliable across different systems. Also, it might interrupt legitimate speech triggered by the user; e.g. if they quickly pressed a command after dismissing the jump dialog.

ScottChesworth commented 1 month ago

Hmm, tricky. I can't find any obvious way to disrupt it from working on Windows and it's so nice and I really, really want to keep it lol. Can we try a short timer, say 150ms just to see whether that shores things up for the Mac folk? FYI we've already got Piotr (who has a decently powerful modern Mac) and Jen (who has a thing that's almost steam punk) here to test the result.

ScottChesworth commented 1 month ago

Another thought/question, I've already had Jen verify that on the same Mac, her report when choosing to go to a track from the More context menu in Params dialog was consistent. What's the difference? Is it that the window redraws when the jump dialog closes, but it doesn't when our params dialog gets destroyed?

jcsteh commented 1 month ago

I honestly have no idea why it's different.

jcsteh commented 1 month ago

Let's start with 50 ms. Also, if we go down this path, I'll want to rewrite this code properly, but this will do for testing.

AppVeyorBot commented 1 month ago

Build succeeded! Build osara pr1045-1385,f64f2fad completed (commit https://github.com/jcsteh/osara/commit/f64f2fad69 by @jcsteh) Downloads:

Windows - [installer/osara_pr1045-1385,f64f2fad.exe](https://ci.appveyor.com/api/buildjobs/vgyrbsjo3xvxhlmq/artifacts/installer/osara_pr1045-1385,f64f2fad.exe) - [locale/osara_pr1045-1385,f64f2fad.pot](https://ci.appveyor.com/api/buildjobs/vgyrbsjo3xvxhlmq/artifacts/locale/osara_pr1045-1385,f64f2fad.pot)
Mac - [installer/osara_pr1045-1385,f64f2fad.dmg](https://ci.appveyor.com/api/buildjobs/5qs3mwrny553622s/artifacts/installer/osara_pr1045-1385,f64f2fad.dmg)
jennykbrennan commented 1 month ago

Right. So now it works more than it fails. And it seems like it was the 21st track that made it fail in this particular project with empty tracks. So yep, this is way better, not a hundred percent but close in this case

AppVeyorBot commented 1 month ago

Build succeeded! Build osara pr1045-1386,528d7b00 completed (commit https://github.com/jcsteh/osara/commit/528d7b00cd by @jcsteh) Downloads:

Windows - [installer/osara_pr1045-1386,528d7b00.exe](https://ci.appveyor.com/api/buildjobs/yi52thl5ok2nm7wr/artifacts/installer/osara_pr1045-1386,528d7b00.exe) - [locale/osara_pr1045-1386,528d7b00.pot](https://ci.appveyor.com/api/buildjobs/yi52thl5ok2nm7wr/artifacts/locale/osara_pr1045-1386,528d7b00.pot)
Mac - [installer/osara_pr1045-1386,528d7b00.dmg](https://ci.appveyor.com/api/buildjobs/rrdp9pnfgpi1097u/artifacts/installer/osara_pr1045-1386,528d7b00.dmg)
jcsteh commented 1 month ago

Thanks for testing. The fact that you can get it to fail at all in a mostly empty project does not bode well for larger projects or higher resource usage. :(

AppVeyorBot commented 1 month ago

Build succeeded! Build osara pr1045-1387,cbff46a0 completed (commit https://github.com/jcsteh/osara/commit/cbff46a083 by @jcsteh) Downloads:

Windows - [installer/osara_pr1045-1387,cbff46a0.exe](https://ci.appveyor.com/api/buildjobs/rjm0xw3ovd7t2lqs/artifacts/installer/osara_pr1045-1387,cbff46a0.exe) - [locale/osara_pr1045-1387,cbff46a0.pot](https://ci.appveyor.com/api/buildjobs/rjm0xw3ovd7t2lqs/artifacts/locale/osara_pr1045-1387,cbff46a0.pot)
Mac - [installer/osara_pr1045-1387,cbff46a0.dmg](https://ci.appveyor.com/api/buildjobs/8uk1xd026n0fof45/artifacts/installer/osara_pr1045-1387,cbff46a0.dmg)
pitermach commented 1 month ago

Fwiw, I just got this new version and tried to stress test it by having 40 tracks of a very resource intensive plugin which on-top of things was also running under Rosetta, and while everything else started lagging, I couldn't get the time announcement to break and I tried at least 10 times. Perhaps it would be different on an older system or if this project had a lot of items on it, but either way after this update things are much more consistent on my end.

On 6 Apr 2024, at 05:01, AppVeyorBot @.***> wrote:

Build succeeded! Build osara pr1045-1387,cbff46a0 completed https://ci.appveyor.com/project/jcsteh/osara/builds/49558758 (commit cbff46a083 https://github.com/jcsteh/osara/commit/cbff46a083 by @jcsteh https://github.com/jcsteh) Downloads:

Windows https://ci.appveyor.com/api/buildjobs/rjm0xw3ovd7t2lqs/artifacts/installer/osara_pr1045-1387,cbff46a0.exe https://ci.appveyor.com/api/buildjobs/rjm0xw3ovd7t2lqs/artifacts/locale/osara_pr1045-1387,cbff46a0.pot Mac https://ci.appveyor.com/api/buildjobs/8uk1xd026n0fof45/artifacts/installer/osara_pr1045-1387,cbff46a0.dmg — Reply to this email directly, view it on GitHub https://github.com/jcsteh/osara/pull/1045#issuecomment-2040910065, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAABTLRXT6OFJIIO5AWACLY35QPVAVCNFSM6AAAAABFY3ITASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBQHEYTAMBWGU. You are receiving this because you commented.

ScottChesworth commented 1 month ago

Solid here on Windows. Re Mac support, so long as it's working the majority of the time, and there aren't any actual regressions being caused when it fails, I reckon we could go with that. There are already weirder things about Mac out in the wild.

jennykbrennan commented 1 month ago

Yep, stress test here and no fails on an older iMac. There's one note to take here though when dealing with the mac. @ScottChesworth. We do need to recommend keeping cursor tracking off and/or make sure mouse cursor do not speak after delay. It should be a standard recommendation for Reaper IMO, but until this, it wasn't a concern for me as infrequent user it's not a problem. But it overrides this nicely I noticed. :) But really, that's just one of those mac quirks. Voiceover settings can break just about anything. I'll just file that under things to be aware of. (Possibly update the Voiceover tutorial coming up. Urgh.)

jennykbrennan commented 1 month ago

I had a sneaking suspicion that the mouse cursor speak and cursor tracking might be the only actual problem here. So I installed the first build again. And with cursor tracking off, there are no fails at all. In other words, it works perfectly. As long as the mouse cursor hint thingy is turned off. It's strange though as now, I can't get it to work properly at all when cursor tracking is on. So the first test I did makes no sense as it did work partially. This time it's either working or not working depending on that setting. Ultimately, as far as I'm concerned this is all good to go. Delay included or no.

It's curious though. @pitermach , have you found the same? Since our initial tests were both partial fails, I wonder how do you have your cursor tracking set and does it make the same difference as on my machine?

jcsteh commented 1 month ago

What are the default cursor tracking/mouse hint settings? In other words, does this work with defaults?

I'm trying to work out whether we land this with no delay or delay. I'd prefer to land it with no delay if that really does work with default settings, but if it doesn't, then we obviously need to land it with a delay.

jennykbrennan commented 1 month ago

So unfortunately, cursor tracking is on by default. As far as mouse hints go it really doesn't seem to matter. A quick test today showed that I had mine turned off, but cursor tracking still broke this.
So I will in fairness to mouse cursors of the world have to remove mouse hints from a potential cause here.

So it did appear to me that the only problem was cursor tracking and that the delay made no difference in my case. But I'm a sample size of one and wanted confirmation from someone before saying I'm sure.

It being a default setting is irritating, but mac users are, or at least they should be, used to turning this on and off a lot. It is needed in the midi editor event list at the moment, as just one example. It's just one of those toggle things we need to remind users about over and over, like Quick nav.

jcsteh commented 1 month ago

Given that cursor tracking on is default, it'd be good if we could get this working with cursor tracking on as well, so I'll wait for @pitermach to comment as to experiences with that before making a decision about delay vs no delay. Aside from my previous arguments regarding delay being brittle, delay is clunkier for users on Windows - you hear a speech hiccup with delay that you don't really notice with no delay - so I'd really prefer no delay... but I'd also prefer to avoid differences across platforms, so there are a lot of trade-offs here.

pitermach commented 1 month ago

All my testing was done with cursor tracking turned on, and I've never seen it cause issues in Reaper.Then again, I don't really use the event list in a MIDI editor often, so I could have very well missed that part.One thing I do have turned off, and I think it is also the default setting that's mouse-related, is I don't have the mouse cursor following voiceover.I have seen that cause issues in many places, like context menus in Juke's plugins and on the web, but as I said, I don't think that option is turned on by default, and cursor tracking itself didn't seem to cause issues with these announcements on my end.Sent from my iPhoneOn 11 Apr 2024, at 04:15, James Teh @.***> wrote: Given that cursor tracking on is default, it'd be good if we could get this working with cursor tracking on as well, so I'll wait for @pitermach to comment as to experiences with that before making a decision about delay vs no delay. Aside from my previous arguments regarding delay being brittle, delay is clunkier for users on Windows - you hear a speech hiccup with delay that you don't really notice with no delay - so I'd really prefer no delay... but I'd also prefer to avoid differences across platforms, so there are a lot of trade-offs here.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

jcsteh commented 1 month ago

Okay. Given that @pitermach has always had cursor tracking on and had problems with no delay but success with delay, I guess we're landing with delay.

AppVeyorBot commented 1 month ago

Build failed! Build osara pr1045-1391,e217c7d8 failed (commit https://github.com/jcsteh/osara/commit/e217c7d855 by @jcsteh)

AppVeyorBot commented 1 month ago

Build succeeded! Build osara pr1045-1391,e217c7d8 completed (commit https://github.com/jcsteh/osara/commit/e217c7d855 by @jcsteh) Downloads:

Mac - [installer/osara_pr1045-1391,e217c7d8.dmg](https://ci.appveyor.com/api/buildjobs/mma0ld2gc6yb1no5/artifacts/installer/osara_pr1045-1391,e217c7d8.dmg)
Windows - [installer/osara_pr1045-1391,e217c7d8.exe](https://ci.appveyor.com/api/buildjobs/dix2nto49j8m0j9n/artifacts/installer/osara_pr1045-1391,e217c7d8.exe) - [locale/osara_pr1045-1391,e217c7d8.pot](https://ci.appveyor.com/api/buildjobs/dix2nto49j8m0j9n/artifacts/locale/osara_pr1045-1391,e217c7d8.pot)
jennykbrennan commented 1 month ago

Yep, thanks for verifying what I didn't realize until this morning to my bottomless irritation. lol. how I hate VoiceOver. Hey Apple, there is such a thing as too much choice. I'm glad this is in the bag now though.