GearVRf icon indicating copy to clipboard operation
GearVRf copied to clipboard

GVRInputPicker enhancement ideas

Open SteveGreatApe opened this issue 7 years ago • 15 comments

Hi, after trying to adapt a couple of projects to using GVRPickerInput I've got a few thoughts on it's behaviour.

First thing to say is that I think it's definitely the right way to go to have a standard way of handling this behaviour, it would make life very difficult if all apps had their own custom picker code. This is especially true when it comes to sharing any code that interacts with the picker.

If you want GVRPickerInput to become the standard then I believe it's going to need to be a bit more comprehensive and flexible in it's behaviour to deal with different use requirements.

The first big point I noticed is that ACTION_MOVE events don't contain a location, which seems to defeat the point.

After trying a modification to my code I can see that the fix is not that straight forward. I'd hoped to just pass mCurrentCollision.getHitLocation() straight through. But that fails as the hit location is for the object the pointer is currently over, not the mPressedSceneObject that the events are delivered to. I can see this case has been dealt with when handling ACTION_UP with the 'if (mPressedSceneObject == mHoveredSceneObject)' check.

It would be nice if there was some way you could keep getting relative co-ords even outside the collider mesh, but that's not as easy as it is with something like 2D mouse or touch screen input, so guess that's not going to be practical to do. But I think we at least want to get the co-ordinates while the picker is still inside the mesh, then pass null to indicate when the picker is outside.

As we can't pass meaningful move events to mPressedSceneObject outside it's bounds, should we then give up on the idea of remembering the object that the down press was in? Simply deliver all events to whatever object the picker is currently over?

Trouble is I don't think there's going to be a one size fits all solution, in some cases once you've clicked on one object you don't want stray move events going elsewhere.

Another currently missing feature is ACTION_HOVER_MOVE events, currently there's no way to get these. I can see that you might not want to generate these all the time due to the overhead as they could be sent non-stop in a scene where the pickers left pointing at a collider. It would be helpful if some objects could declare they do want to receive these events.

I've had a go at implementing a version that sends these events, it seems generatePickEvents() is the place to do it, if there is a current mHoveredSceneObject and we're not sending a Hover Enter event then we can send the ACTION_HOVER_MOVE instead. In doing this I realised that surely this is the correct place to send standard ACTION_MOVE events as well? Do the ACTION_MOVE events received in onTouchEvent from the system have any real meaning in the picker context? I don't quite understand what the system is doing here as I can't figure out how these fit into the picker scheme.

Having worked on similar issues for touch screen window interaction on Psion PDA's years ago I'd suggest if we do want configurable behaviour it needs to be done per object, not as a global picker setting. I'd suggest adding in to GVRCollider something like:

final static int PICK_FLAG_SEND_HOVER_EVENTS = 0x01; final static int PICK_FLAG_GRAB_PICKER_ON_DOWN = 0x02;

// For the application to call. public void setPickFlags(int flags);

// For GVRPickerInput to retrieve the flags. public int getPickFlags();

Thanks, Steve

SteveGreatApe avatar Oct 17 '17 09:10 SteveGreatApe

I've hit another problem with the current implementation, the MotionEvent's handled in onTouchEvent can't distinguish between the main clicker and touch pad button, both come through as MotionEvent.ACTION_DOWN. This is a complete deal breaker for me as I need to handle the two separately.

Can you switch to using GVRCursorController.ControllerEventListener to listen out for KeyEvents instead of using ActivityEventsHandler listening to IActivityEvents? Then replace the MotionEvent parameter in the onTouch event with a KeyEvent parameter which will give the required information?

SteveGreatApe avatar Oct 19 '17 08:10 SteveGreatApe

I am reworking the controller APIs a bit for 4.0. If the tracks and main clicker had a different button, would that work for you? Right now they both do BUTTON_PRIMARY but we could make one of them be BUTTON_SECONDARY.

NolaDonato avatar Oct 19 '17 18:10 NolaDonato

I meant " trackpad"

NolaDonato avatar Oct 19 '17 18:10 NolaDonato

