mirror of
https://github.com/MonitorControl/MonitorControl.git
synced 2026-05-15 14:15:55 -06:00
[PR #134] [MERGED] Performance enhancements for media key shortcuts, repeated keys #1055
Labels
No labels
Status: Abandoned
arm64
beta
beta
bug
done
duplicate
enhancement
feedback needed from reporter
in progress
invalid
investigating
known Issue
monitor Issue
pull-request
translation
unable to reproduce
unreleased
x86
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: github-starred/MonitorControl#1055
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
📋 Pull Request Information
Original PR: https://github.com/MonitorControl/MonitorControl/pull/134
Author: @robertbressi
Created: 10/1/2019
Status: ✅ Merged
Merged: 10/24/2019
Merged by: @JoniVR
Base:
master← Head:user/rbressi/performance-enhancements📝 Commits (4)
674e813Add handling for holding down media shortcut keys7814d03Added handling to not send through DDC commands for values which had already been setef419e9Moved DDC reads and writes onto the main threadff8f6a7Fixed syntax for optional binding📊 Changes
3 files changed (+141 additions, -112 deletions)
View changed files
📝
MonitorControl/AppDelegate.swift(+46 -6)📝
MonitorControl/Display.swift(+52 -56)📝
MonitorControl/Support/Utils.swift(+43 -50)📄 Description
NOTE: This PR depends on
#131, #133 and should not be merged before those two fixes - for the sake of avoiding conflicts.This set of changes is bound to be controversial, so I wanted to get eyes on it ASAP. Each change has been split into a separate commit:
Add handling for holding down media shortcut keys
Currently, the media key handler doesn't handle shortcut keys being held down. This change sets a 50ms timer for key repeats and also handles when the "opposite" key is pressed
Add handling to not send through DDC commands for values which had already been set
Currently, it's possible to keep flooding the DDC interface with values which are already set on the monitor (e.g.: going from 0 to 0 or 100 to 100). This change prevents this from happening by simply not sending through the duplicate command.
Moved DDC reads and writes onto the main thread
This commit is the big change.
I noticed that when setting the volume, brightness and contrast from the sliders (which occurs on the main thread), there is no lag in sending many commands through to the DDC interface.
This is in comparison to the media shortcut keys, in which DDC commands are sent through a background queue, and huge amounts of lag are experienced, especially while holding down the media shortcut keys - as shown below (this experience was so bad that I almost had to quit the app altogether after recording because it was still trying to send through commands after I released the media key):

When running the DDC command writes through the main thread however, the experience looks more like this:

Similarly, for reading DDC values, here's a log with timestamps of the app starting app with values being read on the background queue (reading is 3+ seconds and there are multiple request failures):
Log (background queue)
Compare this to results on the main thread (startup in 800ms, no failed requests):
Log (main queue)
So, I'm not sure whether this experience differs with other configuration sets, such as if there's a longer delay or higher polling set, but I wanted to get it in front of the contributors' eyes to see if this may be a meaningful set of improvements.
As always, happy to accept feedback and make whatever changes are necessary (or to split out individual changes if the whole thing is a little much for one PR!) - let me know :)
🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.