[FFOS7715 v21.][touch]click event can not Response timely in particular page

RESOLVED FIXED

Status

Firefox OS
General
--
major
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jingmei.zhang, Assigned: viralwang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
STR:
a) Power ON 7715 FFOS.
b) Go to Marketplace -> Download Line app.
c) Register Line App.
d) Click on contact name -> chat
e) 7715 FFOS Line app fail to go to chat page
(Reporter)

Comment 1

3 years ago
Hi Shawn,

This might be a dolphin issue.

and I have checked the codes in gecko,something might wrong which lead to the click event can not generate.

in a normal case:
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_START
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_END
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_CANCEL
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_MOVE
in a abnormal case:
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_START
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_MOVE
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_END
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_CANCEL
I/* JINGMEI * ==>( 1717): TabChild::RecvRealTouchEvent NS_TOUCH_MOV

and in the abnormal case,we could catch the flag isTouchPrevented in function TabChild::RecvRealTouchEvent is true,this prevent the generage of click in function AsyncPanZoomController::ProcessPendingInputBlocks.

Can you help to check the issue in dolphin in your side?
Flags: needinfo?(sku)

Comment 2

3 years ago
Viral,
 Please help check this issue.
Flags: needinfo?(sku) → needinfo?(vwang)
(Assignee)

Comment 3

3 years ago
One found here is that dolphin report several touch points (even the position of x and y are exact same) when bug happened.

0001 014a 00000001
0000 0002 00000000
0000 0000 00000000
0003 0035 0000001b
0003 0036 0000028e
0000 0002 00000000
0000 0000 00000000
0003 0035 0000001b
0003 0036 0000028e
0000 0002 00000000
0000 0000 00000000
0003 0035 0000001b
0003 0036 0000028d
0000 0002 00000000
0000 0000 00000000

The reason that this bug is not so easy found in flame v2.1 is flame report only one point here
0003 0039 00000000
0003 0035 000000f5
0003 0036 00000315
0003 0030 00000029
0003 0032 00000029
0000 0000 00000000
0003 0039 ffffffff
0000 0000 00000000

And it looks like the 2.2 gecko/gaia fix this issue (or with even lower reproduce rate).
From previous experience, it may be risky to uplift since there are too many code changes.

My suggestion here is you can change the touch driver first to lower the reproduce rate, we can try to figure out if we can merge the latest code for this part.
Flags: needinfo?(vwang)
(Reporter)

Comment 4

3 years ago
Hi Viral,

Thank you so much for your Analysis :)

I have some thing confusion,can you help to give some instructions?

we also figure out that the move event which comes behind the start event is the main cause of the issue.
As we see in gecko,
>however, if this is the first touchmove after a touchstart,
>it is special in that preventDefault is allowed on it, so
>we must dispatch it to content even if nothing changed. we
>arbitrarily pick the first touch point to be the "changed"
>touch because firing an event with no changed events doesn't
> work

But for a normal tap in another page(such as tap a name of a contact),we could also get a move event behind start event,As the logs show:

>Start Event:
I/* JINGMEI * ==>( 1888): PresShell::DispatchTouchEvent aEvent->message 5200
I/* JINGMEI * ==>( 2145): TabChild::RecvRealTouchEvent NS_TOUCH_START

>Move Event:
I/* JINGMEI * ==>( 2145): TabChild::RecvRealTouchMoveEvent
I/* JINGMEI * ==>( 2145): PresShell::DispatchTouchEvent aEvent->message 5201
>I/* JINGMEI * ==>( 2145): PresShell::DispatchTouchEvent tmpStatus 0 -------this is the main different
I/* JINGMEI * ==>( 2145): PresShell::DispatchTouchEvent newEvent.mFlags.mMultipleActionsPrevented 0
I/* JINGMEI * ==>( 2145): PresShell::DispatchTouchEvent canPrevent 1
I/* JINGMEI * ==>( 2145): PresShell::DispatchTouchEvent preventDefault 0 ---------preventDefault 0 --------means this event will not be prevented,then tap is generated below
I/* JINGMEI * ==>( 2145): TabChild::RecvRealTouchEvent NS_TOUCH_MOVE
I/* JINGMEI * ==>( 1888): TabParent::RecvContentReceivedTouch
I/* JINGMEI * ==>( 1888): AsyncPanZoomController::HandleGestureEvent
I/* JINGMEI * ==>( 1888): AsyncPanZoomController::GenerateSingleTap
I/* JINGMEI * ==>( 2145): TabChild::RecvRealTouchMoveEvent
I/* JINGMEI * ==>( 2145): isTouchPrevented 0
I/* JINGMEI * ==>( 1888): RenderFrameParent HandleSingleTap
I/* JINGMEI * ==>( 1888): TabParent::HandleSingleTap
I/* JINGMEI * ==>( 2145): TabChild::RecvHandleSingleTap Handling single tap at (98.6667,174.667) on { l=4, p=2, v=4 } with 0xb35682e0 0xb3567240 0

and in this bug,we also catch a move behind start,as mentioned above,the main different is the flag 'tmpStatus' in PresShell::DispatchTouchEvent,it return 1 while issue in this bug occur.

Here is my question:
1.should not the move behind start be prevented? But see from the above case,it is not prevented,why?
2.Are there any different between the two move? one is work well,the other is prevented……
Flags: needinfo?(vwang)
(Reporter)

Comment 5

3 years ago
Hi Daidong,

Can you help to update your test in driver here?
Flags: needinfo?(luodd)

Comment 6

3 years ago
Hi Viral
I has changed the report rate of the driver, the test result as following


the default value of 7715
/* set report rate, about 70HZ */
ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 7);
the interval of tow events report are about 14133178 