That would help, I've also found my idea of using KeyEvents doesn't handle the case of tapping the touchpad on the headset while the controller is inactive and I'm using the Eye tracker.

So currently I'm using KeyEvent's to handle the main and trackpad clickers on the controller, plus filtering for onTouchEvent's for type TOOL_TYPE_MOUSE to handle tapping on the headset's touchpad.

In my case I don't actually need the secondary touchpad button to be associated with the picker, I need to be able to poll on each frame whether it's down or not. So for now I've added a new function into my modified Picker Input class:

public boolean isDown(int keycode);

Not sure whether GVRPickerInput is actually the right place for this, but if it needs to listen to KeyEvent's anyway then seems trivial enough to track the up/down state of keys as well.

SteveGreatApe avatar Oct 23 '17 09:10 SteveGreatApe

@SteveGreatApe Thank you for the great feedback. Please switch to the 3.3 release since GVRPickerInput has been superseded by changes to GVRPicker. It is unlikely we will resurrect GVRPickerInput so we need to figure out a way to fix and enhance what is in the release. Thank you for your help.

liaxim avatar Oct 23 '17 21:10 liaxim

Hi, did you mean I should switch to last weeks 4.0 release? It's the 3,3 release where GVRPickerInput was introduced and I've only just switched to it from previously using GVRPicker. The reason I switched to GVRPickerInput was I wanted a standard way of interfacing between components and the picker.

I've got my own GearVRf keyboard I've implemented and was looking at how I could release it as open source, but a big question mark was over how I could integrate it with the picker, as I had my own custom layer over GVRPicker which I would expect to be quite different to how other people had used GVRPicker, so plugging in my keyboard into their code would be tricky without a standard Picker to component interface.

Then I saw your team have done their own keyboard as well, so I looked at how you'd connected that with GVRPicker, which was of course with GVRPickerInput. This seemed to me to be exactly the right approach to make a standard system layer to plug the picker into UI Components like the keyboard, so I've modified my code to do it the same way. If I go back to just using GVRPicker then we lose this standard interface which will make life difficult if we want to share more components like these keyboards.

From a quick scan of the 4.0 source code I can't see any changes to GVRPicker to help with this situation, your GVRKeyboardSceneObject class still uses the ITouchEvents and IHoverEvents interfaces to interface with the picker. Which means to use it you need to use either GVRPickerInput, or do your own similair implementation that generates the same ITouchEvents and IHoverEvents.

SteveGreatApe avatar Oct 24 '17 07:10 SteveGreatApe

Actually my bad. The GVRPickerInput was removed right after we made the 3.3 release. And it is still in the 4.0.

liaxim avatar Oct 24 '17 14:10 liaxim

@SteveGreatApe The work for the next version is ongoing and we can look into making enhancements there. GVRPickerInput has been rolled into GVRPicker. You can see what we currently have in the maint-3.x branch. The GVRKeyboardSO has been updated too so can serve as a reference.

liaxim avatar Oct 24 '17 16:10 liaxim

I am currently working on reworking the controller for 4.1 (It was supposed to go into 4.0 but did not make it). I will make a special version of it for you that works with the 3.3 release. It will appear as a pull request against the maint_v3.x release even though it will not ever be merged into that release. I can probably have this ready by tomorrow afternoon.

NolaDonato avatar Oct 25 '17 01:10 NolaDonato

Thanks, but I'm having a go with 4.0 now so will probably stick with that, I've already knocked up my own alternative modified version I can keep using for now until 4.1 comes along.

SteveGreatApe avatar Oct 25 '17 08:10 SteveGreatApe

OK I will continue to prepare the pull request for 4.0 then. I will let you know when it is ready and you can try it out. Right now I am working on some issues with the 3D cursor extension.

NolaDonato avatar Oct 25 '17 19:10 NolaDonato

