Rotation works incorrectly at 0 deltas

[sdfjhdsjkgfhdfkj]

Prerequisites:

• Turn off any acceleration (set ACCELERATION_MODE to 0 in config.h).
• Enable mouse rotation by setting ROTATION_ANGLE in config.h.
• Don't rotate an actual mouse (keep it straight vertically (normally) on mousepad).

Steps to reproduce:

1. Move mouse either in horizontal or vertical straight line on mousepad.

Expected result:

Mouse cursor draws straight line.

Actual result:

Mouse cursor draws S-curve, zigzag or even straight line but such as if rotation weren't applied.

[AndyFilter]

Hey, I cannot reproduce this issue. Can You share the exact config with me?

Also, the config.h is not really meant to be configured by hand. The ACCELERATION_MODE set to 0 (AccelMode_Current) usually means an error on the driver side.

YeetMouse/shared_definitions.h

Line 7 in 851fbcb

AccelMode_Current = 0, // Mainly used in GUI, denotes lack of a curve on the driver side

[sdfjhdsjkgfhdfkj]

Here is the config file:

// Acceleration Mode
#define ACCELERATION_MODE AccelMode_Current

// Global Parameters
#define SENSITIVITY 0.1 // For compatibility this is named SENSITIVITY, but it really refers just to the X axis
#define SENSITIVITY_Y 0.1 // Only applied when this differs from the SENSITIVITY parameter
#define OUTPUT_CAP 0
#define INPUT_CAP 0
#define OFFSET 0
#define PRESCALE 1

// Angle Snapping (in radians)
#define ANGLE_SNAPPING_THRESHOLD 0 // 0 deg. in rad.
#define ANGLE_SNAPPING_ANGLE 0 // 1.5708 - 90 deg. in rad.

// Rotation (in radians)
#define ROTATION_ANGLE -0.366519

// LUT settings
#define LUT_SIZE 0
#define LUT_DATA 0

// Mode-specific parameters
#define ACCELERATION 0.15
#define MIDPOINT 2
#define MOTIVITY 1.5
#define EXPONENT 0.2
#define USE_SMOOTHING 1 // 1 - True, 0 - False

Also, is there a way to turn off acceleration via a GUI?

[AndyFilter]

Hey, I think I found the issue. When the mouse is moved exactly in one direction (for example horizontally) it doesn't send any events in the other direction (vertically), which means that in case of rotation it can't modify the other axis to apply the rotation.
This now has been fixed on the driver/experimental branch. Please let me know if the results are as expected now.

Also, is there a way to turn off acceleration via a GUI?

No, not really. It's a mouse acceleration software after all. But I can for example add a special case for the Linear mode with acceleration set to 0, so that it completely skips the acceleration.

[sdfjhdsjkgfhdfkj]

I have tried the fix from driver/experimental branch and now rotation works correctly at 0 deltas. Thank you very much! ❤️

But there is another issue: after rotation is applied and I move rotated mouse, for example, horizontally to the right, then cursor jumps up and draws a stairs instead of a horizontal line. This only happens when I move mouse very slowly. And I have noticed that subtracting carry from deltas before applying rotation resolves the issue. So, here is quick fix that works in my case, probably it would be helpful in resolving the stairs issue:

driver/accel.c:

int accelerate(int *x, int *y)
{
...
    // Apply Rotation before everything else to keep the precision
    if(g_RotationAngle != 0) {
        FP_LONG old_delta_x = FP64_Sub(delta_x, carry_x);
        FP_LONG old_delta_y = FP64_Sub(delta_y, carry_y);
        delta_x = FP64_Mul(old_delta_x, modesConst.cos_a) - FP64_Mul(old_delta_y, modesConst.sin_a);
        delta_y = FP64_Mul(old_delta_x, modesConst.sin_a) + FP64_Mul(old_delta_y, modesConst.cos_a);
    }
...
}

But I can for example add a special case for the Linear mode with acceleration set to 0, so that it completely skips the acceleration.

Yes, I would appreciate having non-acceleration mode!

[sdfjhdsjkgfhdfkj]

But my quick fix only partially resolves the issue with slow mouse movements. Because after applying it rotation doesn't work in such case: when mouse is put straight vertically (normally) on mousepad and I move it very slowly in horizontal or vertical direction.

[kitten]

I don't quite remember if rotation was around when I wrote that new input handler.

Just to chime in here, I've personally not encountered a mouse that doesn't send REL_X or REL_Y events. I assume though that this is extremely DPI, mouse, and input driver dependent, and that USBHID itself doesn't prohibit this. Typically, I'd expect that it's extremely difficult to replicate at high DPI without artificial inputs, but I'm guessing that depending on the DPI and sensor resolution, or with some kind of subtle angle snapping, that this is more frequent than I thought.

Personally, I use a really low rotation and high DPI with low sensitivity, which is why I probably haven't noticed this.

I believe we could try to insert the missing REL_* events artificially. Doesn't seem to me like it would cause any problems, unless I'm missing something?

Might get a bit tricky, and might only work on Linux >=6.11, but seems like we could

[AndyFilter]

after rotation is applied and I move rotated mouse, for example, horizontally to the right, then cursor jumps up and draws a stairs instead of a horizontal line.

I don't understand the expected behaviour in this scenario. When the rotation is applied the cursor is going to be drawing stairs when moved horizontally/vertically. That's what rotation does. If there's some odd behaviour when moving the mouse very slowly then I cannot reproduce it, in which case I'm gonna have to ask You to provide a quick video/gif of the problem in action.
What I've noticed is that the cursor is inaccurate when moving at slower speeds.

But I can't conclude much from this other than the fact that it's to be expected, because the resolution of the space containing possible points (pixels) is limited, and movement with rotation cannot always be quantized to fit to that space.

