Skip to content
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.

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

Release of touch input device results in wrong coordinates while using "invert-x/y" (lvgl-pointer-input) due to double inversion of coordinates #70539

Closed
faxe1008 opened this issue Mar 21, 2024 Discussed in #70500 · 2 comments
Assignees
Labels
area: Input Input Subsystem and Drivers area: LVGL Light and Versatile Graphics Library Support bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@faxe1008
Copy link
Collaborator

Discussed in #70500

Originally posted by Benni77 March 20, 2024
I'm wondering if this is a bug with request for confirmation before I open an issue:

I'm using a gc9a01 display with CST816S touch controller with lvgl.
When I use the lvgl Dropdown Widget the selection of the list options is not working correctly.
Digging into it made me realize this only happens if invert-x or invert-y (propably with swap-xy as well) configuration is activated in zephyr,lvgl-pointer-input device:

lvgl_pointer {
     compatible = "zephyr,lvgl-pointer-input";
     input = <&cst816s>;
     invert-x;
     invert-y;
};

So i digged deeper:
In the input drivers input_cst816s.c processing function (cst816s_process()) a input is reported on release of the touch device in the else statement:

	if (pressed) {
		input_report_abs(dev, INPUT_ABS_X, col, false, K_FOREVER);
		input_report_abs(dev, INPUT_ABS_Y, row, false, K_FOREVER);
		input_report_key(dev, INPUT_BTN_TOUCH, 1, true, K_FOREVER);
	} else {
	        input_report_key(dev, INPUT_BTN_TOUCH, 0, true, K_FOREVER);
	}

Now in the lvgl_pointer_process_event() function in the lvgl_pointer_input.c file the coordinates of the input event get inverted:

	if (cfg->invert_x) {
		if (cap->current_orientation == DISPLAY_ORIENTATION_NORMAL ||
		    cap->current_orientation == DISPLAY_ORIENTATION_ROTATED_180) {
			point->x = cap->x_resolution - point->x;
		} else {
			point->x = cap->y_resolution - point->x;
		}
	}

	if (cfg->invert_y) {
		if (cap->current_orientation == DISPLAY_ORIENTATION_NORMAL ||
		    cap->current_orientation == DISPLAY_ORIENTATION_ROTATED_180) {
			point->y = cap->y_resolution - point->y;
		} else {
			point->y = cap->x_resolution - point->y;
		}
	}

Problem: Since this inversion also happens on the "RELEASE" event without new coordinates passed by the input device this results in a double inversion of the last coordinates: First when new coordinates are passed, second when "RELEASE" is passed.

Result: Since the Dropdown widget uses the double inverted coordinates from the "RELEASE" event, this unfortunately results in a malfunction of the widget. The list items can not be selected correctly.

For me it looks like a bug in the lvgl_pointer_process_event() function, which should not invert on "RELEASE" event, what do you guys think?

Cheers
Benjamin

@faxe1008 faxe1008 self-assigned this Mar 21, 2024
faxe1008 added a commit to faxe1008/zephyr that referenced this issue Mar 21, 2024
This patch fixes two issue in the coordinate handling of the
`zephyr,lvgl-pointer-input` compatible:
- If the swap-xy flag is set the coordinates need to be swapped even
  before the sync event is received.
- If the invert-{x,y} property is set the coordinates should only be
  inverted when the state is pressed. This is because the pointer saves the
  coordinates of a previous press event and upon release event it reuses
  them, causing the inversion to be applied twice.

Resolves issue zephyrproject-rtos#70539.

Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>
@faxe1008 faxe1008 added area: LVGL Light and Versatile Graphics Library Support area: Input Input Subsystem and Drivers labels Mar 21, 2024
@nashif nashif added bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug labels Mar 26, 2024
@fabiobaltieri
Copy link
Member

Yeah it's a bug in no prob that is , thanks for debugging it.

I'm thinking that the easiest way of handling this would be to refactor the point inversion and rotation into the switch statement, so that the data only gets processed when the raw point arrives. @faxe1008 what do you think?

faxe1008 added a commit to faxe1008/zephyr that referenced this issue Apr 24, 2024
This patch fixes two issues in the coordinate handling of the
`zephyr,lvgl-pointer-input` compatible:
- If the swap-xy flag is set the coordinates need to be swapped even
  before the sync event is received.
- The coordinates are stored into an additional variable instead of the
  pending_event. This is done to prevent the double inversion for touch
  release events.

Resolves issue zephyrproject-rtos#70539.

Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>

modules: lvgl

Will be squashed if ok
faxe1008 added a commit to faxe1008/zephyr that referenced this issue Apr 30, 2024
This patch fixes two issues in the coordinate handling of the
`zephyr,lvgl-pointer-input` compatible:
- If the swap-xy flag is set the coordinates need to be swapped even
  before the sync event is received.
- The coordinates are stored into an additional variable instead of the
  pending_event. This is done to prevent the double inversion for touch
  release events.

Resolves issue zephyrproject-rtos#70539.

Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>
carlescufi pushed a commit that referenced this issue Apr 30, 2024
This patch fixes two issues in the coordinate handling of the
`zephyr,lvgl-pointer-input` compatible:
- If the swap-xy flag is set the coordinates need to be swapped even
  before the sync event is received.
- The coordinates are stored into an additional variable instead of the
  pending_event. This is done to prevent the double inversion for touch
  release events.

Resolves issue #70539.

Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>
@faxe1008
Copy link
Collaborator Author

Resolved via #70541

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Input Input Subsystem and Drivers area: LVGL Light and Versatile Graphics Library Support bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

3 participants