I modify as following
ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 5);
the interval of tow events report are about 20157883,but the line test fail.

ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 3);
the interval of tow events report are about 29510000, If I test in hand , I can return chat continuous,But sometimes also fail。 If I test line and put the 7715 on table ,there is no improve,just fail。

ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 1);
the interval of tow events report are about 17848867, the time just become shorter, so line test fail。also sometimes driver can‘t report touch event when press the screen。

So, I think reducing the frequency which is not the root cause, also this can effect the experience of user。

I think the comment 4 is the important clue for us to resolve this bug。
Flags: needinfo?(luodd)
(Reporter)

Comment 7

3 years ago
(In reply to luodaidong from comment #6)
could we both use rate & the change of Coordinate as the judging condition to decide if we should report the move Event?

See from comment3,the Coordinate of the touch rarely changed when move is reported.

This is just a suggestion which need lots of investigation and research.If need any help,let me know.
(Assignee)

Comment 8

3 years ago
Hi Kats,

It looks like there are different behaviors in APZ controller touch gesture detection.
Not sure if you met similar bug before and can give us some suggestions.
I try to check the diff between v2.1 and v2.2, there are over 50 commits changes in APZ
Any suggestion will be help. Thank you.
Flags: needinfo?(vwang) → needinfo?(bugmail.mozilla)
(Assignee)

Comment 9

3 years ago
(In reply to luodaidong from comment #6)
> Hi Viral
> I has changed the report rate of the driver, the test result as following
> 
> 
> the default value of 7715
> /* set report rate, about 70HZ */
> ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 7);
> the interval of tow events report are about 14133178 
> 
> I modify as following
> ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 5);
> the interval of tow events report are about 20157883,but the line test fail.
> 
> ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 3);
> the interval of tow events report are about 29510000, If I test in hand , I
> can return chat continuous,But sometimes also fail。 If I test line and put
> the 7715 on table ,there is no improve,just fail。
> 
> ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 1);
> the interval of tow events report are about 17848867, the time just become
> shorter, so line test fail。also sometimes driver can‘t report touch event
> when press the screen。
> 
> So, I think reducing the frequency which is not the root cause, also this
> can effect the experience of user。
> 
> I think the comment 4 is the important clue for us to resolve this bug。

