feat(web): touchpath segment objects 🐵#7142
Conversation
User Test ResultsTest specification and instructions User tests are not required |
mcdurdin
left a comment
There was a problem hiding this comment.
The code LGTM. Not much I can say on the algorithm without spending a very long time on it -- it seems like there is a lot of complexity and that concerns me a little because I don't think anyone other than you is going to be able to maintain this at present without a lot of research. I guess I will have to wait until you have an interactive working solution and actually trying it out before I can give much more feedback 😁
Hmm. Is there anything in particular that could use additional documentation? I definitely noted the complexity happening and tried to mitigate it via docs, but it sounds like it's still a bit opaque, based on your feedback. I'll admit the biggest offender is probably the "pending commit" mechanism - that definitely raised the complexity level, but it's necessary for maintaining segment recognition guarantees once they're made. "Recognition" is an event (of sorts, even if Promise-based here), so once we've raised that event, we need to commit to it. Things could behave quite peculiarly if we internally revert the cause of such an event and fire off new events based on new conclusions that ignore the old conclusion. Performing any sort of 'rollback' would nigh-certainly be far more complex than what we have here. |
…ecks for pre-commit
|
OK, yeah, the most critical method for the "pending commit" mechanism's integration into the algorithm wasn't exactly documented. I've just documented that one. Definitely isn't the cleanest implementation of a mechanism I've ever coded, but at least it should be (more) possible to follow now. And added a little bit of centralization for split-point management while I was at it, because that was bugging me. |
|
I think it's still all a bit of a black box -- as I spend more time on it, it may get better. I worry that the segmentation abstraction that we've come up with may not be the most optimum approach for solving the problem -- it may be overkill and introduce more complexity than we need? |
Continuing from #7058, this PR seeks to construct & publish "proper" Segment objects to the
TrackedPathobject that may be used for later stages of gesture recognition. These are synthesized by examining the sequence of "subsegments" produced by the segmentation algorithm and recombining them as appropriate based on a few common patterns and rules.This implements "stage one" from our "Gesture Recognition" design doc.
Additionally, this PR's changes seek to provide a better, more long-term-maintenance-friendly set of debugging functions, logging methods, etc - rather than maintaining the slew of raw
console.log()statements its predecessor had. Accordingly, any debug-logging statements are no longer accessible except through user interaction via the developer console.While I could see there being a desire to split this one into pieces... I'm a little stumped at the moment on how to approach that well - the most-easily separable changes are required by those most directly connected to already-completed work. Apologies that it grew to this point before being submitted for review, though I'm still glad it wasn't as large as I feared.
While user tests are technically possible to write, they'd require parsing the output logs to verify success - and that's not the simplest thing to ask of our user testers. (I actually started to write a user test or two and then realized just how complex the "verify" instructions were going to be.)
As I plan to follow this up a PR or two for unit-testing, and as this part of the module is meant to be quite "internal" in the grand scheme of things...
@keymanapp-test-bot skip
See #7157 for tests.