The code snippet You provided simply removes the rotation in cases where mouse didn't move fast enough (it discards the carry from the last event). Which kind of works like angle snapping. I don't think that's the expected behaviour.

Now to answer @kitten

I've personally not encountered a mouse that doesn't send REL_X or REL_Y events

Yea, I was also surprised, but I've debugged everything and my mouse simply omits the REL_Y event when it's not needed.

I believe we could try to insert the missing REL_* events artificially. Doesn't seem to me like it would cause any problems, unless I'm missing something?

This is exactly (I think) what I've done on the driver/experimental branch for this issue:

YeetMouse/driver/driver.c

Lines 127 to 138 in 8e85ae8

	if (x != NONE_EVENT_VALUE && !seen_x) {
	end->type = EV_REL;
	end->code = REL_X;
	end->value = x;
	end++;
	}
	if (y != NONE_EVENT_VALUE && !seen_y) {
	end->type = EV_REL;
	end->code = REL_Y;
	end->value = y;
	end++;
	}

(This apparently doesn't cause OOB write because of the discarded REL_* frame)

Might get a bit tricky, and might only work on Linux >=6.11

Thanks for pointing that out. I'll check this on an older system before merging.

[kitten]

Hm, interesting! I'm not entirely sure if this is indeed can never cause an OOB write, since:

• if an EV_REL is discarded,
• no event is skipped and no event is provided to be overwritten
• we're not necessarily receiving more events beyond count we can use

But I'm not sure how large that buffer is, so it's possible enough space is always provided and we can simply add a bounds check just in case? Minor detail though, since it's good enough to test this! 🙌 Awesome

[AndyFilter]

Well... I've tried to verify that and I wrote this:

            if (x != NONE_EVENT_VALUE && !seen_x) {
                if (end - vals >= count) {
                    printk("YeetMouse Warning: Cannot force another event! (count = %i)\n", count);
                }
                end->type = EV_REL;
                end->code = REL_X;
                end->value = x;
                end++;
            }

And the printk actually gets called, meaning that it's writing OOB from what I understand:

...
[10028.080514] YeetMouse Warning: Cannot force another event! (count = 2)
[10028.108514] YeetMouse Warning: Cannot force another event! (count = 2)
[10028.242523] YeetMouse Warning: Cannot force another event! (count = 2)
...

And count = 2 suggests that it only sent REL_X (or REL_Y) and REL_WHEEL. The problem is that I don't think I can reallocate (to expand) the vals buffer, can I? I'm worried it's some OS-managed kernel buffer that has to be exactly where it is.

UPDATE:
I've implemented a realloc and it seems to be working just fine, but I'm still unsure.

      if (x != NONE_EVENT_VALUE && !seen_x) {
          if (end - vals >= count) {
              void* _vals = krealloc_array(vals, count + 1, sizeof(struct input_value), GFP_KERNEL);
              if(!_vals) {
                  printk("YeetMouse: Failed to allocate memory for vals\n");
                  return -ENOMEM;
              }
              vals = _vals;
          }
          end->type = EV_REL;
          end->code = REL_X;
          end->value = x;
          end++;
      }

(I don't know if this function can return negative values (like -ENOMEM))

[AndyFilter]

Ok, The rotation is completely broken. At least at low speed values, it can be off on one of the axis by like 20%. For example with rotation at 45deg. when moving the mouse to the right, it would move 20% more down than to the side.
I've managed to separate the entire acceleration code from the driver and pass artificial inputs to it, and it was totally fine. After this I took a look at the driver code again, and it turns out by removing the if (__cleanup_events && x == NONE_EVENT_VALUE) from this (and 2 other occurrences):

YeetMouse/driver/driver.c

Lines 104 to 108 in 8e85ae8

	case REL_X:
	if (__cleanup_events && x == NONE_EVENT_VALUE)
	continue;
	v->value = x;
	break;

fixed all the issues with the rotation.

Now this is quite interesting, because I have no idea why. But, @kitten - do You remember what exactly this if was meant to do? I don't want to remove it just because it fixed this one issue, as it might create 10 more.

YeetMouse/driver/driver.c

Lines 15 to 19 in 8e85ae8

	#if (LINUX_VERSION_CODE < KERNEL_VERSION(6, 11, 0))
	#define __cleanup_events 0
	#else
	#define __cleanup_events 1
	#endif

Because I understand the need for __cleanup_events, but not why some events had to be skipped when it's set to true. Especially because the said events are Null, so it should do nothing on empty events.

[kitten]

That's a leftover and safe to remove. It was from old behaviour, so I don't think it's relevant anymore.

If you note the original definition it's a 0 not a NULL value but I believe the original code was sensitive to the first event being zero, so it was safer to skip the first incoming reported event. I don't think it's sensitive to that anymore and the check is already done at the top of the function

[AndyFilter]

Ok, I've committed the changes that should fix any issues with the rotation, but this still includes the questionable code that might be causing an OOB write. I've tested this on kernel 6.8 and it doesn't work, meaning that it still omits the missing event if it's not in the original frame. So this fix only works for kernel 6.11 and newer.
Also, You should now be able to apply no acceleration by setting the mode to Linear and setting the acceleration to 0.
(The code is on the driver/experimental branch)

Precision compared to RawAccel on that patch:

Apparently RawAccel is "really" off sometimes when using rotation.

[AndyFilter]

I've tested maccel just to make sure it's not something obvious we missed, but it's the same thing. The rotation is not being applied when moving the mouse in exactly one direction (and events are skipped).
I'll have to dig into this, but from what I've gathered so far it doesn't seem like something simple to fix, so You can use the current version from the driver/experimental branch and if I'll make any progress I'll let You know.