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

[android] Added pressure support for stylus. #49609

Closed
wants to merge 1 commit into from

Conversation

ModProg
Copy link
Contributor

@ModProg ModProg commented Jun 14, 2021

Fixes godotengine/godot-proposals#735 for android.

I verified that this works on 3.2.2 and builds for master,
as I was not able to run the master godot engine.

There are no tests as I didn't find anything about tests for android.

@ModProg ModProg requested a review from a team as a code owner June 14, 2021 19:49
@Calinou
Copy link
Member

Calinou commented Jun 14, 2021

There are no tests as I didn't find anything about tests for android.

We don't have an Android test suite yet, but I don't know how such a test suite would work anyway.

@Calinou Calinou added enhancement platform:android topic:input topic:porting cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Jun 14, 2021
@Calinou Calinou added this to the 4.0 milestone Jun 14, 2021
@ModProg
Copy link
Contributor Author

ModProg commented Jun 14, 2021

I just noticed that this does not trigger "MouseButton" events for the stylus and no InputEventScreenTouch either.

@akien-mga
Copy link
Member

CC @HEAVYPOLY if you can test.

@ModProg
Copy link
Contributor Author

ModProg commented Jun 14, 2021

I just noticed that this does not trigger "MouseButton" events for the stylus and no InputEventScreenTouch either.

I investigated that a little bit, and it seams like it dosn't fire any other event except move for my stylus (Samsung Note 10 lite) so a MouseButton event would need to be emulated.

But I'm very new to the codebase so maybe I overlooked something:
I returned a MouseButton event with the button_index set to event_action for all event_actions in process_mouse_event, but I only got 2 for move and 0 on double_tapping with the stylus.

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 17, 2021

I just noticed that this does not trigger "MouseButton" events for the stylus and no InputEventScreenTouch either.

I investigated that a little bit, and it seams like it dosn't fire any other event except move for my stylus (Samsung Note 10 lite) so a MouseButton event would need to be emulated.

But I'm very new to the codebase so maybe I overlooked something:
I returned a MouseButton event with the button_index set to event_action for all event_actions in process_mouse_event, but I only got 2 for move and 0 on double_tapping with the stylus.

@thebestnom For additional review.

@ModProg In order to debug why no other events are fired, it may be easier to write a sample android app that just logs all the input from the stylus. Using that app, once you figure out how to properly identify stylus input events, then you can replicate the logic in the Godot codebase.
Unfortunately, I don't have a stylus available, so I won't be able to assist with troubleshooting.

@thebestnom
Copy link
Contributor

Interesting, sounds like the same bug I had with mouse that I had to hack around, Ill test that later today maybe I was a complete fool, or we need the same hack for stylus too

@thebestnom
Copy link
Contributor

@m4gr3d Looks good! The only thing Im trying to understand is in that case why we need special case for stylus in onGenericMotionEvent

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 25, 2021

@m4gr3d Looks good! The only thing Im trying to understand is in that case why we need special case for stylus in onGenericMotionEvent

@thebestnom Yea I agree, in theory we shouldn't need special casing.
@ModProg Any luck on figuring out the issues with detecting stylus events?

@ModProg
Copy link
Contributor Author

ModProg commented Jun 28, 2021

@ModProg Any luck on figuring out the issues with detecting stylus events?

