Bug 1348

Here is my pull request: https://github.com/sgothel/jogl/pull/102

Very good job and your pull request is quite clean :)

Thank you Danny, reviewing your pull request.
Excellent work.

The only issue I have is the usage of the static 'touch_coordinates' array,
which may lead to threading issues having multiple EDT threads on different X11 Displays running?

I will pull and merge your work and may add a fix to this.

commit 746383476aa449e9cab4a25df27be85b61aa074b

(Note to Danny: I cannot test this now - please re-test and/or review)
    
X11Common::JavaWindow
- Owns XI extension's xiOpcode, selected xiTouchDeviceId and tracked XITouchPosition array
    
X11Window::CreateWindow
- Query XI extension only once @ window creation and earmark xiOpcode in JavaWindow instance
    
- Fix: Device selection code was "class->type != XITouchClass",
  but shouldn't it be 'XITouchClass == class->type' (as patched here)
    
- Fix: Free XIQueryDevice returned XIDeviceInfo array via XIFreeDeviceInfo
    
- Earmark deviceid in JavaWindow instance
    
X11Display
- Moved global static touch_coordinates to JavaWindow::xiTouchCoords instance
    
X11Display::DispatchMessage
- Changed event handling structure similar to https://keithp.com/blogs/Cursor_tracking/
    
- Fix: Free XGetEventData's optional memory allocation via XFreeEventData
    
- Reuse JavaWindow's queried xiOpcode
    
- Fix: Don't overrise windowPointer, instead validate and require a match. JavaWindow must match!
    
- Fix: Also validate chosen deviceid with JavaWindow's registered device
    
Newt Build:
- Added libXi in build recipe and doc

Add more verbose DBG_PRINT
- @ CreateWindow: extension, scanning device/class, registered deviceid
- @ DispatchMessage: XI_TouchBegin, XI_TouchUpdate and XI_TouchEnd
    
On my test machine w/o a touchscreen I correctly:
- detected extension
- detected no XITouchClass device, hence no deviceid registered

This needs validation with a touchscreen now :)

X11: [CreateWindow]: XI: Window 0x6600016, Extension 131
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[1/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[2/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[3/13].class[1/3]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[3/13].class[2/3]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[3/13].class[3/3]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[4/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[5/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[6/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[7/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[8/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[9/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[10/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[11/13].class[1/1]: type 0 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[1/7]: type 1 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[2/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[3/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[4/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[5/7]: type 2 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[6/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[12/13].class[7/7]: type 3 (is XITouchClass 0)
X11: [CreateWindow]: XI: Scan Window 0x6600016, device[13/13].class[1/1]: type 0 (is XITouchClass 0)

Sven, thanks for the review and improvements.

I tested your changes and unfortunately it didn't work.

I got following warning:
Warning: NEWT X11 DisplayDispatch 0x7fba740018f0, Couldn't handle event 35 for X11 window 0x1300000083

I will take a deeper look to the problem in the next days.

Fix X11 XI Multitouch
    
I got access to a touchscreen laptop w/ Debian 9, hence I could fix and test the implementation.
    
X11 DisplayDriver.java:
- Store and pass through xi_opcode of XI extension, queried at initialization stage
    
X11Window.c Fixes:
- Initialize JavaWindow's xiTouchCoords[].id w/ -1, as required to track the pointer
    
- Pass through xi_opcode as stored in X11 DisplayDriver
    
X11Display.c Fixes:
- sendTouchScreenEvent: Throw RuntimeException if 0 > actionId (Internal Error: based on xiTouchCoords[].id tracking)
    
- DispatchMessages's windowPointer determination:
-- Query potenial XI Event first: IF XI Event, must use XIDeviceEvent's event Window
-- Only IF not an XI Event, we can use evt.xany.window as the event window
    
- DispatchMessages's XI Event Handling:
-- Always break deviceid search loop if id found, preserving index and time spend
    
Works on my Debian 9 device, tested w/ com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2NEWT:
- One pointer (finger) press, drag and release (click)
- PinchToZoomGesture works
- DoubleTabScrollGesture works
    
+++ 
    
Potential Issues:
    
JavaWindow's xiTouchCoords[].id accuracy is crucial to pointer tracking
during XI_TouchBegin -> XI_TouchUpdate -> XI_TouchEnd.
    
In the normal course of action:
- XI_TouchBegin sets the id, assuming it is yet set 
- XI_TouchUpdate assumes it is set 
- XI_TouchEnd clears the id, assuming it is set 
    
This field in the JavaWindow array only gets reset to -1 once at native window
creation. We may need to figure out when to reset this field to -1. 
    
If the XI_TouchEnd events would get lost for whatever reason,
the above tracking state would be broken.

+++

In case there are issues left and/or a good preemptive solution for above
potential issue is found: reopen and add the fix before 2.4.0 release.

Works for me too. Thank you!

In my opinion the potential issue is not so great. The implementation supports 10 finger multitouch. If one XI_TouchEnd is lost (because of a bug of X11 or the display driver?) there still stay 9 fingers ;)

Possible solution:
* struct XITouchPosition gets a new field which is set with the current timestamp on every XI_TouchBegin and XI_TouchUpdate event
* on every new event we check the stored timestamps. At timestamps older than 30 seconds we can assume the XI_TouchEnd event was lost and we reset the id to -1 (also we send EVENT_MOUSE_RELEASED to the application?)

(In reply to danny.koernig from comment #8)
> Works for me too. Thank you!
> 
> In my opinion the potential issue is not so great. The implementation
> supports 10 finger multitouch. If one XI_TouchEnd is lost (because of a bug
> of X11 or the display driver?) there still stay 9 fingers ;)
> 
> Possible solution:
> * struct XITouchPosition gets a new field which is set with the current
> timestamp on every XI_TouchBegin and XI_TouchUpdate event
> * on every new event we check the stored timestamps. At timestamps older
> than 30 seconds we can assume the XI_TouchEnd event was lost and we reset
> the id to -1 (also we send EVENT_MOUSE_RELEASED to the application?)

Personally, I'd prefer leaving the source code as it is right now as we're not sure that this problem will really occur and the suggested fix is messy. I think that the time stamp is already stored elsewhere.

(In reply to Julien Gouesse from comment #9)
> (In reply to danny.koernig from comment #8)
> > Works for me too. Thank you!
> > 
> > In my opinion the potential issue is not so great. The implementation
> > supports 10 finger multitouch. If one XI_TouchEnd is lost (because of a bug
> > of X11 or the display driver?) there still stay 9 fingers ;)
> > 
> > Possible solution:
> > * struct XITouchPosition gets a new field which is set with the current
> > timestamp on every XI_TouchBegin and XI_TouchUpdate event
> > * on every new event we check the stored timestamps. At timestamps older
> > than 30 seconds we can assume the XI_TouchEnd event was lost and we reset
> > the id to -1 (also we send EVENT_MOUSE_RELEASED to the application?)
> 
> Personally, I'd prefer leaving the source code as it is right now as we're
> not sure that this problem will really occur and the suggested fix is messy.
> I think that the time stamp is already stored elsewhere.

Agreed. We could handle this from the Java side and in case we need to reset the touch IDs issue a native call to X11Window.c.

Will let this idea keep floating around for a while .. 2.4.0 release is not yet due anyways.

Great we have this feature finally supported on X11 now,
thank you for your initial 'wall breaking' contribution!