Skip to content

Commit

Permalink
Update touch state definition
Browse files Browse the repository at this point in the history
  • Loading branch information
@Goshin @usr-sse2
Goshin authored and usr-sse2 committed Mar 19, 2021
1 parent d64b0f4 commit a1e92d7
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 21 deletions.
27 changes: 7 additions & 20 deletions VoodooInput/VoodooInputSimulator/VoodooInputSimulatorDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,7 @@ void VoodooInputSimulatorDevice::constructReportGated(const VoodooInputEvent& mu

// in case the obtained id is greater than 14, usually 0~4 for common devices.
UInt16 touch_id = transducer->secondaryId % 15;
if (!transducer->isTransducerActive) {
touch_state[touch_id] = input_report->Button ? 0 : 2;
} else {
input_active = true;
}
input_active |= transducer->isTransducerActive;

MAGIC_TRACKPAD_INPUT_REPORT_FINGER& finger_data = input_report->FINGERS[i];

Expand All @@ -121,11 +117,8 @@ void VoodooInputSimulatorDevice::constructReportGated(const VoodooInputEvent& mu
}
}

if (touch_state[touch_id] == 4) {
finger_data.State = 0x4;
} else {
finger_data.State = ++touch_state[touch_id];
}
finger_data.State = touch_active[touch_id] ? kTouchStateActive : kTouchStateStart;
touch_active[touch_id] = transducer->isTransducerActive || transducer->isPhysicalButtonDown;

finger_data.Finger = transducer->fingerType;

Expand All @@ -145,8 +138,8 @@ void VoodooInputSimulatorDevice::constructReportGated(const VoodooInputEvent& mu
finger_data.Pressure = 120;
}