actually I think you should try to filter the touch event if the position of X and Y not change.
Not lower the report rate here.
It sounds to me based on comment 4 that the line app is calling preventDefault on the first touchmove event in some cases but not others. That is why the touch block gets prevented and we do not detect a tap gesture. It sounds to me like you should look in the line app touch event handler at that position to see what it is doing (you can inspect this with WebIDE). My guess is that you will find the line app does a prevent default if the touchmove is over a certain distance from the touchstart. In some cases your finger will move only slightly when you tap and that distance will be too small to trigger the preventDefault; in other cases your finger will move slightly more and it will trigger it.
Flags: needinfo?(bugmail.mozilla)
(Reporter)

Comment 11

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10)
Hi Kats,

Something need you to explain more :(
> if the touchmove is over a certain distance from the touchstart. In some
> cases your finger will move only slightly when you tap and that distance
> will be too small to trigger the preventDefault; 
first issue:
But see from comment3,there is no distance but preventDefault is triggered,??

>in other cases your finger will move slightly more and it will trigger it.
second issue:
You mean if the finger move in the screen,the preventDefault is true and nothing change,but if the finger doe not moves and move event is triggered,it will work ,Is that right?

A nearly 5/5 issue in dolphin,It will be really kind for you to check it in dolphin.

Our driver engineer had tried to prevent the report of move when position changed slightly,but the result does not so well(daidong may help to update the result later)
Flags: needinfo?(bugmail.mozilla)

Comment 12

3 years ago
(In reply to viral [:viralwang] from comment #9)
> (In reply to luodaidong from comment #6)
> > Hi Viral
> > I has changed the report rate of the driver, the test result as following
> > 
> > 
> > the default value of 7715
> > /* set report rate, about 70HZ */
> > ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 7);
> > the interval of tow events report are about 14133178 
> > 
> > I modify as following
> > ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 5);
> > the interval of tow events report are about 20157883,but the line test fail.
> > 
> > ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 3);
> > the interval of tow events report are about 29510000, If I test in hand , I
> > can return chat continuous,But sometimes also fail。 If I test line and put
> > the 7715 on table ,there is no improve,just fail。
> > 
> > ft5x0x_write_reg(FT5X0X_REG_PERIODACTIVE, 1);
> > the interval of tow events report are about 17848867, the time just become
> > shorter, so line test fail。also sometimes driver can‘t report touch event
> > when press the screen。
> > 
> > So, I think reducing the frequency which is not the root cause, also this
> > can effect the experience of user。
> > 
> > I think the comment 4 is the important clue for us to resolve this bug。
> 
> actually I think you should try to filter the touch event if the position of
> X and Y not change.
> Not lower the report rate here.

I set the report rate 7, just as default,and filter the position X and Y just positive and negative one。but test has some stragne result, sometime when I press the screen, there is no reaction,I saw the following log:
I/ActiveElementManager.cpp :==> (  781): Touch start, aCanBePan: 0
I/ActiveElementManager.cpp :==> (  781): Touch start, aCanBePan: 0
I/ActiveElementManager.cpp :==> (  781): Multiple fingers on-screen, clearing touch block state
I/ActiveElementManager.cpp :==> (  781): Cancelling task 0x0
I/ActiveElementManager.cpp :==> (  781): Resetting active from 0x0

also line test has hight failure rate。

I set  the report rate 3,and filter the position X and Y just all the same,Line test  failure rate is 1/10 or 1/5.

I set the report rate 3,and filter the position X and Y just positive and negative one, Line test failure rate is 1/20 or 1/30.

This modify method just low the failure rate ,but can't resolve this bug 。Also from the point of driver , it is just responsible for sample datas which are user finger touched。 The service logic is in gecko which is responsible for handling this。 I think there maybe the rootcause we have to focus. as comment 10 ,we will analyse the line app touch to find why。

Comment 13

