Skip to content

MPP v2+, Slim Pen 2, Surface Pro 9 improvements #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Mar 3, 2024
Merged

MPP v2+, Slim Pen 2, Surface Pro 9 improvements #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1202ab8
core: config: Add pen protocol version to config
iwanders Feb 25, 2024
b844a07
ipts: Handle MPP v2 button detection.
iwanders Feb 25, 2024
c80f1f5
etc: presets: Add Surface Pro 9 preset
iwanders Feb 25, 2024
c8cfcec
ipts: Fix eraser for MPP v2, change button threshold.
iwanders Feb 25, 2024
b4b466a
core: linux: Improve configuration logging
iwanders Feb 25, 2024
3fad801
ipts: Add more reliable contact detection for MPP v2
iwanders Feb 26, 2024
1f8d6bf
core: Prefix MPP version with stylus
iwanders Feb 26, 2024
772fbaf
core: Eliminate specifying pen version
iwanders Feb 26, 2024
9aee2cd
core, ipts: Incorporate minor PR feedback.
iwanders Feb 26, 2024
b4ef3bd
presets: Remove surface pro 9 config
iwanders Feb 27, 2024
1623671
core: linux: Add info when no configuration files are loaded
iwanders Feb 27, 2024
5f399fe
core: Incorporate PR feedback around mppv2
iwanders Feb 27, 2024
2d49ff7
core: generic: Allow eraser state on mppv2 button state
iwanders Feb 28, 2024
4895549
core: Remove unused variable from configuration
iwanders Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/core/generic/config.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class Config {
usize dft_button_min_mag = 1000;
usize dft_freq_min_mag = 10000;
usize dft_tilt_min_mag = 10000;
usize dft_mpp2_button_min_mag = 50000;
usize dft_mpp2_contact_min_mag = 50000;
f64 dft_tilt_distance = 0.6;

public:
Expand Down
92 changes: 89 additions & 3 deletions src/core/generic/dft.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ class DftStylus {
i32 m_imag = 0;
std::optional<u32> m_group = std::nullopt;

// 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<u32> 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<bool> 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<bool> m_mppv2_in_contact = std::nullopt;

public:
DftStylus(Config config, const std::optional<const ipts::Metadata> &metadata)
: m_config {std::move(config)},
Expand All @@ -50,6 +64,12 @@ class DftStylus {
case ipts::protocol::dft::Type::Pressure:
this->handle_pressure(dft);
break;
case ipts::protocol::dft::Type::PositionMPP_2:
this->handle_position_mpp_2(dft);
break;
case ipts::protocol::dft::Type::BinaryMPP_2:
this->handle_dft_binary_mpp_2(dft);
break;
default:
// Ignored
break;
Expand Down Expand Up @@ -171,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] +
Expand Down Expand Up @@ -206,11 +228,72 @@ class DftStylus {
m_stylus.contact = true;
m_stylus.pressure = std::clamp(p, 0.0, 1.0);
} else {
m_stylus.contact = false;
m_stylus.contact = m_mppv2_in_contact.value_or(false);
m_stylus.pressure = 0;
}
}

/*!
* 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.
Comment on lines +237 to +239
Copy link
Member

Choose a reason for hiding this comment

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

From your notes I gather that this can't be used with MPP v1 pens because it contains garbage?

Is there any way to detect if the data inside is usable, and adjust the behaviour of the DFT parser accordingly? Dropping the need for a config option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any way to detect if the data inside is usable, and adjust the behaviour of the DFT parser accordingly? [...] I don't really like the idea of having to make the model / type of the pen a config option

Hmm, yeah, I'm not a fan of it either, because it means you can't use a mpp v1 pen without changing the config.

It may actually be possible... if a v1 pen is present, the magnitudes for the 0x0a frame are very small (100s). So what we could do I guess is check if the signal in the 0x0a frame is above a threshold (50000); if so, it trumps the button detection from the button frame.

Let me do a bit more exploration around this to see if I can make it work without explicit configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I think this is possible, we can use a std::optional<bool> to specify whether the v2 signal has been detected.

I do think it is a good idea to allow different minimum magnitudes for the v2 button vs the v1 button detection though, the scales are completely different;

SP, without touching button.
mag4 and 5: 3668922    69
Button mag x and y: 4321    25085
mag4 and 5: 3583622    54
Button mag x and y: 4930    24469
mag4 and 5: 3627867    34
Button mag x and y: 5128    25154

SP, 5mm, going into button
Button mag x and y: 4    16
mag4 and 5: 333657    2
Button mag x and y: 39464    33930
mag4 and 5: 26    333492
Button mag x and y: 41810    34442

SP, touching, going into button
Button mag x and y: 5    32
mag4 and 5: 3980021    61
Button mag x and y: 308425    521441
mag4 and 5: 114    3756423

m1, 5mm, going into button
Button mag x and y: 5    0
mag4 and 5: 20    15
Button mag x and y: 26065    26065
mag4 and 5: 8    10
Button mag x and y: 23953    22730

m1, 5mm
Button mag x and y: 17945    16769
mag4 and 5: 70    155
Button mag x and y: 19109    16840
mag4 and 5: 127    37
Button mag x and y: 19825    16160
mag4 and 5: 43    19

m1, touching
Button mag x and y: 434394    273589
mag4 and 5: 42    31
Button mag x and y: 558101    235801
mag4 and 5: 29    74

m2, touching
Button mag x and y: 275221    193257
mag4 and 5: 8502    2500942
Button mag x and y: 276692    186628
mag4 and 5: 8338    2490670

If we use the same threshold, I can only detect my m1 pen's button when it is super close to the screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, this is now implemented under 772fbaf.

I'll need to give it a bit more thorough testing, but from the data I see no reason why this shouldn't work. And brief tests with all four pens show that this works, it does allow me to just switch between the SP, M1, M2 and CN pens without any problems in regards to button detection.

Great idea to just detect this, I did add different magnitude detection for the v2 signals to the configuration, because they do have a very different amplitude from the v1 button signal, but that's still a lot better than having to set a boolean.

*/
void handle_dft_binary_mpp_2(const ipts::DftWindow &dft)
{
if (dft.x.size() <= 5) { // not sure if this can happen?
return;
}

// 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;

// Clearing the state in case we can't determine it.
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.
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_mpp2_button_min_mag;

if (mag_4 < threshold && mag_5 < threshold) {
// 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_mppv2_button_or_eraser = 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_position_mpp_2(const ipts::DftWindow &dft)
{
// 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;
}

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_mpp2_contact_min_mag;
if (mag_2 < threshold && mag_3 < threshold) {
// Not enough signal to make a decision.
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.
*
Expand Down Expand Up @@ -335,6 +418,9 @@ class DftStylus {
m_stylus.contact = false;
m_stylus.button = false;
m_stylus.rubber = false;

m_mppv2_in_contact = std::nullopt;
m_mppv2_button_or_eraser = std::nullopt;
}
};

Expand Down
10 changes: 10 additions & 0 deletions src/core/linux/config-loader.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const ipts::Metadata> &metadata)
: m_info {info}
Expand Down Expand Up @@ -56,6 +58,9 @@ class ConfigLoader {
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.");
}

/*!
Expand Down Expand Up @@ -127,6 +132,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)
Expand Down Expand Up @@ -169,12 +176,15 @@ 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);
this->get(ini, "Contacts", "SizeThreshold", m_config.contacts_size_thresh_max);

// clang-format on
m_loaded_config = true;
}

/*!
Expand Down
8 changes: 5 additions & 3 deletions src/ipts/protocol/dft.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ constexpr u8 MAX_ROWS = 16;
constexpr u8 PRESSURE_ROWS = 6;

enum class Type : u8 {
Position = 6,
Button = 9,
Pressure = 11,
Position = 0x6,
PositionMPP_2 = 0x7,
Button = 0x9,
BinaryMPP_2 = 0xA,
Pressure = 0xB,
};

struct [[gnu::packed]] Metadata {
Expand Down