if (!transducer->isTransducerActive && !input_report->Button) {
finger_data.State = 0x7;
if (!transducer->isTransducerActive && !transducer->isPhysicalButtonDown) {
finger_data.State = kTouchStateStop;
finger_data.Size = 0x0;
finger_data.Pressure = 0x0;
finger_data.Touch_Minor = 0;
Expand Down Expand Up @@ -174,9 +167,7 @@ void VoodooInputSimulatorDevice::constructReportGated(const VoodooInputEvent& mu
}

if (!input_active) {
for (int i = 0; i < 15; i++) {
touch_state[i] = 2;
}
memset(touch_active, false, sizeof(touch_active));

input_report->FINGERS[0].Size = 0x0;
input_report->FINGERS[0].Pressure = 0x0;
Expand All @@ -192,7 +183,7 @@ void VoodooInputSimulatorDevice::constructReportGated(const VoodooInputEvent& mu
sendReport();

input_report->FINGERS[0].Finger = kMT2FingerTypeUndefined;
input_report->FINGERS[0].State = 0x0;
input_report->FINGERS[0].State = kTouchStateInactive;
input_report_buffer->setLength(total_report_len);
sendReport();

Expand Down Expand Up @@ -249,10 +240,6 @@ bool VoodooInputSimulatorDevice::start(IOService* provider) {
PMinit();
provider->joinPMtree(this);
registerPowerDriver(this, PMPowerStates, kIOPMNumberPowerStates);

for (int i = 0; i < 15; i++) {
touch_state[i] = 2;
}

new_get_report_buffer = nullptr;

Expand Down
14 changes: 13 additions & 1 deletion VoodooInput/VoodooInputSimulator/VoodooInputSimulatorDevice.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@
#define MT2_MAX_X 8134
#define MT2_MAX_Y 5206

/* State bits reference: linux/drivers/hid/hid-magicmouse.c#L58-L63 */
#define MT2_TOUCH_STATE_BIT_TRANSITION (0x1)
#define MT2_TOUCH_STATE_BIT_NEAR (0x1 << 1)
#define MT2_TOUCH_STATE_BIT_CONTACT (0x1 << 2)

enum TouchStates {
kTouchStateInactive = 0x0,
kTouchStateStart = MT2_TOUCH_STATE_BIT_NEAR | MT2_TOUCH_STATE_BIT_TRANSITION,
kTouchStateActive = MT2_TOUCH_STATE_BIT_CONTACT,
kTouchStateStop = MT2_TOUCH_STATE_BIT_CONTACT | MT2_TOUCH_STATE_BIT_NEAR | MT2_TOUCH_STATE_BIT_TRANSITION
};

/* Finger Packet
+---+---+---+---+---+---+---+---+---+
| | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
Expand Down Expand Up @@ -113,7 +125,7 @@ class EXPORT VoodooInputSimulatorDevice : public IOHIDDevice {
VoodooInput* engine {nullptr};
AbsoluteTime start_timestamp {};
OSData* new_get_report_buffer {nullptr};
UInt8 touch_state[15] {};
bool touch_active[15] {false};
IOWorkLoop* work_loop {nullptr};
IOCommandGate* command_gate {nullptr};
IOBufferMemoryDescriptor* input_report_buffer {nullptr};
Expand Down

8 comments on commit a1e92d7

@lvs1974
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goshin: this commit produces some issues with my trackpad (when I have to select or drag something while keeping one finger in a corner of touchpad and moving another finger to select a long block or move something in a long distance, so I have to up this finger and touch again):
https://drive.google.com/file/d/1MVhIQeI9FnlSoNIh6JgycrYrjfeX-Tyq/view?usp=sharing
https://drive.google.com/file/d/18VfySvw5-c0MlNUIJ9d7tqWdsLa_rTjz/view?usp=sharing
https://drive.google.com/file/d/1_ymIW6C8upa2Z7g9nx113XTNPVbIeJEz/view?usp=sharing

@vit9696
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lvs1974 are you sure there is any functional change in this commit? It looked like plain refactoring to me. Better make an issue…

@lvs1974
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vit9696: it is not just a plain refactoring...

     if (!transducer->isTransducerActive) {
         touch_state[touch_id] = input_report->Button ? 0 : 2;
     } else {
         input_active = true;
     }

is not the same as:
input_active |= transducer->isTransducerActive;

@Goshin
Copy link
Contributor Author

@Goshin Goshin commented on a1e92d7 Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lvs1974 When would the cursor jump happen? When you release or re-contact the second finger?

@lvs1974
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goshin: Cursor jumps when I re-contact the second finger.

@Goshin
Copy link
Contributor Author

@Goshin Goshin commented on a1e92d7 Mar 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lvs1974 as discussed in #10, this is really a device-specific problem. From the log file you gave in #10 (comment), the states are messed up when you do the gesture. isTransducerActive of the first finger wrongly becomes false, and isTransducerActive for the second finger wrongly remains true when the second finger leaves.

VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 1, finger_id = 0, pressure = 0, x = 1827, y = 1479, width = 0
VoodooInputSimulatorDevice: transducer index = 1, phys_button = 0, isActive = 1, finger_id = 1, pressure = 0, x = 1015, y = 2242, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1733
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 0, finger_id = 0, pressure = 0, x = 1827, y = 1479, width = 0
VoodooInputSimulatorDevice: transducer index = 1, phys_button = 0, isActive = 1, finger_id = 1, pressure = 0, x = 1015, y = 2242, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1734
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 0, finger_id = 0, pressure = 0, x = 1827, y = 1479, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1735
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 0, finger_id = 0, pressure = 0, x = 1827, y = 1479, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1736
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 0, finger_id = 0, pressure = 0, x = 1827, y = 1479, width = 0

and the position of the first finger drifts when the second finger re-contact.

VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 0, finger_id = 0, pressure = 0, x = 1827, y = 1479, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1763
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 0, finger_id = 0, pressure = 0, x = 1827, y = 1479, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1764
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 1, finger_id = 0, pressure = 0, x = 1690, y = 918, width = 0
VoodooInputSimulatorDevice: transducer index = 1, phys_button = 0, isActive = 1, finger_id = 1, pressure = 0, x = 1015, y = 2242, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1765
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 1, finger_id = 0, pressure = 0, x = 1690, y = 918, width = 0
VoodooInputSimulatorDevice: transducer index = 1, phys_button = 0, isActive = 1, finger_id = 1, pressure = 0, x = 1015, y = 2242, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1766
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 1, finger_id = 0, pressure = 0, x = 1693, y = 919, width = 0
VoodooInputSimulatorDevice: transducer index = 1, phys_button = 0, isActive = 1, finger_id = 1, pressure = 0, x = 1015, y = 2242, width = 0
VoodooInputSimulatorDevice::constructReportGated: enter, counter = 1767
VoodooInputSimulatorDevice: transducer index = 0, phys_button = 1, isActive = 1, finger_id = 0, pressure = 0, x = 1695, y = 920, width = 0
VoodooInputSimulatorDevice: transducer index = 1, phys_button = 0, isActive = 1, finger_id = 1, pressure = 0, x = 1015, y = 2242, width = 0

You can try the workaround here, like before to temporarily ignore the first finger in this case.

         if (input_report->Button) {
             finger_data.Pressure = 120;
         }

+        if (!transducer->isTransducerActive && transducer->isPhysicalButtonDown) {
+            finger_data.State = MT2_TOUCH_STATE_BIT_TRANSITION;
+        }
         if (!transducer->isTransducerActive && !transducer->isPhysicalButtonDown) {
             finger_data.State = kTouchStateStop;
             finger_data.Size = 0x0;

This is ugly and now I kind of wonder if it should be solved in VoodooInput because it is intended to be a generic MT2 adapter while this is a device problem.

In VoodooI2C/VoodooI2C#445 they are going to handle button clicks via the other TrackpointDevice IOHIPointing device, and consider removing the isPhysicalButtonDown field. Afterward maybe this problem will be solved automatically.

@lvs1974
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goshin: I have tried a workaround, but unfortunately it did not fix this issue.
I totally agree it is a device-specific problem.
Probably it will be fixed with VoodooI2C/VoodooI2CHID#39.

@lvs1974
Copy link
Contributor

@lvs1974 lvs1974 commented on a1e92d7 Mar 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goshin: I built a VoodooI2C & VoodooI2CHID from https://github.com/usr-sse2/VoodooI2C (branch for PR VoodooI2C/VoodooI2C#445) and used the latest VoodooInput from acidanthera master (without workaround you suggested today).
And it works so far!

Please sign in to comment.