feat: Add keystroke overlay #496
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ADd # modified: parser/parser_test.go
This, as you might expect, includes KeyStrokes coverage!
We're going to need this, at least temporarily, for supporting the incoming key stroke recording types.
This currently includes a few different things: - KeyStrokeEvent: Mostly taken from steno almost verbatim. - KeyStrokeEvents: A wrapper around a slice of KeyStrokeEvents that helps us package a little state with the events such that we can record exactly when they should be rendered. - Page/Keyboard: Another wrapper type around their equivalently named rod types. These help us ensure that we continue to log the keypresses without substantial changes to the call sites, and also minimizes the chance of bugs when command execution is modified since all inputs go through these types.
I was hesitant about this approach at first, and technically still am to a degree. However, ApplyLoopOffset() seems to be setting a precedent where VideoOpts may be modified at a later point in the program's lifespan, so I think this might be a fine approach.
This also makes it easier to interop with potential future changes that make the overlay follow the set font family for the recording.
This allows us to not have to do things like draw an actual newline and instead draw some readable symbol for it.
This is actually still too simple. We'll have to do testing with the margin or whatever settings in vhs.
This also gets rid of the minor TODO comment about an empty events slice, which could technically happen.
Makes it so that the keystroke overlay's drawing agrees with the specified margin, otherwise, we'd be drawing on the margin which is probably OK, but doesn't make as much sense as honoring the margin. For padding, I opted to just reuse opts.Style.Padding for consistency.
Before, we were either showing all the overlay the moment things were turned on, or never at all. Now we properly support turning it on/off in the middle of the recording.
We weren't actually using them, and in general from our talks with Maas, we want to err on implementing less in the first implementation, and add more as requests come in.
This is mostly to avoid confusion with so-called 'keypress commands'.
This also fixes the time-drift that became more and more pronounced as the recording proceeded.
We need to count the runes in the conditional check, not just the truncating of the display string. Simple oversight.
Does some tune-up of our special symbols: - Delete now uses its correct symbol - PageUp & PageDown is replaced with their correct symbols - Escape replaced with left diagonal arrow (way more readable too) - Alt replaced with the option key symbol, not sure how I feel about this. - Control replaced with ^. - Shift added. It should be noted that "correct" here is a bit misleading. I don't know if there are any 'standard' symbols for some of these keystrokes, but I'm just choosing ones that seem to map to the key when you search them up online.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm sorry it took so long. This wasn't really my priority the last couple of weeks so I didn't give it as much time as I could have.
Adds functionality for injecting a keystroke overlay into a
vhsrecording. This addresses #164 & #468. A large portion of this work is adapted from my work on steno.The main overview of the approach here is that:
Set KeyStrokes [Hide|Show]in tape files. This can be toggled on/off throughout the recording.rod, includingrod.Page,rod.KeyActionsandrod.Keyboard, and make it so that any timevhscodebase wants to "type" something in some fashion, it goes through these decorated wrappers. The wrappers will record keystrokes.ffmpegto generate output files, we use a series ofdrawtextfilters based on the recorded keystroke events to render the keystroke overlay.Testing was done via some extended/added unit tests, which can be found in this PR.
I also ran E2E tests, but the verification was manual and via eyeball since there's no good way to automatically test
vhsin an E2E manner (at least currently). You can find my test cases here, which are executed via a Python script, also on that branch (here).NOTE 1: This PR should be reviewable commit-by-commit if you prefer.
NOTE 2: Force-pushes were from me trying to fix my GPG signing configuration on WSL.
Demo
Open Questions
fontconfig, which is whatffmpeguses, but I'm open to changing the code to only use basic ASCII for example.defaultFontSize, or should we match whatever the specified font size is?