Samsung / GearVRf

The GearVR framework(GearVRf) is an Open Source VR rendering library for application development on VR-supported Android devices.
http://www.gearvrf.org
Apache License 2.0
407 stars 217 forks source link

onKeyUp is called multiple times when back button is pressed #148

Closed niusounds closed 9 years ago

niusounds commented 9 years ago

I want to handle GearVR back button pressing event to cancel showing default Oculus Home dialog. I implement public boolean onKeyUp(int keyCode, KeyEvent event) in my Activity class.

public boolean onKeyUp(int keyCode, KeyEvent event) {

  if (showingPlayerScene()) {
    backToContentBrowser();
    return true; // Cancel showing default dialog.
  }

  return false; // Show default Oculus Home dialog.
}

return true; cancels dialog. It's I expected. But onKeyUp method was actually called twice unexpectedly.

I suspect that would be wrong key event handling in GVRActivity::OnKeyEvent.

bool GVRActivity::OnKeyEvent(const int keyCode, const int repeatCode,
        const OVR::KeyEventType eventType) {

    // 1: KeyState::KEY_EVENT_DOWN, 0: KeyState::KEY_EVENT_UP. Other information is lost from Oculus side.
    int isDown = (eventType == OVR::KEY_EVENT_DOWN) ? 1 : 0;

    return app->GetVrJni()->CallBooleanMethod(javaObject,
            onKeyEventNativeMethodId, keyCode, isDown);
}

In above code, isDown will be 0 when eventType is not only KEY_EVENT_UP but also KEY_EVENT_SHORT_PRESS, KEY_EVENT_LONG_PRESS or some other values.

These events (KEY_EVENT_SHORT_PRESS or KEY_EVENT_LONG_PRESS) should not be handled by onKeyUp.

jason2kim commented 9 years ago

Hi, I remember the KEY UP event was tested working correctly with the previous Oculus version of 0.5. Can you please let us know a more details, including Ovr version, which key code (BACK only or other keys), long press, and test log if you have? thanks.

niusounds commented 9 years ago

I'm using Oculus Mobile SDK 0.6.0 because I found "Initial check-in of port to OVR SDK 0.6.0" commit message. (...Is GVRf not supporting 0.6.0 yet?)

My problem occurs with key code 4 (android.view.KeyEvent.KEYCODE_BACK). This is back button on GearVR near touch pad. I have not tried other keys (such as gamepad) because I have no gamepads.

dknaan commented 9 years ago

dispatchKeyEvent() is also called twice when I use a gamepad. I checked time between clicks to ignore the second click.

jason2kim commented 9 years ago

Hi, thanks for the details. Let me try to reproduce your situation from here and get back to you soon. (I was asking ovr version, as these events are intercepted and delivered from ovr’s native side)

jason2kim commented 9 years ago

On my try, it does not happen. I mean, it calls onKeyUp() only once when short pressed.

Hi Chao, can you please try on your side, in case I am missing something? Thank you! I was testing by adding some log messages in the existing onKeyUp() handler in our gvr video sample:

public class VideoActivity extends GVRActivity implements OnTouchPadGestureListener { @Override public boolean onKeyUp(int keyCode, KeyEvent event) { Log.d("keyboard-trace VideoActivity", "onKeyUp " + keyCode + ", event = " + event); if (keyCode == KeyEvent.KEYCODE_BACK) { Log.d("gvrvideo VideoActivity onKeyUp", " KEYCODE_BACK"); onBackPressed(); return true; } return false; }

niusounds commented 9 years ago

I have tried your code. onKeyUp still called twice when back button is pressed. This log line is printed twice.

07-08 15:12:21.691  19453-19482/com.eje_c.gvrtest D/keyboard-trace VideoActivity﹕ onKeyUp 4, event = KeyEvent { action=ACTION_UP, keyCode=KEYCODE_BACK, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=0, downTime=0, deviceId=-1, displayId=0, source=0x0 }

code:

public class MainActivity extends GVRActivity {

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setScript(new SampleViewManager(), "gvr_note4.xml");
    }

    @Override
    public boolean onKeyUp(int keyCode, KeyEvent event) {
        Log.d("keyboard-trace VideoActivity", "onKeyUp " + keyCode + ", event = " + event);
        if (keyCode == KeyEvent.KEYCODE_BACK) {
            Log.d("gvrvideo VideoActivity onKeyUp", " KEYCODE_BACK");
            onBackPressed();
            return true;
        }
        return false;
    }
}

I tried VrApi and VrAppFramework of OVR 0.6.0 and 0.6.0.1. Both are same behavior. GearVRf is most recent master branch.

jason2kim commented 9 years ago

After many tries using several HMD devices (pressing the back button on the device, short and long), I could see the KEY UP event was delivered twice sometimes. It usually happens when the repeat count was not greater than 0 (short pressed). It is not easy to reproduce it consistently while wearing the HMD.

As said in my previous posting, the Java key up and down handlers were called from OVR native side. Further, the event information including repeat counter is lost in the process and not under tight control of GearVRf. Considering this situation, I would like to recommend you to instead use the KEY DOWN event, i.e., onKeyDown(), which is consistently called once. Please let us know if you have any further questions or concerns.

jason2kim commented 9 years ago

Hi Dotan, Do you have the adb log captured to share with me when the dispatch was called twice? Just wanted to make sure. Thanks.

From: dknaan [mailto:notifications@github.com] Sent: Saturday, July 04, 2015 11:24 PM To: Samsung/GearVRf Cc: Jason Kim Subject: Re: [GearVRf] onKeyUp is called multiple times when back button is pressed (#148)

dispatchKeyEvent() is also called twice when I use a gamepad

— Reply to this email directly or view it on GitHubhttps://github.com/Samsung/GearVRf/issues/148#issuecomment-118585211.