So I created a test app (https://github.com/ModProg/PenTest)
These are the different Events I got:

Tap Move
Finger
grafik grafik
Pen Normal
grafik grafik
Pen with button (eraser)
grafik grafik

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 28, 2021

@ModProg The Pen with button screenshot may point to the cause of the issue you're seeing.
In your code you're using MotionEvent::toString() and both this method and most of the logic in the GodotInputHandler use MotionEvent::getAction() to retrieve the action.

We may instead need to start using MotionEvent::getActionMasked() to get the correct value.
Can you add a print statement to display the value for getActionMasked() in your test app and see if you get expected values instead of 211, 212 and 213.

Note: You can use MotionEvent::actionToString() in order to return the action corresponding name.

@ModProg
Copy link
Contributor Author

ModProg commented Jun 28, 2021

@m4gr3d this doesn't change anything still get 211,212,213. Those may be proprietary from Samsung: Samsung/ChromiumGStreamerBackend

We should still support those tho.

@HEAVYPOLY
Copy link
Contributor

@ModProg I would like to test the 3.2 version but I'm still a beginner with github. I have my own branch that is modified and based on 3.x godot branch.

Should I clone your branch, rebase on 3.x godot, then rebase my branch ontop of that?

@ModProg
Copy link
Contributor Author

ModProg commented Jul 3, 2021

@HEAVYPOLY yes you can do that, use this branch: https://github.com/ModProg/godot/tree/android-stylus-support

that is the 3.2 version of my fix, currently using this as well.

@HEAVYPOLY
Copy link
Contributor

@ModProg semi success, I'm getting the InputEventMouseMotion events with pressure! But not getting InputEventMouseButton events. (No mouse button pressed events)

@thebestnom
Copy link
Contributor

Sounds correct, Im pretty sure you forgot to change onGenericMotionEvent to fire mouse event

@HEAVYPOLY
Copy link
Contributor

HEAVYPOLY commented Jul 4, 2021

is it this section? case AMOTION_EVENT_ACTION_BUTTON_PRESS:

	int event_buttons_mask = _android_button_mask_to_godot_button_mask(event_android_buttons_mask);
	switch (event_action) {
		case AMOTION_EVENT_ACTION_BUTTON_PRESS:
		case AMOTION_EVENT_ACTION_BUTTON_RELEASE: {
			Ref<InputEventMouseButton> ev;
			ev.instance();
			_set_key_modifier_state(ev);
			ev->set_position(event_pos);
			ev->set_global_position(event_pos);
			ev->set_pressed(event_action == AMOTION_EVENT_ACTION_BUTTON_PRESS);
			int changed_button_mask = buttons_state ^ event_buttons_mask;

			buttons_state = event_buttons_mask;

			ev->set_button_index(_button_index_from_mask(changed_button_mask));
			ev->set_button_mask(event_buttons_mask);
			input->parse_input_event(ev);
		} break;

		case AMOTION_EVENT_ACTION_MOVE: {
			Ref<InputEventMouseMotion> ev;
			ev.instance();
			_set_key_modifier_state(ev);
			ev->set_position(event_pos);
			ev->set_global_position(event_pos);
			ev->set_relative(event_pos - hover_prev_pos);
			ev->set_button_mask(event_buttons_mask);
      ev->set_pressure(pressure);
			input->parse_input_event(ev);
			hover_prev_pos = event_pos;
		} break;```

@thebestnom
Copy link
Contributor

No, I meant on the Java side, the native side should be ok

@thebestnom
Copy link
Contributor

In GodotInputHandler in onGenericMotionEvent there is a case for stylus, Im not fully remember what happens there but Im pretty sure it doesn't fire mouse event because I think it relies on touch to send normal touch event (which not sent in the on touch)

@HEAVYPOLY
Copy link
Contributor

looks like it fires godotlib.hover. should it just be lumped in with mouse in this case since it should behave same as mouse( hover is just mousemovement )?

@@ -170,7 +170,7 @@ public void run() {

public boolean onTouchEvent(final MotionEvent event) {
// Mouse drag (mouse pressed and move) doesn't fire onGenericMotionEvent so this is needed
if (event.isFromSource(InputDevice.SOURCE_MOUSE)) {
if (event.isFromSource(InputDevice.SOURCE_MOUSE) || event.isFromSource(InputDevice.SOURCE_STYLUS)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This means that now onGenericMotionEvent need to do a the work except for ACTION_MOVE, and because you didn't implemented touch there only move works for now

@thebestnom
Copy link
Contributor

thebestnom commented Jul 4, 2021

#49609 (comment)
As I said previously, I think we should merge the implementation, yes 😄
(Same for hover mostly)

@thebestnom
Copy link
Contributor

That's really weird and it makes bo sense to me 😅
If the log is there the event should too

@thebestnom
Copy link
Contributor

Maybe try printing event.getSource because that the only thing that filtered by the native side? Or maybe AMOTION_EVENT_ACTION_POINTER_DOWN wasn't added to the native side?

@HEAVYPOLY
Copy link
Contributor

HEAVYPOLY commented Jul 5, 2021

got it working! Sortof. The issue was button mask defaulting to 0 for pen. Made the default of _button_index_from_mask() return BUTTON_LEFT instead of 0, now it works as left mouse.

void OS_Android::process_mouse_event(int event_action, int event_android_buttons_mask, Point2 event_pos, float event_vertical_factor, float event_horizontal_factor, float pressure) {
	int event_buttons_mask = _android_button_mask_to_godot_button_mask(event_android_buttons_mask);
	switch (event_action) {
		case AMOTION_EVENT_ACTION_DOWN:
		case AMOTION_EVENT_ACTION_UP:
		case AMOTION_EVENT_ACTION_POINTER_DOWN:
		case AMOTION_EVENT_ACTION_POINTER_UP:
		case AMOTION_EVENT_ACTION_BUTTON_PRESS:
		case AMOTION_EVENT_ACTION_BUTTON_RELEASE: {
			Ref<InputEventMouseButton> ev;
			ev.instance();
			_set_key_modifier_state(ev);
			ev->set_position(event_pos);
			ev->set_global_position(event_pos);
			ev->set_pressed(event_action == AMOTION_EVENT_ACTION_BUTTON_PRESS || event_action == AMOTION_EVENT_ACTION_DOWN || event_action == AMOTION_EVENT_ACTION_POINTER_DOWN);
			int changed_button_mask = buttons_state ^ event_buttons_mask;
			buttons_state = event_buttons_mask;
			ev->set_button_index(_button_index_from_mask(changed_button_mask));
			ev->set_button_mask(event_buttons_mask);
			input->parse_input_event(ev);
                        hover_prev_pos = event_pos;
		} break;

@thebestnom
Copy link
Contributor

thebestnom commented Jul 5, 2021

Well, in android it does count as nothing in the mouse is clicked, I suggest changing this in the java side in an if source stylus statement
Good job 👏👏

@HEAVYPOLY
Copy link
Contributor

HEAVYPOLY commented Jul 5, 2021

what do you think of this?

  int OS_Android::_button_index_from_mask(int button_mask) {
	  switch (button_mask) {
		  case BUTTON_MASK_LEFT:
			  return BUTTON_LEFT;
		  case BUTTON_MASK_RIGHT:
			  return BUTTON_RIGHT;
		  case BUTTON_MASK_MIDDLE:
			  return BUTTON_MIDDLE;
		  case BUTTON_MASK_XBUTTON1:
			  return BUTTON_XBUTTON1;
		  case BUTTON_MASK_XBUTTON2:
			  return BUTTON_XBUTTON2;
		  default:
			  return BUTTON_LEFT;  //Changed from return 0 <---
	  }

@ModProg
Copy link
Contributor Author

ModProg commented Jul 5, 2021

@HEAVYPOLY do you have your version on Github? Then I would merge that here and try with my phone as well.

@HEAVYPOLY
Copy link
Contributor

@ModProg https://github.com/HEAVYPOLY/godot/tree/ui_scale_3x
the last commit is all the android pen stuff, there is other unrelated stuff in the branch though.

@thebestnom
Copy link
Contributor

How is it going?

@ModProg
Copy link
Contributor Author

ModProg commented Aug 2, 2021

@thebestnom currently a little busy with live stuff, but I tried experimenting with @HEAVYPOLY's changes. should be possible to get this in a mergable state soon, but I probably need to get the mouse down events in here first.

@thebestnom
Copy link
Contributor

Need any help? Got time to burn

@ModProg
Copy link
Contributor Author

ModProg commented Aug 3, 2021

@thebestnom If you want to finish this up I think it's mostly getting Mouse-down events working and supporting these strange Samsung codes: #49609 (comment)
I probably won't be able to work on this for atleast another week or so.
I hope my changes should be self explanatory if not ask away ;)

@ModProg
Copy link
Contributor Author

ModProg commented Aug 16, 2021

I just noticed something, is it correct that MotionEvent.ACTION_SCROLL does not return true?

case MotionEvent.ACTION_SCROLL: {
final float x = event.getX();
final float y = event.getY();
final int buttonsMask = event.getButtonState();
final int action = event.getAction();
final float verticalFactor = event.getAxisValue(MotionEvent.AXIS_VSCROLL);
final float horizontalFactor = event.getAxisValue(MotionEvent.AXIS_HSCROLL);
GodotLib.touch(event.getSource(), action, 0, 1, new float[] { 0, x, y }, buttonsMask, verticalFactor, horizontalFactor);
}

@ModProg
Copy link
Contributor Author

ModProg commented Aug 16, 2021

@thebestnom I'll map the Stylus events to Left and Right click for now? I do think that we should add actual stylus buttons in the future to differentiate mouse and stylus not only on android.

@thebestnom
Copy link
Contributor

I usually test on windows to see the event are identical 😅

@ModProg
Copy link
Contributor Author

ModProg commented Aug 16, 2021

@thebestnom currently don't have a Windowstablet availible.

I have a working version with the sidebutton mapped to rightclick on here https://github.com/ModProg/godot/pull/1/files it is compared to the 3.3 branch as that is where I was able to actually test it. If that lookes ok to you I'll rebase it on master and get it in here.

Due to #51737 right click will map to 0.

@HEAVYPOLY
Copy link
Contributor

HEAVYPOLY commented Oct 29, 2021

I've been trying to refactor this for 3.x branch for the past week and have run up against a wall. I'm getting the Log prints from ACTION_MOVE with correct pressure, but I'm not seeing the pressure in Godot engine (returns pressure of 1) and also not seeing the pen down and pen up events. Does anyone know what is wrong here?

https://github.com/HEAVYPOLY/godot/tree/android-pen-modprog

	private boolean handleMouseEvent(final MotionEvent event) {
		// Log.i(tag, "HANDLE MOUSEY " + event.getSource());
		switch (event.getActionMasked()) {
			case MotionEvent.ACTION_HOVER_ENTER:
			case MotionEvent.ACTION_HOVER_MOVE:
			case MotionEvent.ACTION_HOVER_EXIT: {
				Log.i(tag, "STYLUS MOUSE HOVER INSIDE HANDLEMOUSE EVENT");
				final float x = event.getX();
				final float y = event.getY();
				final int type = event.getAction();
				GodotLib.hover(type, x, y);
				return true;
			}
			case MotionEvent.ACTION_DOWN:
			case MotionEvent.ACTION_UP:
			case MotionEvent.ACTION_POINTER_DOWN:
			case MotionEvent.ACTION_POINTER_UP:
			case MotionEvent.ACTION_BUTTON_PRESS:
			case MotionEvent.ACTION_BUTTON_RELEASE: {
				final float x = event.getX();
				final float y = event.getY();
				final int buttonsMask = event.getButtonState();
				final int action = event.getAction();
				Log.i(tag, "STYLUS MOUSE TOUCH INSIDE HANDLEMOUSE EVENT");
				GodotLib.touch(event.getSource(), action, 0, 1, new float[] { 0, x, y }, buttonsMask);
				return true;
			}
			case MotionEvent.ACTION_MOVE: {
				final float x = event.getX();
				final float y = event.getY();
				final int buttonsMask = event.getButtonState();
				final int action = event.getAction();
				final float pressure = event.getPressure();
				Log.i(tag, "STYLUS MOUSE TOUCH ACTION: " + action + " BUTTON STATE : " + buttonsMask + " PRESSURE " + pressure);
				GodotLib.touch(event.getSource(), action, 0, 1, new float[] { 0, x, y }, buttonsMask, pressure);
				return true;
			}
			case MotionEvent.ACTION_SCROLL: {
				final float x = event.getX();
				final float y = event.getY();
				final int buttonsMask = event.getButtonState();
				final int action = event.getAction();
				final float verticalFactor = event.getAxisValue(MotionEvent.AXIS_VSCROLL);
				final float horizontalFactor = event.getAxisValue(MotionEvent.AXIS_HSCROLL);
				GodotLib.touch(event.getSource(), action, 0, 1, new float[] { 0, x, y }, buttonsMask, verticalFactor, horizontalFactor);
			}
			default: {
				return true;
			}
		}
		// return false;
	}
}

@ModProg
Copy link
Contributor Author

ModProg commented Oct 31, 2021

@HEAVYPOLY did it work at some point? And did https://github.com/ModProg/godot/tree/android-stylus-support work for you?

Does your current version build? I got

> Task :java:lib:compileReleaseJavaWithJavac FAILED
/home/modprog/Dokumente/Development/FOSS/GoDot/engine/platform/android/java/lib/src/org/godotengine/godot/GodotView.java:101: error: cannot find symbol
                Log.i(tag, " ON TOUCH EVENT ");
                      ^
  symbol:   variable tag
  location: class GodotView
/home/modprog/Dokumente/Development/FOSS/GoDot/engine/platform/android/java/lib/src/org/godotengine/godot/GodotView.java:120: error: cannot find symbol
                Log.i(tag, " ON GENERIC EVENT ");
                      ^
  symbol:   variable tag
  location: class GodotView
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
2 errors

> Task :generateGodotTemplates FAILED

> Task :zipCustomBuild
Generating Godot custom build template

FAILURE: Build failed with an exception.

@HEAVYPOLY
Copy link
Contributor

HEAVYPOLY commented Nov 1, 2021

@ModProg should it be like this instead?

	} else if ((input_device & AINPUT_SOURCE_STYLUS) == AINPUT_SOURCE_STYLUS) {
		// input_handler->process_mouse_event(ev, buttons_mask, points[0].pos, vertical_factor, horizontal_factor, pressure);
		input_handler->process_mouse_event(ev, buttons_mask, points[0].pos, pressure, vertical_factor, horizontal_factor);

I'm trying to match this:

		
void AndroidInputHandler::process_mouse_event(int event_action, int event_android_buttons_mask, Point2 event_pos, float event_vertical_factor, float event_horizontal_factor, float pressure) {
	int event_buttons_mask = _android_button_mask_to_godot_button_mask(event_android_buttons_mask);
	switch (event_action) {
		case AMOTION_EVENT_ACTION_BUTTON_PRESS:
		case AMOTION_EVENT_ACTION_BUTTON_RELEASE: {
			Ref<InputEventMouseButton> ev;
			ev.instance();
			_set_key_modifier_state(ev);
			ev->set_position(event_pos);
			ev->set_global_position(event_pos);
			ev->set_pressed(event_action == AMOTION_EVENT_ACTION_BUTTON_PRESS);
			int changed_button_mask = buttons_state ^ event_buttons_mask;

			buttons_state = event_buttons_mask;

			ev->set_button_index(_button_index_from_mask(changed_button_mask));
			ev->set_button_mask(event_buttons_mask);
			input->parse_input_event(ev);
		} break;
		case AMOTION_EVENT_ACTION_MOVE: {
			Ref<InputEventMouseMotion> ev;
			ev.instance();
			_set_key_modifier_state(ev);
			ev->set_position(event_pos);
			ev->set_global_position(event_pos);
			ev->set_relative(event_pos - hover_prev_pos);
			ev->set_button_mask(event_buttons_mask);
			ev->set_pressure(pressure);
			input->parse_input_event(ev);
			hover_prev_pos = event_pos;
		} break;
		case AMOTION_EVENT_ACTION_SCROLL: {
			Ref<InputEventMouseButton> ev;
			ev.instance();
			_set_key_modifier_state(ev);
			ev->set_position(event_pos);
			ev->set_global_position(event_pos);
			ev->set_pressed(true);
			buttons_state = event_buttons_mask;
			if (event_vertical_factor > 0) {
				_wheel_button_click(event_buttons_mask, ev, BUTTON_WHEEL_UP, event_vertical_factor);
			} else if (event_vertical_factor < 0) {
				_wheel_button_click(event_buttons_mask, ev, BUTTON_WHEEL_DOWN, -event_vertical_factor);
			}

			if (event_horizontal_factor > 0) {
				_wheel_button_click(event_buttons_mask, ev, BUTTON_WHEEL_RIGHT, event_horizontal_factor);
			} else if (event_horizontal_factor < 0) {
				_wheel_button_click(event_buttons_mask, ev, BUTTON_WHEEL_LEFT, -event_horizontal_factor);
			}
		} break;
	}
}

I had this all working for a while but must have forgotten to commit...now trying to get back to where I was. I tried your android-stylus-support but was getting some with queue_event

@HEAVYPOLY
Copy link
Contributor

HEAVYPOLY commented Nov 1, 2021

Ahh nevermind, I see in your branch the solution of 2 placeholder ints for vertical and horizontal factor.

It worked! Getting the pressure mousemotion events again finally!
Now need to work on press and release events, for some reason they show up as EventPanGestures. Will keep digging.

@HEAVYPOLY
Copy link
Contributor

HEAVYPOLY commented Nov 1, 2021

Ended up using your handlesylusevent function, removing queue event and it worked for me, thanks @ModProg

@ModProg
Copy link
Contributor Author

ModProg commented Nov 1, 2021

Ended up using your handlesylusevent function, removing queue event and it worked for me, thanks @ModProg

No problem, I really want this to happen, but sadly don't have the time currently to finalize this.

If there is anything I can help with to make this a thing let me know.

@Calinou
Copy link
Member

Calinou commented Aug 15, 2023

Thanks for the contribution nonetheless 🙂

@Calinou Calinou closed this Aug 15, 2023
@Calinou Calinou added archived and removed cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Aug 15, 2023
@AThousandShips AThousandShips removed this from the 4.x milestone Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add iOS Android pen pressure / tilt support
7 participants