3 years ago
I has updated the info you need
(In reply to jingmei.zhang from comment #11)
> > if the touchmove is over a certain distance from the touchstart. In some
> > cases your finger will move only slightly when you tap and that distance
> > will be too small to trigger the preventDefault; 
> first issue:
> But see from comment3,there is no distance but preventDefault is triggered,??

Sorry, I don't understand the data in comment 3. Can you explain what it means? It's just a bunch of random numbers to me.

> >in other cases your finger will move slightly more and it will trigger it.
> second issue:
> You mean if the finger move in the screen,the preventDefault is true and
> nothing change,but if the finger doe not moves and move event is
> triggered,it will work ,Is that right?

My hypothesis is that the line app is listening for touchmove events, and calls preventDefault on them in some scenarios. Whether or not touchmove events are sent to the app depends in large part on the sensitivity of the hardware and will vary from device to device. For example I have noticed that Hamachi/Buri always sends a touchmove between a touchstart and a touchend, even if your finger doesn't move. The Flame does not, though.

> A nearly 5/5 issue in dolphin,It will be really kind for you to check it in
> dolphin.

I don't have a Dolphin device, sorry.

> Our driver engineer had tried to prevent the report of move when position
> changed slightly,but the result does not so well(daidong may help to update
> the result later)

From comment 12 and also comment 3 it sounds like you're having a problem with multiple touch points getting detected. That might also be important here, I don't know why that is happening and it's probably a hardware/driver issue. If you resolve that issue first, it might fix everything else you're seeing too.
Flags: needinfo?(bugmail.mozilla)
(Reporter)

Comment 15

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #14)
> Sorry, I don't understand the data in comment 3. Can you explain what it
> means? It's just a bunch of random numbers to me.

I catch it in a more easy understand way:
/dev/input/event5: EV_ABS       ABS_MT_POSITION_X    000000d3            
/dev/input/event5: EV_ABS       ABS_MT_POSITION_Y    00000137            
/dev/input/event5: EV_KEY       BTN_TOUCH            DOWN                
/dev/input/event5: EV_SYN       SYN_MT_REPORT        00000000            
/dev/input/event5: EV_SYN       SYN_REPORT           00000000            
/dev/input/event5: EV_ABS       ABS_MT_POSITION_X    000000d3            
/dev/input/event5: EV_ABS       ABS_MT_POSITION_Y    00000136            
/dev/input/event5: EV_SYN       SYN_MT_REPORT        00000000            
/dev/input/event5: EV_SYN       SYN_REPORT           00000000            
/dev/input/event5: EV_ABS       ABS_MT_POSITION_X    000000d3            
/dev/input/event5: EV_ABS       ABS_MT_POSITION_Y    00000136            
/dev/input/event5: EV_SYN       SYN_MT_REPORT        00000000            
/dev/input/event5: EV_SYN       SYN_REPORT           00000000            
/dev/input/event5: EV_KEY       BTN_TOUCH            UP                  
/dev/input/event5: EV_SYN       SYN_MT_REPORT        00000000            
/dev/input/event5: EV_SYN       SYN_REPORT           00000000

The ABS_MT_POSITION_X/ABS_MT_POSITION_Y means the position of the touch.
 
> My hypothesis is that the line app is listening for touchmove events, and
> calls preventDefault on them in some scenarios.

I might misunderstand you……sorry.

We will make a further investigation and research in our locale.

Thank you so much for your advice!
(Assignee)

Comment 16

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10)
> It sounds to me based on comment 4 that the line app is calling
> preventDefault on the first touchmove event in some cases but not others.
> That is why the touch block gets prevented and we do not detect a tap
> gesture. It sounds to me like you should look in the line app touch event
> handler at that position to see what it is doing (you can inspect this with
> WebIDE). My guess is that you will find the line app does a prevent default
> if the touchmove is over a certain distance from the touchstart. In some
> cases your finger will move only slightly when you tap and that distance
> will be too small to trigger the preventDefault; in other cases your finger
> will move slightly more and it will trigger it.
we found preventDefault in line app with WebIDE, but it not only happened in "chat" button.
I mean most the divisions are using preventDefault but the only icon has no response.
I try to send a click event to "chat" button and then it works.
https://copy.com/2MRkxvlmsKtFSdlo

