From 1202ab8348e63cbaf3146346d1988484869c36a4 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Sun, 25 Feb 2024 11:35:51 -0500 Subject: [PATCH 01/14] core: config: Add pen protocol version to config --- src/core/generic/config.hpp | 9 +++++++++ src/core/linux/config-loader.hpp | 14 ++++++++++++++ src/core/linux/errors.hpp | 3 +++ 3 files changed, 26 insertions(+) diff --git a/src/core/generic/config.hpp b/src/core/generic/config.hpp index fb90d6f..075a784 100644 --- a/src/core/generic/config.hpp +++ b/src/core/generic/config.hpp @@ -17,6 +17,14 @@ namespace iptsd::core { class Config { public: + // Enum to denote which Pen Protocol is being used. + enum class MPPVersion { + // Used for Microsoft Pen Protocol v1 compliant pens. + V1, + // Used for Microsoft Pen Protocol v2 compliant pens. + V2, + }; + // [Config] bool invert_x = false; bool invert_y = false; @@ -49,6 +57,7 @@ class Config { // [Stylus] bool stylus_disable = false; f64 stylus_tip_distance = 0; + MPPVersion mpp_version = MPPVersion::V1; // [DFT] usize dft_position_min_amp = 50; diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index 61b57a3..f9b45c2 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -161,6 +161,7 @@ class ConfigLoader { this->get(ini, "Stylus", "Disable", m_config.stylus_disable); this->get(ini, "Stylus", "TipDistance", m_config.stylus_tip_distance); + this->get(ini, "Stylus", "MPPVersion", m_config.mpp_version); this->get(ini, "DFT", "PositionMinAmp", m_config.dft_position_min_amp); this->get(ini, "DFT", "PositionMinMag", m_config.dft_position_min_mag); @@ -199,6 +200,19 @@ class ConfigLoader { value = gsl::narrow_cast(ini.GetReal(section, name, value)); else if constexpr (std::is_same_v) value = ini.GetString(section, name, value); + else if constexpr (std::is_same_v) { + // Parse the pen protocol verison by first reading into a string. + const auto mpp_version = ini.GetString(section, name, ""); + if (!mpp_version.empty()) { + if (mpp_version == "v1") { + value = Config::MPPVersion::V1; + } else if (mpp_version == "v2") { + value = Config::MPPVersion::V2; + } else { + throw common::Error {"Stylus mpp_version was not 'v1' or 'v2', got: " + mpp_version}; + } + } + } else throw common::Error {typeid(T).name()}; } diff --git a/src/core/linux/errors.hpp b/src/core/linux/errors.hpp index 80a4478..b152b7d 100644 --- a/src/core/linux/errors.hpp +++ b/src/core/linux/errors.hpp @@ -9,6 +9,7 @@ namespace iptsd::core::linux { enum class Error { ParsingFailed, + ParsingInvalidValue, ParsingTypeNotImplemented, RunnerInitError, @@ -25,6 +26,8 @@ inline std::string format_as(Error err) switch (err) { case Error::ParsingFailed: return "core: linux: Failed to parse INI file {}!"; + case Error::ParsingInvalidValue: + return "core: linux: Parsing encountered invalid value {}!"; case Error::ParsingTypeNotImplemented: return "core: linux: Parsing not implemented for type {}!"; case Error::RunnerInitError: From b844a0783e523b3f07615190d97d06a0211f939a Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Sun, 25 Feb 2024 12:52:34 -0500 Subject: [PATCH 02/14] ipts: Handle MPP v2 button detection. --- src/core/generic/dft.hpp | 51 +++++++++++++++++++++++++++++++++++++++ src/ipts/parser.hpp | 3 ++- src/ipts/protocol/dft.hpp | 1 + 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index ee297b0..558a329 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -28,6 +28,12 @@ class DftStylus { i32 m_imag = 0; std::optional m_group = std::nullopt; + // This is a bit of a hack, for the MPP v2 button detection we only + // care about the first 0x0a dft window, but there's two in the frame, + // here we keep track of the group when 0x0a was encountered, this + // allows comparing against this group and only using the first 0x0a + // frame. + std::optional m_dft_0x0a_group = std::nullopt; public: DftStylus(Config config, const std::optional &metadata) : m_config {std::move(config)}, @@ -50,6 +56,9 @@ class DftStylus { case ipts::protocol::dft::Type::Pressure: this->handle_pressure(dft); break; + case ipts::protocol::dft::Type::Dft0x0a: + this->handle_dft_0x0a(dft); + break; default: // Ignored break; @@ -160,6 +169,11 @@ class DftStylus { */ void handle_button(const ipts::DftWindow &dft) { + // Only use this for pen v1. + if (m_config.mpp_version != Config::MPPVersion::V1) { + return; + } + if (dft.x.empty()) return; @@ -211,6 +225,43 @@ class DftStylus { } } + + /*! + * Determines the current button state from the 0x0a frame. + */ + void handle_dft_0x0a(const ipts::DftWindow &dft) + { + if (m_config.mpp_version != Config::MPPVersion::V2) { + return; + } + + // Second time we see the 0x0a frame in this group, skip it. + if (!dft.group.has_value() || m_dft_0x0a_group == dft.group) + return; + + m_dft_0x0a_group = dft.group; + + // Now, we can process the frame to determine button state. + // First, collapse x and y, they convey the same information. + const auto mag_4 = dft.x[4].magnitude + dft.y[4].magnitude; + const auto mag_5 = dft.x[5].magnitude + dft.y[5].magnitude; + const auto threshold = 2 * m_config.dft_button_min_mag; + + if (mag_4 < threshold && mag_5 < threshold) { + // Not enough signal, lets disable the button + m_stylus.button = false; + m_stylus.rubber = false; + return; + } + + // One of them is above the threshold, if 5 is higher than 4, button + // is held. + m_stylus.button = mag_4 < mag_5; + + // This needs a todo still :) + m_stylus.rubber = false; + } + /*! * Interpolates the current stylus position from a list of antenna measurements. * diff --git a/src/ipts/parser.hpp b/src/ipts/parser.hpp index dff29f8..0bea855 100644 --- a/src/ipts/parser.hpp +++ b/src/ipts/parser.hpp @@ -16,6 +16,8 @@ #include #include +#include + #include #include @@ -71,7 +73,6 @@ class Parser { { Reader reader(data); reader.skip(header); - this->parse_hid_frame(reader); } diff --git a/src/ipts/protocol/dft.hpp b/src/ipts/protocol/dft.hpp index f6267bc..ee23016 100644 --- a/src/ipts/protocol/dft.hpp +++ b/src/ipts/protocol/dft.hpp @@ -17,6 +17,7 @@ enum class Type : u8 { Position = 6, Button = 9, Pressure = 11, + Dft0x0a = 0x0a, }; struct [[gnu::packed]] Metadata { From c80f1f5ae451be402cfcf539650623380b5e8738 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Sun, 25 Feb 2024 12:52:49 -0500 Subject: [PATCH 03/14] etc: presets: Add Surface Pro 9 preset --- etc/presets/surface-pro-9.conf | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 etc/presets/surface-pro-9.conf diff --git a/etc/presets/surface-pro-9.conf b/etc/presets/surface-pro-9.conf new file mode 100644 index 0000000..3bd5451 --- /dev/null +++ b/etc/presets/surface-pro-9.conf @@ -0,0 +1,6 @@ +[Stylus] +MPPVersion = v2 + +[DFT] +ButtonMinMag = 1000 +PositionMinMag = 500 From c8cfcecdec9e721aaed7323eccf227c50d353b91 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Sun, 25 Feb 2024 13:13:45 -0500 Subject: [PATCH 04/14] ipts: Fix eraser for MPP v2, change button threshold. --- etc/presets/surface-pro-9.conf | 3 ++- src/core/generic/dft.hpp | 18 +++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/etc/presets/surface-pro-9.conf b/etc/presets/surface-pro-9.conf index 3bd5451..9e4680f 100644 --- a/etc/presets/surface-pro-9.conf +++ b/etc/presets/surface-pro-9.conf @@ -2,5 +2,6 @@ MPPVersion = v2 [DFT] -ButtonMinMag = 1000 +# This is really high, but SP2 creates a strong signal. +ButtonMinMag = 50000 PositionMinMag = 500 diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index 558a329..9b2e862 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -169,11 +169,6 @@ class DftStylus { */ void handle_button(const ipts::DftWindow &dft) { - // Only use this for pen v1. - if (m_config.mpp_version != Config::MPPVersion::V1) { - return; - } - if (dft.x.empty()) return; @@ -199,7 +194,10 @@ class DftStylus { rubber = val > 0; } - m_stylus.button = button; + // Only set the button value if we're using a v1 pen. + if (m_config.mpp_version == Config::MPPVersion::V1) { + m_stylus.button = button; + } m_stylus.rubber = rubber; } @@ -227,7 +225,9 @@ class DftStylus { /*! - * Determines the current button state from the 0x0a frame. + * Determines the current button state from the 0x0a frame, it can + * only be used for MPP v2 pens. The eraser is still obtained from the + * phase using the button frame. */ void handle_dft_0x0a(const ipts::DftWindow &dft) { @@ -250,16 +250,12 @@ class DftStylus { if (mag_4 < threshold && mag_5 < threshold) { // Not enough signal, lets disable the button m_stylus.button = false; - m_stylus.rubber = false; return; } // One of them is above the threshold, if 5 is higher than 4, button // is held. m_stylus.button = mag_4 < mag_5; - - // This needs a todo still :) - m_stylus.rubber = false; } /*! From b4b466a460f1d9187afa21f0ceb7db274db65199 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Sun, 25 Feb 2024 17:41:10 -0500 Subject: [PATCH 05/14] core: linux: Improve configuration logging Print paths to loaded configuration files, warn if no configuration file is loaded at all. --- src/core/generic/dft.hpp | 4 ++-- src/core/linux/config-loader.hpp | 20 ++++++++++++++++---- src/ipts/parser.hpp | 2 -- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index 9b2e862..eb01368 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -34,6 +34,7 @@ class DftStylus { // allows comparing against this group and only using the first 0x0a // frame. std::optional m_dft_0x0a_group = std::nullopt; + public: DftStylus(Config config, const std::optional &metadata) : m_config {std::move(config)}, @@ -223,11 +224,10 @@ class DftStylus { } } - /*! * Determines the current button state from the 0x0a frame, it can * only be used for MPP v2 pens. The eraser is still obtained from the - * phase using the button frame. + * phase using the button frame. */ void handle_dft_0x0a(const ipts::DftWindow &dft) { diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index f9b45c2..5dd76c9 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -27,6 +27,8 @@ class ConfigLoader { Config m_config {}; DeviceInfo m_info; + bool m_loaded_config {false}; + public: ConfigLoader(const DeviceInfo &info, const std::optional &metadata) : m_info {info} @@ -48,14 +50,21 @@ class ConfigLoader { * known working main system configuration. */ if (const char *config_file_path = std::getenv("IPTSD_CONFIG_FILE")) { + spdlog::info("Loading config {}, specified with IPTSD_CONFIG_FILE.", + config_file_path); this->load_file(config_file_path); return; } - if (std::filesystem::exists(common::buildopts::ConfigFile)) + if (std::filesystem::exists(common::buildopts::ConfigFile)) { + spdlog::info("Loading config {}.", common::buildopts::ConfigFile); this->load_file(common::buildopts::ConfigFile); + } this->load_dir(common::buildopts::ConfigDir, false); + + if (!m_loaded_config) + spdlog::warn("No config file loaded at all, this is not good."); } /*! @@ -95,6 +104,7 @@ class ConfigLoader { continue; } + spdlog::info("Loading config {}.", p.path().c_str()); this->load_file(p.path()); } } @@ -176,6 +186,7 @@ class ConfigLoader { this->get(ini, "Contacts", "SizeThreshold", m_config.contacts_size_thresh_max); // clang-format on + m_loaded_config = true; } /*! @@ -209,11 +220,12 @@ class ConfigLoader { } else if (mpp_version == "v2") { value = Config::MPPVersion::V2; } else { - throw common::Error {"Stylus mpp_version was not 'v1' or 'v2', got: " + mpp_version}; + throw common::Error { + "Stylus mpp_version was not 'v1' or 'v2', got: " + + mpp_version}; } } - } - else + } else throw common::Error {typeid(T).name()}; } }; diff --git a/src/ipts/parser.hpp b/src/ipts/parser.hpp index 0bea855..b56bbce 100644 --- a/src/ipts/parser.hpp +++ b/src/ipts/parser.hpp @@ -16,8 +16,6 @@ #include #include -#include - #include #include From 3fad8016e9467cf3ed233573d719cb1c2fbdf369 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Sun, 25 Feb 2024 19:04:06 -0500 Subject: [PATCH 06/14] ipts: Add more reliable contact detection for MPP v2 If there's contact, the pen signal clearly switches columns in the Position2 window. --- etc/presets/surface-pro-9.conf | 6 ++++- src/core/generic/config.hpp | 1 + src/core/generic/dft.hpp | 43 +++++++++++++++++++++++++++++++- src/core/linux/config-loader.hpp | 1 + src/ipts/protocol/dft.hpp | 1 + 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/etc/presets/surface-pro-9.conf b/etc/presets/surface-pro-9.conf index 9e4680f..f9ba5e6 100644 --- a/etc/presets/surface-pro-9.conf +++ b/etc/presets/surface-pro-9.conf @@ -1,7 +1,11 @@ +[Device] +Vendor = 0x045E +Product = 0x0C52 + [Stylus] MPPVersion = v2 [DFT] -# This is really high, but SP2 creates a strong signal. ButtonMinMag = 50000 +ContactMinMag = 50000 PositionMinMag = 500 diff --git a/src/core/generic/config.hpp b/src/core/generic/config.hpp index 075a784..cba47a5 100644 --- a/src/core/generic/config.hpp +++ b/src/core/generic/config.hpp @@ -66,6 +66,7 @@ class Config { usize dft_button_min_mag = 1000; usize dft_freq_min_mag = 10000; usize dft_tilt_min_mag = 10000; + usize dft_contact_min_mag = 10000; f64 dft_tilt_distance = 0.6; public: diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index eb01368..dda8a71 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -35,6 +35,10 @@ class DftStylus { // frame. std::optional m_dft_0x0a_group = std::nullopt; + // Boolean to track whether the pen is in contact, used by the method + // that processes the pressure frame. + bool m_mppv2_in_contact {false}; + public: DftStylus(Config config, const std::optional &metadata) : m_config {std::move(config)}, @@ -57,6 +61,9 @@ class DftStylus { case ipts::protocol::dft::Type::Pressure: this->handle_pressure(dft); break; + case ipts::protocol::dft::Type::Position2: + this->handle_position2(dft); + break; case ipts::protocol::dft::Type::Dft0x0a: this->handle_dft_0x0a(dft); break; @@ -219,7 +226,7 @@ class DftStylus { m_stylus.contact = true; m_stylus.pressure = std::clamp(p, 0.0, 1.0); } else { - m_stylus.contact = false; + m_stylus.contact = false || m_mppv2_in_contact; // NOLINT m_stylus.pressure = 0; } } @@ -235,6 +242,11 @@ class DftStylus { return; } + if (dft.x.size() <= 5) { // not sure if this can happen? + m_stylus.button = false; + return; + } + // Second time we see the 0x0a frame in this group, skip it. if (!dft.group.has_value() || m_dft_0x0a_group == dft.group) return; @@ -258,6 +270,35 @@ class DftStylus { m_stylus.button = mag_4 < mag_5; } + /*! + * Determines whether the pen is making contact with the screen, it can + * only be used for MPP v2 pens. + */ + void handle_position2(const ipts::DftWindow &dft) + { + if (m_config.mpp_version != Config::MPPVersion::V2) { + return; + } + + // Set to false in case we return early. + m_mppv2_in_contact = false; + + if (dft.x.size() <= 3) { // not sure if this can happen? + return; + } + + const auto mag_2 = dft.x[2].magnitude + dft.y[2].magnitude; + const auto mag_3 = dft.x[3].magnitude + dft.y[3].magnitude; + + const auto threshold = 2 * m_config.dft_contact_min_mag; + if (mag_2 < threshold && mag_3 < threshold) { + return; + } + + // The pen switches the row from two to three when there's contact. + m_mppv2_in_contact = mag_2 < mag_3; + } + /*! * Interpolates the current stylus position from a list of antenna measurements. * diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index 5dd76c9..ff51e4d 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -176,6 +176,7 @@ class ConfigLoader { this->get(ini, "DFT", "PositionMinAmp", m_config.dft_position_min_amp); this->get(ini, "DFT", "PositionMinMag", m_config.dft_position_min_mag); this->get(ini, "DFT", "PositionExp", m_config.dft_position_exp); + this->get(ini, "DFT", "ContactMinMag", m_config.dft_contact_min_mag); this->get(ini, "DFT", "ButtonMinMag", m_config.dft_button_min_mag); this->get(ini, "DFT", "FreqMinMag", m_config.dft_freq_min_mag); this->get(ini, "DFT", "TiltMinMag", m_config.dft_tilt_min_mag); diff --git a/src/ipts/protocol/dft.hpp b/src/ipts/protocol/dft.hpp index ee23016..9539f88 100644 --- a/src/ipts/protocol/dft.hpp +++ b/src/ipts/protocol/dft.hpp @@ -15,6 +15,7 @@ constexpr u8 PRESSURE_ROWS = 6; enum class Type : u8 { Position = 6, + Position2 = 7, Button = 9, Pressure = 11, Dft0x0a = 0x0a, From 1f8d6bf10cbbe4f6442f2dee910217cbc44de98d Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Mon, 26 Feb 2024 08:35:05 -0500 Subject: [PATCH 07/14] core: Prefix MPP version with stylus --- src/core/generic/config.hpp | 2 +- src/core/generic/dft.hpp | 6 +++--- src/core/linux/config-loader.hpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/generic/config.hpp b/src/core/generic/config.hpp index cba47a5..509cc90 100644 --- a/src/core/generic/config.hpp +++ b/src/core/generic/config.hpp @@ -57,7 +57,7 @@ class Config { // [Stylus] bool stylus_disable = false; f64 stylus_tip_distance = 0; - MPPVersion mpp_version = MPPVersion::V1; + MPPVersion stylus_mpp_version = MPPVersion::V1; // [DFT] usize dft_position_min_amp = 50; diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index dda8a71..2f7d12f 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -203,7 +203,7 @@ class DftStylus { } // Only set the button value if we're using a v1 pen. - if (m_config.mpp_version == Config::MPPVersion::V1) { + if (m_config.stylus_mpp_version == Config::MPPVersion::V1) { m_stylus.button = button; } m_stylus.rubber = rubber; @@ -238,7 +238,7 @@ class DftStylus { */ void handle_dft_0x0a(const ipts::DftWindow &dft) { - if (m_config.mpp_version != Config::MPPVersion::V2) { + if (m_config.stylus_mpp_version != Config::MPPVersion::V2) { return; } @@ -276,7 +276,7 @@ class DftStylus { */ void handle_position2(const ipts::DftWindow &dft) { - if (m_config.mpp_version != Config::MPPVersion::V2) { + if (m_config.stylus_mpp_version != Config::MPPVersion::V2) { return; } diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index ff51e4d..0a421ce 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -171,7 +171,7 @@ class ConfigLoader { this->get(ini, "Stylus", "Disable", m_config.stylus_disable); this->get(ini, "Stylus", "TipDistance", m_config.stylus_tip_distance); - this->get(ini, "Stylus", "MPPVersion", m_config.mpp_version); + this->get(ini, "Stylus", "MPPVersion", m_config.stylus_mpp_version); this->get(ini, "DFT", "PositionMinAmp", m_config.dft_position_min_amp); this->get(ini, "DFT", "PositionMinMag", m_config.dft_position_min_mag); From 772fbaf8003288bab37fe3a2875d2e796d0d2506 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Mon, 26 Feb 2024 16:16:46 -0500 Subject: [PATCH 08/14] core: Eliminate specifying pen version Instead, rely on magnitude for determining if the v2 signal is present. --- etc/presets/surface-pro-9.conf | 8 ++---- src/core/generic/config.hpp | 11 ++------ src/core/generic/dft.hpp | 48 +++++++++++++++++--------------- src/core/linux/config-loader.hpp | 19 ++----------- 4 files changed, 34 insertions(+), 52 deletions(-) diff --git a/etc/presets/surface-pro-9.conf b/etc/presets/surface-pro-9.conf index f9ba5e6..6429481 100644 --- a/etc/presets/surface-pro-9.conf +++ b/etc/presets/surface-pro-9.conf @@ -2,10 +2,8 @@ Vendor = 0x045E Product = 0x0C52 -[Stylus] -MPPVersion = v2 - [DFT] -ButtonMinMag = 50000 -ContactMinMag = 50000 +ButtonMinMag = 10000 +Mpp2ContactMinMag = 50000 +Mpp2ButtonMinMag = 50000 PositionMinMag = 500 diff --git a/src/core/generic/config.hpp b/src/core/generic/config.hpp index 509cc90..a23a1bb 100644 --- a/src/core/generic/config.hpp +++ b/src/core/generic/config.hpp @@ -17,14 +17,6 @@ namespace iptsd::core { class Config { public: - // Enum to denote which Pen Protocol is being used. - enum class MPPVersion { - // Used for Microsoft Pen Protocol v1 compliant pens. - V1, - // Used for Microsoft Pen Protocol v2 compliant pens. - V2, - }; - // [Config] bool invert_x = false; bool invert_y = false; @@ -57,7 +49,6 @@ class Config { // [Stylus] bool stylus_disable = false; f64 stylus_tip_distance = 0; - MPPVersion stylus_mpp_version = MPPVersion::V1; // [DFT] usize dft_position_min_amp = 50; @@ -67,6 +58,8 @@ class Config { usize dft_freq_min_mag = 10000; usize dft_tilt_min_mag = 10000; usize dft_contact_min_mag = 10000; + usize dft_mpp2_button_min_mag = 50000; + usize dft_mpp2_contact_min_mag = 50000; f64 dft_tilt_distance = 0.6; public: diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index 2f7d12f..427855b 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -35,9 +35,13 @@ class DftStylus { // frame. std::optional m_dft_0x0a_group = std::nullopt; - // Boolean to track whether the pen is in contact, used by the method - // that processes the pressure frame. - bool m_mppv2_in_contact {false}; + // Boolean to track whether the pen is in contact according to mpp v2. + // This is used to override the contact state in the pressure frame handling. + std::optional m_mppv2_in_contact = std::nullopt; + + // Boolean to track whether the button is held according to mpp v2. + // This is used to override the button state in the button frame handling. + std::optional m_mppv2_button = std::nullopt; public: DftStylus(Config config, const std::optional &metadata) @@ -202,10 +206,12 @@ class DftStylus { rubber = val > 0; } - // Only set the button value if we're using a v1 pen. - if (m_config.stylus_mpp_version == Config::MPPVersion::V1) { - m_stylus.button = button; - } + // Check if the v2 version of the protocol detect the button state, if so + // overwrite the state that was calculated here. + if (m_mppv2_button.has_value()) + button = m_mppv2_button.value(); + + m_stylus.button = button; m_stylus.rubber = rubber; } @@ -226,7 +232,8 @@ class DftStylus { m_stylus.contact = true; m_stylus.pressure = std::clamp(p, 0.0, 1.0); } else { - m_stylus.contact = false || m_mppv2_in_contact; // NOLINT + m_stylus.contact = + m_mppv2_in_contact.has_value() ? m_mppv2_in_contact.value() : false; m_stylus.pressure = 0; } } @@ -238,12 +245,10 @@ class DftStylus { */ void handle_dft_0x0a(const ipts::DftWindow &dft) { - if (m_config.stylus_mpp_version != Config::MPPVersion::V2) { - return; - } + // Clearing the state in case we can't determine it. + m_mppv2_button = std::nullopt; if (dft.x.size() <= 5) { // not sure if this can happen? - m_stylus.button = false; return; } @@ -260,14 +265,13 @@ class DftStylus { const auto threshold = 2 * m_config.dft_button_min_mag; if (mag_4 < threshold && mag_5 < threshold) { - // Not enough signal, lets disable the button - m_stylus.button = false; + // Not enough signal to make a decision. return; } // One of them is above the threshold, if 5 is higher than 4, button // is held. - m_stylus.button = mag_4 < mag_5; + m_mppv2_button = mag_4 < mag_5; } /*! @@ -276,12 +280,8 @@ class DftStylus { */ void handle_position2(const ipts::DftWindow &dft) { - if (m_config.stylus_mpp_version != Config::MPPVersion::V2) { - return; - } - - // Set to false in case we return early. - m_mppv2_in_contact = false; + // Clearing the state in case we can't determine it. + m_mppv2_in_contact = std::nullopt; if (dft.x.size() <= 3) { // not sure if this can happen? return; @@ -290,8 +290,9 @@ class DftStylus { const auto mag_2 = dft.x[2].magnitude + dft.y[2].magnitude; const auto mag_3 = dft.x[3].magnitude + dft.y[3].magnitude; - const auto threshold = 2 * m_config.dft_contact_min_mag; + const auto threshold = 2 * m_config.dft_mpp2_contact_min_mag; if (mag_2 < threshold && mag_3 < threshold) { + // Not enough signal to make a decision. return; } @@ -423,6 +424,9 @@ class DftStylus { m_stylus.contact = false; m_stylus.button = false; m_stylus.rubber = false; + + m_mppv2_in_contact = std::nullopt; + m_mppv2_button = std::nullopt; } }; diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index 0a421ce..dbca90f 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -171,7 +171,6 @@ class ConfigLoader { this->get(ini, "Stylus", "Disable", m_config.stylus_disable); this->get(ini, "Stylus", "TipDistance", m_config.stylus_tip_distance); - this->get(ini, "Stylus", "MPPVersion", m_config.stylus_mpp_version); this->get(ini, "DFT", "PositionMinAmp", m_config.dft_position_min_amp); this->get(ini, "DFT", "PositionMinMag", m_config.dft_position_min_mag); @@ -181,6 +180,8 @@ class ConfigLoader { this->get(ini, "DFT", "FreqMinMag", m_config.dft_freq_min_mag); this->get(ini, "DFT", "TiltMinMag", m_config.dft_tilt_min_mag); this->get(ini, "DFT", "TiltDistance", m_config.dft_tilt_distance); + this->get(ini, "DFT", "Mpp2ContactMinMag", m_config.dft_mpp2_contact_min_mag); + this->get(ini, "DFT", "Mpp2ButtonMinMag", m_config.dft_mpp2_button_min_mag); // Legacy options that are kept for compatibility this->get(ini, "DFT", "TipDistance", m_config.stylus_tip_distance); @@ -212,21 +213,7 @@ class ConfigLoader { value = gsl::narrow_cast(ini.GetReal(section, name, value)); else if constexpr (std::is_same_v) value = ini.GetString(section, name, value); - else if constexpr (std::is_same_v) { - // Parse the pen protocol verison by first reading into a string. - const auto mpp_version = ini.GetString(section, name, ""); - if (!mpp_version.empty()) { - if (mpp_version == "v1") { - value = Config::MPPVersion::V1; - } else if (mpp_version == "v2") { - value = Config::MPPVersion::V2; - } else { - throw common::Error { - "Stylus mpp_version was not 'v1' or 'v2', got: " + - mpp_version}; - } - } - } else + else throw common::Error {typeid(T).name()}; } }; From 9aee2cd463207fb476c5296c90569653ba3aa7f9 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Mon, 26 Feb 2024 16:32:15 -0500 Subject: [PATCH 09/14] core, ipts: Incorporate minor PR feedback. --- src/core/generic/dft.hpp | 12 ++++++------ src/core/linux/config-loader.hpp | 12 ++---------- src/ipts/parser.hpp | 1 + src/ipts/protocol/dft.hpp | 10 +++++----- 4 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index 427855b..489f7ee 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -65,11 +65,11 @@ class DftStylus { case ipts::protocol::dft::Type::Pressure: this->handle_pressure(dft); break; - case ipts::protocol::dft::Type::Position2: - this->handle_position2(dft); + case ipts::protocol::dft::Type::PositionMPP_2: + this->handle_position_mpp_2(dft); break; - case ipts::protocol::dft::Type::Dft0x0a: - this->handle_dft_0x0a(dft); + case ipts::protocol::dft::Type::BinaryMPP_2: + this->handle_dft_binary_mpp_2(dft); break; default: // Ignored @@ -243,7 +243,7 @@ class DftStylus { * only be used for MPP v2 pens. The eraser is still obtained from the * phase using the button frame. */ - void handle_dft_0x0a(const ipts::DftWindow &dft) + void handle_dft_binary_mpp_2(const ipts::DftWindow &dft) { // Clearing the state in case we can't determine it. m_mppv2_button = std::nullopt; @@ -278,7 +278,7 @@ class DftStylus { * Determines whether the pen is making contact with the screen, it can * only be used for MPP v2 pens. */ - void handle_position2(const ipts::DftWindow &dft) + void handle_position_mpp_2(const ipts::DftWindow &dft) { // Clearing the state in case we can't determine it. m_mppv2_in_contact = std::nullopt; diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index dbca90f..623a7f1 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -27,8 +27,6 @@ class ConfigLoader { Config m_config {}; DeviceInfo m_info; - bool m_loaded_config {false}; - public: ConfigLoader(const DeviceInfo &info, const std::optional &metadata) : m_info {info} @@ -50,21 +48,15 @@ class ConfigLoader { * known working main system configuration. */ if (const char *config_file_path = std::getenv("IPTSD_CONFIG_FILE")) { - spdlog::info("Loading config {}, specified with IPTSD_CONFIG_FILE.", - config_file_path); this->load_file(config_file_path); return; } if (std::filesystem::exists(common::buildopts::ConfigFile)) { - spdlog::info("Loading config {}.", common::buildopts::ConfigFile); this->load_file(common::buildopts::ConfigFile); } this->load_dir(common::buildopts::ConfigDir, false); - - if (!m_loaded_config) - spdlog::warn("No config file loaded at all, this is not good."); } /*! @@ -104,7 +96,6 @@ class ConfigLoader { continue; } - spdlog::info("Loading config {}.", p.path().c_str()); this->load_file(p.path()); } } @@ -137,6 +128,8 @@ class ConfigLoader { */ void load_file(const std::filesystem::path &path) { + spdlog::info("Loading config {}.", path.c_str()); + const INIReader ini {path}; if (ini.ParseError() != 0) @@ -188,7 +181,6 @@ class ConfigLoader { this->get(ini, "Contacts", "SizeThreshold", m_config.contacts_size_thresh_max); // clang-format on - m_loaded_config = true; } /*! diff --git a/src/ipts/parser.hpp b/src/ipts/parser.hpp index b56bbce..dff29f8 100644 --- a/src/ipts/parser.hpp +++ b/src/ipts/parser.hpp @@ -71,6 +71,7 @@ class Parser { { Reader reader(data); reader.skip(header); + this->parse_hid_frame(reader); } diff --git a/src/ipts/protocol/dft.hpp b/src/ipts/protocol/dft.hpp index 9539f88..c85db86 100644 --- a/src/ipts/protocol/dft.hpp +++ b/src/ipts/protocol/dft.hpp @@ -14,11 +14,11 @@ constexpr u8 MAX_ROWS = 16; constexpr u8 PRESSURE_ROWS = 6; enum class Type : u8 { - Position = 6, - Position2 = 7, - Button = 9, - Pressure = 11, - Dft0x0a = 0x0a, + Position = 0x6, + PositionMPP_2 = 0x7, + Button = 0x9, + BinaryMPP_2 = 0xA, + Pressure = 0xB, }; struct [[gnu::packed]] Metadata { From b4ef3bdd6243c21f4af33c86abb84ff6d74a5e17 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Tue, 27 Feb 2024 18:53:30 -0500 Subject: [PATCH 10/14] presets: Remove surface pro 9 config It is unnecessary now that the mppv2 values are separate. --- etc/presets/surface-pro-9.conf | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 etc/presets/surface-pro-9.conf diff --git a/etc/presets/surface-pro-9.conf b/etc/presets/surface-pro-9.conf deleted file mode 100644 index 6429481..0000000 --- a/etc/presets/surface-pro-9.conf +++ /dev/null @@ -1,9 +0,0 @@ -[Device] -Vendor = 0x045E -Product = 0x0C52 - -[DFT] -ButtonMinMag = 10000 -Mpp2ContactMinMag = 50000 -Mpp2ButtonMinMag = 50000 -PositionMinMag = 500 From 1623671b32151f52cefffb0043235bb0ef515813 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Tue, 27 Feb 2024 18:54:26 -0500 Subject: [PATCH 11/14] core: linux: Add info when no configuration files are loaded --- src/core/linux/config-loader.hpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index 623a7f1..66007be 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -27,6 +27,8 @@ class ConfigLoader { Config m_config {}; DeviceInfo m_info; + bool m_loaded_config = false; + public: ConfigLoader(const DeviceInfo &info, const std::optional &metadata) : m_info {info} @@ -52,11 +54,13 @@ class ConfigLoader { return; } - if (std::filesystem::exists(common::buildopts::ConfigFile)) { + if (std::filesystem::exists(common::buildopts::ConfigFile)) this->load_file(common::buildopts::ConfigFile); - } this->load_dir(common::buildopts::ConfigDir, false); + + if (!m_loaded_config) + spdlog::info("No config file loaded, using default values."); } /*! @@ -181,6 +185,7 @@ class ConfigLoader { this->get(ini, "Contacts", "SizeThreshold", m_config.contacts_size_thresh_max); // clang-format on + m_loaded_config = true; } /*! From 5f399fee731786d990e3e2982455efa4bf299621 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Tue, 27 Feb 2024 18:55:21 -0500 Subject: [PATCH 12/14] core: Incorporate PR feedback around mppv2 --- src/core/generic/dft.hpp | 25 +++++++++++-------------- src/core/linux/errors.hpp | 3 --- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index 489f7ee..10682cd 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -29,11 +29,10 @@ class DftStylus { std::optional m_group = std::nullopt; // This is a bit of a hack, for the MPP v2 button detection we only - // care about the first 0x0a dft window, but there's two in the frame, - // here we keep track of the group when 0x0a was encountered, this - // allows comparing against this group and only using the first 0x0a - // frame. - std::optional m_dft_0x0a_group = std::nullopt; + // care about the first binary (0x0a) dft window, but there's two in the + // frame, here we keep track of the group when 0x0a was encountered, + // this allows comparing against this group and only using the first frame. + std::optional m_mppv2_binary_group = std::nullopt; // Boolean to track whether the pen is in contact according to mpp v2. // This is used to override the contact state in the pressure frame handling. @@ -232,8 +231,7 @@ class DftStylus { m_stylus.contact = true; m_stylus.pressure = std::clamp(p, 0.0, 1.0); } else { - m_stylus.contact = - m_mppv2_in_contact.has_value() ? m_mppv2_in_contact.value() : false; + m_stylus.contact = m_mppv2_in_contact.value_or(false); m_stylus.pressure = 0; } } @@ -245,24 +243,23 @@ class DftStylus { */ void handle_dft_binary_mpp_2(const ipts::DftWindow &dft) { - // Clearing the state in case we can't determine it. - m_mppv2_button = std::nullopt; - if (dft.x.size() <= 5) { // not sure if this can happen? return; } - // Second time we see the 0x0a frame in this group, skip it. - if (!dft.group.has_value() || m_dft_0x0a_group == dft.group) + // Second time we see this dft window in this group, skip it. + if (!dft.group.has_value() || m_mppv2_binary_group == dft.group) return; + m_mppv2_binary_group = dft.group; - m_dft_0x0a_group = dft.group; + // Clearing the state in case we can't determine it. + m_mppv2_button = std::nullopt; // Now, we can process the frame to determine button state. // First, collapse x and y, they convey the same information. const auto mag_4 = dft.x[4].magnitude + dft.y[4].magnitude; const auto mag_5 = dft.x[5].magnitude + dft.y[5].magnitude; - const auto threshold = 2 * m_config.dft_button_min_mag; + const auto threshold = 2 * m_config.dft_mpp2_button_min_mag; if (mag_4 < threshold && mag_5 < threshold) { // Not enough signal to make a decision. diff --git a/src/core/linux/errors.hpp b/src/core/linux/errors.hpp index b152b7d..80a4478 100644 --- a/src/core/linux/errors.hpp +++ b/src/core/linux/errors.hpp @@ -9,7 +9,6 @@ namespace iptsd::core::linux { enum class Error { ParsingFailed, - ParsingInvalidValue, ParsingTypeNotImplemented, RunnerInitError, @@ -26,8 +25,6 @@ inline std::string format_as(Error err) switch (err) { case Error::ParsingFailed: return "core: linux: Failed to parse INI file {}!"; - case Error::ParsingInvalidValue: - return "core: linux: Parsing encountered invalid value {}!"; case Error::ParsingTypeNotImplemented: return "core: linux: Parsing not implemented for type {}!"; case Error::RunnerInitError: From 2d49ff7673c1a797348b84f48b75b919d507a842 Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Wed, 28 Feb 2024 18:21:02 -0500 Subject: [PATCH 13/14] core: generic: Allow eraser state on mppv2 button state The mppv2 button detection can also happen when the eraser is pressed through a thumb button. So instead of assuming it is a barrel button, use the phase to decide whether it is the eraser or barrel button. --- src/core/generic/dft.hpp | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/core/generic/dft.hpp b/src/core/generic/dft.hpp index 10682cd..33e72c9 100644 --- a/src/core/generic/dft.hpp +++ b/src/core/generic/dft.hpp @@ -28,20 +28,20 @@ class DftStylus { i32 m_imag = 0; std::optional m_group = std::nullopt; - // This is a bit of a hack, for the MPP v2 button detection we only - // care about the first binary (0x0a) dft window, but there's two in the - // frame, here we keep track of the group when 0x0a was encountered, - // this allows comparing against this group and only using the first frame. + // For the MPP v2 button detection we only care about the first binary + // (0x0a) dft window, but there's two in the group, we keep track of the + // group when 0x0a was encountered, this allows comparing against this + // value to use only the first window in the group. std::optional m_mppv2_binary_group = std::nullopt; + // Boolean to track whether a button is held according to mpp v2. + // This is used instead of the button threshold detection. + std::optional m_mppv2_button_or_eraser = std::nullopt; + // Boolean to track whether the pen is in contact according to mpp v2. // This is used to override the contact state in the pressure frame handling. std::optional m_mppv2_in_contact = std::nullopt; - // Boolean to track whether the button is held according to mpp v2. - // This is used to override the button state in the button frame handling. - std::optional m_mppv2_button = std::nullopt; - public: DftStylus(Config config, const std::optional &metadata) : m_config {std::move(config)}, @@ -191,8 +191,10 @@ class DftStylus { bool button = false; bool rubber = false; - if (dft.x[0].magnitude > m_config.dft_button_min_mag && - dft.y[0].magnitude > m_config.dft_button_min_mag) { + // If mppv2 has decided on a button state use that, else use the magnitude decision. + if (m_mppv2_button_or_eraser.value_or( + dft.x[0].magnitude > m_config.dft_button_min_mag && + dft.y[0].magnitude > m_config.dft_button_min_mag)) { const i32 real = dft.x[0].real[ipts::protocol::dft::NUM_COMPONENTS / 2] + dft.y[0].real[ipts::protocol::dft::NUM_COMPONENTS / 2]; const i32 imag = dft.x[0].imag[ipts::protocol::dft::NUM_COMPONENTS / 2] + @@ -205,11 +207,6 @@ class DftStylus { rubber = val > 0; } - // Check if the v2 version of the protocol detect the button state, if so - // overwrite the state that was calculated here. - if (m_mppv2_button.has_value()) - button = m_mppv2_button.value(); - m_stylus.button = button; m_stylus.rubber = rubber; } @@ -253,7 +250,7 @@ class DftStylus { m_mppv2_binary_group = dft.group; // Clearing the state in case we can't determine it. - m_mppv2_button = std::nullopt; + m_mppv2_button_or_eraser = std::nullopt; // Now, we can process the frame to determine button state. // First, collapse x and y, they convey the same information. @@ -268,7 +265,7 @@ class DftStylus { // One of them is above the threshold, if 5 is higher than 4, button // is held. - m_mppv2_button = mag_4 < mag_5; + m_mppv2_button_or_eraser = mag_4 < mag_5; } /*! @@ -423,7 +420,7 @@ class DftStylus { m_stylus.rubber = false; m_mppv2_in_contact = std::nullopt; - m_mppv2_button = std::nullopt; + m_mppv2_button_or_eraser = std::nullopt; } }; From 4895549b95654930869240708e677bfe4a0203fe Mon Sep 17 00:00:00 2001 From: Ivor Wanders Date: Thu, 29 Feb 2024 07:27:29 -0500 Subject: [PATCH 14/14] core: Remove unused variable from configuration --- src/core/generic/config.hpp | 1 - src/core/linux/config-loader.hpp | 1 - 2 files changed, 2 deletions(-) diff --git a/src/core/generic/config.hpp b/src/core/generic/config.hpp index a23a1bb..dfeb0c2 100644 --- a/src/core/generic/config.hpp +++ b/src/core/generic/config.hpp @@ -57,7 +57,6 @@ class Config { usize dft_button_min_mag = 1000; usize dft_freq_min_mag = 10000; usize dft_tilt_min_mag = 10000; - usize dft_contact_min_mag = 10000; usize dft_mpp2_button_min_mag = 50000; usize dft_mpp2_contact_min_mag = 50000; f64 dft_tilt_distance = 0.6; diff --git a/src/core/linux/config-loader.hpp b/src/core/linux/config-loader.hpp index 66007be..93eea34 100644 --- a/src/core/linux/config-loader.hpp +++ b/src/core/linux/config-loader.hpp @@ -172,7 +172,6 @@ class ConfigLoader { this->get(ini, "DFT", "PositionMinAmp", m_config.dft_position_min_amp); this->get(ini, "DFT", "PositionMinMag", m_config.dft_position_min_mag); this->get(ini, "DFT", "PositionExp", m_config.dft_position_exp); - this->get(ini, "DFT", "ContactMinMag", m_config.dft_contact_min_mag); this->get(ini, "DFT", "ButtonMinMag", m_config.dft_button_min_mag); this->get(ini, "DFT", "FreqMinMag", m_config.dft_freq_min_mag); this->get(ini, "DFT", "TiltMinMag", m_config.dft_tilt_min_mag);