I have a pull request for controller changes that is somewhat substantial and needs lots of testing before it is merged. Pull request #1646 fixes a lot of issues with the 3D cursor stuff and controllers in general. I made it a lot easier to select controllers - you now specify which ones you want in gvr.xml if you are not using the 3D cursor stuff. That library still uses the settings.xml file to control cursors because it allows multiple cursors at once whereas the new GVRInputManager.selectController just selects one.

I cleaned up the event mess in the 3D cursor library. Now the picker generates ITouchEvents which are converted to ICursorEvents by the 3D cursor library. The only difference is that ICursorEvents includes which cursor triggered the event. MovableBehavior and SelectableBehavior work as they did before.

I did a bunch of work on removing and switching controllers. That did not work very well before. Now it will dynamically adapt to removing and adding controllers better.

The only thing that seems to be wonky is scrolling when using the widget plugin. I would stay away from the widget plugin in general because the library is really old, we don't have the source and we are planning to deprecate it.

So if you want an early preview of 4.1 controller reform, check out PR #1646.

The demos have all been updated to work with this pull request. You can find them in GearVRf-Demos PR #556.

NolaDonato avatar Dec 22 '17 23:12 NolaDonato

After working with this for a while, please give me feedback as to whether the new APIs meet your needs. I have added ITouchEvents which merge picking and touching in a more sensible way. You can listen for these events on individual scene objects via the event receiver on the object. Or you can have one listener for all the events if you use the event receiver on the controller's picker. The intent was to make the API a little higher level than motion events so it would work with a wide variety of input devices, not just hand controllers.

NolaDonato avatar Jan 11 '18 00:01 NolaDonato

Thanks, it's getting there now. There is one area I think needs a bit of help to make it easier when dealing with different types of controller .

As I only want my controls to operate on the main clicker, not the touchpad, all my onTouchStart/onTouchEnd need this check at the start:

    @Override
    public void onTouchEnd(GVRSceneObject gvrSceneObject, GVRPicker.GVRPickedObject gvrPickedObject) {
        if (gvrPickedObject.motionEvent != null && gvrPickedObject.motionEvent.getButtonState() == BUTTON_SECONDARY) {
            // Do something...
        }
    }

The trouble is this doesn't work with the headset touchpad as touches on there come through as BUTTON_PRIMARY instead of BUTTON_SECONDARY, so everyone of my onTouchStart/onTouchEnd need a way to check the controller type, then decide what check to do to make sure it's the right button pressed. I haven't tried yet, but I imagine doing this for Mouse and Gamepad controllers will make this even more complicated.

What I really need is a nice simple way of setting things up so I only receiving these events on the button(s) I want, then I can dispense with the numerous checks in each function.

To make it more complicated at the moment there's no quick and easy way to check the current controller type without listening out for the onCursorControllerSelected events and keeping note of the current setting. Trouble with this plan is it makes an undesirable dependency between each section of code handling touches and my main top level code monitoring the controller selected events.

To see what I mean try modifying your GVRKeyboard to work as my code does work on the main clicker or by touching the headset pad. You'll realise GVRKeyboard has no easy way of detecting the current controller type without adding it's own event listener.

One quick and easy help would be a simple function to query the current controller without having to keep track of the onCursorControllerSelected events.

Possibly another quick and simple fix might be to just swap the gear controller button classifications so the main clicker becomes BUTTON_PRIMARY, and the touchpad becomes BUTTON_SECONDARY. Then make sure all other controller types have a sensible setting for BUTTON_PRIMARY. Then I can just check for BUTTON_PRIMARY everywhere without worrying about what type of controller it is.

I think my preferred solution would be going back to what I mentioned earlier, a way of configuring the controller so I only get the onTouchStart/onTouchEnd events on button(s) I want.

SteveGreatApe avatar Jan 12 '18 17:01 SteveGreatApe

I have added GVRCursorController.setTouchButtons to let you specify which buttons will produce touch events. The cursor controller still generates MotionEvents for all of the buttons. It just won't call onTouchStart, onTouchEnd unless these buttons are pressed. Let me know if that works for you. I tested it with the keyboard app.

NolaDonato avatar Jan 12 '18 22:01 NolaDonato