comment #15 shows the position of X, Y has slightly movement, it looks like the slightly change make the click event disappear in "chat" icon :(
(Reporter)

Comment 17

3 years ago
Hi Viral,

We found the functional preventDefault in line app with WebIDE:

>'ǏІlṮȈṮŢḬŦÌ': function () {
>  this[Ae].off().on(vv.EVENT_CLICK, this.ÎlṪІIٱĨȊŤḬ.bind(this), !1).on(vv.EVENT_TOUCH_MOVE, function >(a) {
>    a.preventDefault()
>  }),

The line app add a 'a.preventDefault()' for the page when a 'vv.EVENT_TOUCH_MOVE' event is reported.this prevents the generate of tap gestures.

as mentioned in comment14 ' Whether or not touchmove events are sent to the app depends in large part on the sensitivity of the hardware and will vary from device to device',So we don't think it a issue to report a TOUCH_MOVE event after TOUCH_START. I think not only dolphin,but Hamachi/Buri will hold such a issue.

We are a little confusion why a 'a.preventDefault()' function is called here?
Should it be more reasonable to check this issue in line gaia?
It might be a little hard to modify driver only for this issue,What do you think??

Need your help :(
Flags: needinfo?(vwang)
I've seen this happen in a couple of different places; the intent of the web page is usually to prevent panning. Going forward the touch-action implementation of the pointer events spec is the correct way to do this but in the meantime we could work around it in gecko by only sending the touchmove event if it passes a certain distance threshold from the touchstart. This threshold would be dependent on the touch slop amount in APZ, and would only come into effect if the page is pannable. In particular it would not be in effect for fullscreen or other non-pannable content because for those cases we want to deliver touchmove events as aoon as possible to the content.
(Reporter)

Comment 19

3 years ago
Hi Kats,

Thank you for your advice :).

But there are still something need your help.

For the methods you mentioned in comment18,We would try it in our locale,but we have known little for this part you mentioned,So would you please give us some relevant pdf files which could help us to have a deep Understanding of this part?

As far as I can know,AsyncPanZoomController.cpp or nsPresShell.cpp should be the relevant file to be modified,is that right?
(Assignee)

Comment 20

3 years ago
(In reply to jingmei.zhang from comment #17)
> Hi Viral,
> 
> We found the functional preventDefault in line app with WebIDE:
> 
> >'ǏІlṮȈṮŢḬŦÌ': function () {
> >  this[Ae].off().on(vv.EVENT_CLICK, this.ÎlṪІIٱĨȊŤḬ.bind(this), !1).on(vv.EVENT_TOUCH_MOVE, function >(a) {
> >    a.preventDefault()
> >  }),
> 
> The line app add a 'a.preventDefault()' for the page when a
> 'vv.EVENT_TOUCH_MOVE' event is reported.this prevents the generate of tap
> gestures.
> 
> as mentioned in comment14 ' Whether or not touchmove events are sent to the
> app depends in large part on the sensitivity of the hardware and will vary
> from device to device',So we don't think it a issue to report a TOUCH_MOVE
> event after TOUCH_START. I think not only dolphin,but Hamachi/Buri will hold
> such a issue.
> 
> We are a little confusion why a 'a.preventDefault()' function is called here?
> Should it be more reasonable to check this issue in line gaia?
> It might be a little hard to modify driver only for this issue,What do you
> think??
> 
> Need your help :(

I guess they use "preventDefault" to avoid user press the parent element.
I will pass it to partner to see if we have any feedback.

Actually I still think to modify the driver should be the fastest solution on this bug since we can't modify the Line app in our side. I think I can have a quick try in kernel side first.
Flags: needinfo?(vwang)
(In reply to jingmei.zhang from comment #19)
> For the methods you mentioned in comment18,We would try it in our locale,but
> we have known little for this part you mentioned,So would you please give us
> some relevant pdf files which could help us to have a deep Understanding of
> this part?
> 
> As far as I can know,AsyncPanZoomController.cpp or nsPresShell.cpp should be
> the relevant file to be modified,is that right?

Yeah it would be AsyncPanZoomController and some related classes that would need to be modified, but it won't be that easy to do; I'm not sure myself exactly how it should be done, I will need to think about it. The general idea is that the code at [1] is what decides if the touchmove is big enough to consider, or if it is not yet big enough to trigger panning. What we want to end up doing is to have the code at [2] take that into account, so that if the pointer events are consumable but we are still within the slop area then we return nsEventStatus_eConsumeNoDefault and the touch events don't end up going through gecko. This would potentially have other negative implications though, because it make things like bug 1058255 comment 0 worse. So I'm not really sure if this is a good idea, specially if this is for the 2.1 codeline which is pretty old now.

It might be better to let Viral try a kernel/driver patch first if he can come up with something that doesn't regress other things.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=c7a90f1a321c#1201
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/InputQueue.cpp?rev=992050dc1e5f#115
(Assignee)

Comment 22

3 years ago
Created attachment 8558459 [details] [diff] [review]
add a filter in touch driver to make the click event works

This kernel patch add a filter in touch driver, just a quick patch to fix the bug. I think this patch can also improve some performance but I don't really care coding style in this patch.

The result looks good to me, it shows fine in 2/2 testing in video:
https://www.copy.com/s/Jq8wjzBzyqNgO1ug/bug1126615_with_patch.mp4
Attachment #8558459 - Flags: feedback?(jingmei.zhang)
(Assignee)

Updated

3 years ago
Attachment #8558459 - Attachment description: add a filter in touch driver to make the click event ea → add a filter in touch driver to make the click event works
(Reporter)

Comment 23

3 years ago
Hi Viral,

I have tested in my locale,it works well ^_^.


Thank you so much!!
(Reporter)

Updated

3 years ago
Attachment #8558459 - Flags: feedback?(jingmei.zhang) → feedback+
(Assignee)

Comment 24

3 years ago
(In reply to jingmei.zhang from comment #23)
> Hi Viral,
> 
> I have tested in my locale,it works well ^_^.
> 
> 
> Thank you so much!!

Hi Jingmei,

Good to hear that :)

I think you can put it in your kernel code base if you don't have other concern.
Please let us know the time you update your kernel with this patch, then we can close this bug.
Thank you!

Comment 25

3 years ago
hi viral
the patch has some bug
		input_report_key(data->input_dev, BTN_TOUCH, 1);
+		for(i = 0; i < event->touch_point; i++) { //this line may has some problem.
+			if((buf[6*i+3] & 0xc0) == 0xc0)
+				continue;
+
the change like following:
		input_report_key(data->input_dev, BTN_TOUCH, 1);
+		for(i = 0; i < event->TS_MAX_FINGER; i++) { 
+			if((buf[6*i+3] & 0xc0) == 0xc0)
+				continue;
+
because this is a muti touch screen.

I also have some questions:
1.
if((buf[6*i+3] & 0x40) == 0x0) { 
can you tell me why you add this line? I can't find datasheet.

2. this just resove one finger move, if user have muti fingers,this may have some problem.
(Assignee)

Comment 26

3 years ago
(In reply to luodaidong from comment #25)
> hi viral
> the patch has some bug
> 		input_report_key(data->input_dev, BTN_TOUCH, 1);
> +		for(i = 0; i < event->touch_point; i++) { //this line may has some
> problem.
> +			if((buf[6*i+3] & 0xc0) == 0xc0)
> +				continue;
> +
> the change like following:
> 		input_report_key(data->input_dev, BTN_TOUCH, 1);
> +		for(i = 0; i < event->TS_MAX_FINGER; i++) { 
> +			if((buf[6*i+3] & 0xc0) == 0xc0)
> +				continue;
> +
> because this is a muti touch screen.
That's not a bug, actually it improve some performance :)
Since you already have your finger numbers in "event->touch_point", you don't really need to scan all TS_MAX_FINGER (5 in this case) and report the redundant finger data.

When you have 2 fingers on touch screen, "event->touch_point" will be 2, and it can report two fingers.
You can check it by zoom in / zoom out gesture, it works just as our expect.
> 
> I also have some questions:
> 1.
> if((buf[6*i+3] & 0x40) == 0x0) { 
> can you tell me why you add this line? I can't find datasheet.
> 
I don't have your datasheet, the code exist in your code and I just rewrite the flow.
-               if((buf[6*i+3] & 0x40) == 0x0) {

> 2. this just resove one finger move, if user have muti fingers,this may have
> some problem.
Yes, all i care is one finger case. When you have more than one finger, than we don't care about click event.
I don't think we should consider click event in multi-touch case, right? I mean, we don't care about scenario the one finger move and another finger click at same time.

Comment 27

3 years ago
(In reply to viral [:viralwang] from comment #26)
> (In reply to luodaidong from comment #25)
> > hi viral
> > the patch has some bug
> > 		input_report_key(data->input_dev, BTN_TOUCH, 1);
> > +		for(i = 0; i < event->touch_point; i++) { //this line may has some
> > problem.
> > +			if((buf[6*i+3] & 0xc0) == 0xc0)
> > +				continue;
> > +
> > the change like following:
> > 		input_report_key(data->input_dev, BTN_TOUCH, 1);
> > +		for(i = 0; i < event->TS_MAX_FINGER; i++) { 
> > +			if((buf[6*i+3] & 0xc0) == 0xc0)
> > +				continue;
> > +
> > because this is a muti touch screen.
> That's not a bug, actually it improve some performance :)
> Since you already have your finger numbers in "event->touch_point", you
> don't really need to scan all TS_MAX_FINGER (5 in this case) and report the
> redundant finger data.
> 
> When you have 2 fingers on touch screen, "event->touch_point" will be 2, and
> it can report two fingers.
> You can check it by zoom in / zoom out gesture, it works just as our expect.
> > 
> > I also have some questions:
> > 1.
> > if((buf[6*i+3] & 0x40) == 0x0) { 
> > can you tell me why you add this line? I can't find datasheet.
> > 
> I don't have your datasheet, the code exist in your code and I just rewrite
> the flow.
> -               if((buf[6*i+3] & 0x40) == 0x0) {
> 
> > 2. this just resove one finger move, if user have muti fingers,this may have
> > some problem.
> Yes, all i care is one finger case. When you have more than one finger, than
> we don't care about click event.
> I don't think we should consider click event in multi-touch case, right? I
> mean, we don't care about scenario the one finger move and another finger
> click at same time.

hi viral:
for example: If you press one finger ,then press the second finger, then up the first finger, this time
the second finger's data can't report.
so the loop should be
for(i = 0; i < event->TS_MAX_FINGER; i++) {
(Assignee)

Comment 28

3 years ago
(In reply to luodaidong from comment #27)
> (In reply to viral [:viralwang] from comment #26)
> > (In reply to luodaidong from comment #25)
> > > hi viral
> > > the patch has some bug
> > > 		input_report_key(data->input_dev, BTN_TOUCH, 1);
> > > +		for(i = 0; i < event->touch_point; i++) { //this line may has some
> > > problem.
> > > +			if((buf[6*i+3] & 0xc0) == 0xc0)
> > > +				continue;
> > > +
> > > the change like following:
> > > 		input_report_key(data->input_dev, BTN_TOUCH, 1);
> > > +		for(i = 0; i < event->TS_MAX_FINGER; i++) { 
> > > +			if((buf[6*i+3] & 0xc0) == 0xc0)
> > > +				continue;
> > > +
> > > because this is a muti touch screen.
> > That's not a bug, actually it improve some performance :)
> > Since you already have your finger numbers in "event->touch_point", you
> > don't really need to scan all TS_MAX_FINGER (5 in this case) and report the
> > redundant finger data.
> > 
> > When you have 2 fingers on touch screen, "event->touch_point" will be 2, and
> > it can report two fingers.
> > You can check it by zoom in / zoom out gesture, it works just as our expect.
> > > 
> > > I also have some questions:
> > > 1.
> > > if((buf[6*i+3] & 0x40) == 0x0) { 
> > > can you tell me why you add this line? I can't find datasheet.
> > > 
> > I don't have your datasheet, the code exist in your code and I just rewrite
> > the flow.
> > -               if((buf[6*i+3] & 0x40) == 0x0) {
> > 
> > > 2. this just resove one finger move, if user have muti fingers,this may have
> > > some problem.
> > Yes, all i care is one finger case. When you have more than one finger, than
> > we don't care about click event.
> > I don't think we should consider click event in multi-touch case, right? I
> > mean, we don't care about scenario the one finger move and another finger
> > click at same time.
> 
> hi viral:
> for example: If you press one finger ,then press the second finger, then up
> the first finger, this time
> the second finger's data can't report.
> so the loop should be
> for(i = 0; i < event->TS_MAX_FINGER; i++) {

Okay, I understand your concern now. You are worried about the timing that 2nd finger press and then 1st finger leave, we may miss the data of 2nd finger.
It happens in some touch chips but it looks like touch chip in dolphin didn't save finger id and keep the finger count starting from finger 1.
When your 1st finger leave, your 2nd finger become finger one in next scan, so the event should be fine.
Let's check the event below:

EV_KEY       BTN_TOUCH            DOWN                // 1st finger press
EV_ABS       ABS_MT_POSITION_X    00000077            
EV_ABS       ABS_MT_POSITION_Y    0000017a            
EV_SYN       SYN_MT_REPORT        00000000            
EV_SYN       SYN_REPORT           00000000            
EV_ABS       ABS_MT_POSITION_X    00000077            // 2nd finger press
EV_ABS       ABS_MT_POSITION_Y    0000017a            
EV_SYN       SYN_MT_REPORT        00000000            
EV_ABS       ABS_MT_POSITION_X    000001c2            
EV_ABS       ABS_MT_POSITION_Y    000002c9            
EV_SYN       SYN_MT_REPORT        00000000            
EV_SYN       SYN_REPORT           00000000
...                                                   
EV_ABS       ABS_MT_POSITION_X    00000077            // 2 fingers stay for a while
EV_ABS       ABS_MT_POSITION_Y    0000017a            
EV_SYN       SYN_MT_REPORT        00000000            
EV_ABS       ABS_MT_POSITION_X    000001ba            
EV_ABS       ABS_MT_POSITION_Y    000002c6            
EV_SYN       SYN_MT_REPORT        00000000            
EV_SYN       SYN_REPORT           00000000            
EV_ABS       ABS_MT_POSITION_X    000001ba            // 1st finger leave, 2nd finger become the only finger
EV_ABS       ABS_MT_POSITION_Y    000002c6            // on screen and the position of X and Y are correct
EV_SYN       SYN_MT_REPORT        00000000            
EV_SYN       SYN_REPORT           00000000
...
EV_ABS       ABS_MT_POSITION_X    000001b8            
EV_ABS       ABS_MT_POSITION_Y    000002c5            
EV_SYN       SYN_MT_REPORT        00000000            
EV_SYN       SYN_REPORT           00000000            
EV_KEY       BTN_TOUCH            UP                  // 2nd finger leave
EV_SYN       SYN_MT_REPORT        00000000            
EV_SYN       SYN_REPORT           00000000
(Reporter)

Comment 29

3 years ago
Hi Viral,

The bug is fixed in our locale,you can set this bug fixed now.^_^
(Assignee)

Comment 30

3 years ago
Thank you for your information :)
Assignee: nobody → vwang
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.