RInput, gyro emulation #3

john-peterson · 2012-12-05T18:06:14Z

gyro emulation

files

https://www.dropbox.com/sh/rrd5iy2jivzrapq/AACMW-TeC1D5VE17u42PZIsFa?dl=0 contain

input_*.zip: builds of input@john-peterson
test/user/Config/Profiles/Wiimot: Zelda SS gyro input profiles in "Zelda SS*.ini"
Pics/m+: pictures

gyro log

			IR		 		wm acc				n acc				gyro			
41:46:028  0.00 -0.00  |  0.03  0.00  1.00 |  0.00  0.00  1.00 |  0.00 -0.00 -0.00 ***

emulated input

gyroscope and accelerometer input

https://github.com/john-peterson/dolphin-emu/blob/input/Source/Core/Core/Src/HW/WiimoteEmu/WiimoteEmu.cpp

in the input settings

"swing" is a force in the X, Y, Z directions that affect the accelerometer
"tilt" is a force in the pitch, roll, yaw directions that affect the accelerometer and gyro

Settings

Axis mouse input

Relative mouse input (called "Axis" in the mouse input configuration) should be used because that allow mouse movement input regardless of where the system cursor is compared to absolute input (called "Cursor" in the input configuration) who's input won't change when the cursor is at the screen edge

Range

Range, Gyro Range and Acc Range multiply the output by 0.01 x to 10 x (by setting its range 1 to 1000). The fast modifier activates the gyro fast mode for all three rotation directions, the fast and slow mode is explained here

gyro settle

https://github.com/john-peterson/dolphin-emu/blob/input/Source/Core/InputCommon/Src/ControllerEmu.h#L491

gyro settle determine after how many frames the gyro will return to zero after the wiimote stop moving (the wiimote is constant but not necessarily nonzero)

a high setting, for example 9999, will keep the gyro in constant motion if input is nonzero

Mouse and keyboard

Gyro Sensitivity: 200 works with a standard sensitivity mouse

IR sensitivity: 20 to move the vertical accelerometer position relatively slowly.

Gamepad

Gyro Range: 0.1 provide smooth aiming and a smooth cursor, the range modifier is set at 1000 (10x)

If the Gyro Range is 0.2 the Range modifier should be 500 (5x) to reach 1.0 with max input

Zelda SS

Moves

Swing sword left/right: yaw left/right

Swing sword up/down: pitch up/down

Stab attack: thrust forward + show IR

Spin attack left/right: yaw left/right + nunchuk thrust in any direction

Land/water spin attack up/down: pitch up/down + nunchuk thrust in any direction (or wiimote thrust forward/backward + nunchuk thrust in any direction)

Skyward charge: pitch accelerometer back (gyro range must be lowered enough that it doesn't trigger a swing)

Fatal blow: lock on enemy + wiimote thrust forward + nunchuk thrust forward
Shield: nunchuk thrust in any direction

Roll: dash and nunchuk thrust in any direction

Leap on vine: Nunchuk stick left/right + tilt fast left/right

Balance on the tightrope: Strong yaw input (slow 0.5+ or fast 0.1+)

Key binding

Have the Gyro Range low by default, for example 0.1, to control the cursor and view, and assign a 10 x (range 1000) Gyro Range modifier to a gamepad trigger (or keyboard button or analog stick button) and hold it to perform fast moves; slash sword, leap on vines, swing on rope, shake rope etc.

On the gamepad, bind roll and yaw to the same axis, roll and yaw is used independently in rare occasions only (the boss key for example) so it doesn't hurt, and lets you control the bird or the beetle that use roll.

Use the | OR option with the modifier keys and moves that require multiple input. For example assigning A|B to yaw left and B to Nunchuk thrust makes A yaw left and B do the spin attack.

"Hide IR" should be enabled (and Thrust forward and B should be bound to IR→ Show) because IR restrict horizojntal cursor movement and doesn't have rotation use besides resetting the gyro rotation matrix forward direction

FAQ

Q: Why's the aiming or cursor sensitive
A: Lower the sensitivity setting

Q: Why's the aiming or cursor jumpy
A: Disable Tilt accelerometer input with "Acc. Range"

Q: The cursor is locked along the horizontal axis
A: It's because the cursor can't go to the edge of the screen if IR is visible, turning off IR solve that. Without IR data, 45° yaw move the cursor from one edge of the screen to the other, if additional yaw is applied to cursor move along the screen border

Q: The cursor drift vertically to the center (doesn't affect aiming mode)
A: Too little accelerometer pitch is applied because the IR pointer position is not at the end of the screen, possibly because of too low IR sensitivity, the IR implied pitch code is here, as the IR is pointing at the end of the screen (mouse input -1 or 1) the pitch is pi/4 rad (45°).

Q: Why is the item selection cursor difficult to control?
A: Bind "Thumb R" or B to "Gyro Range 1" that's set to 1000 (10× multiplier) so that adequate gyro range is applied when selecting item. Because the item selection cursor work the same way as the map cursor so that good IR position data control it. Without IR data a 360° degree pitch rotate the cursor past the whole item circle. In the example gamepad configuration and bound to

Q: Why's the Bird, the Beetle and Swimming hard to control? Sometimes the unit moves sideways on its own.
A: Showing IR (that you may have set to the stab attack key) makes these controls easier, just showing IR for for a moment (at the center of the screen position) will reset the gyro rotation matrix so that the unit swim or fly straight again. To gain altitude with the Bird repeatedly dive and immediately call the Bird.

Q: Why is it hard to rotate the sword to enable the Skyview Temple Eye Switch?
A: A perfect circle can be created without IR data but IR data is important to reset the gyro rotation matrix forward direction

Q: How do I reset the sword neutral position (gyro rotation matrix)?
A: Show IR for a moment

Q: How do I pull the Goddess Sword out of the stone in the Goddess Statue?
A. Tilt 90° (full tilt input at 100 acc. range) downward and thrust backward

Red Steel 2

Moves

Swing sword left/right: yaw left/right
Swing sword up/down: pitch up/down

john-peterson · 2013-05-30T15:45:56Z

old discussions

These pages contain old discussions about this patch that have been copied here

Commit organisation

@_RachelB

[08:15] <_RachelB> JPeterson: m+ and gc pad radius for example, are not related at all

The GC pad radius patch is in this PR because

it changes the files "InputConfigDiagBitmaps.cpp" and "ControllerEmu.h" that's changed in other commits

If the patch is approved it can be cherry-pick to origin/master and removed from this branch

@Zardo1

Since I wanted to use the Motion Plus emulation with the newest revisions of Dolphin, I merged jpetersons branch with Dolphin master and reverted all unrelated changes.

I don't object to this patch because the differences (see attachment) in this patch are few (but the removed changes might have introduced bugs)

@skidau

Please separate the changes marked with * out into their own branch:

We can merge the ones marked with * at a later stage.

Do you have reservations about the changes you have marked with *?

I haven't started any sort of code review on this branch so no reservations so far

Then it's probably better to wait for a consensus on all changes, before splitting the patch into pieces.

@Sonicadvance1

The problems specifically in this issue on your last merge request.

Can you pick one? because there's multiple issues brought up and some are fixed.

your ... patches need to be split up in to separate patch files since a lot of the changes affect many different things

The changes are split into separate commits

what about first integrating m+ and then talk about the other stuff?

the m+ changes affect primarily one game, these changes affect me more.

your committing style is a bit worrying ... all kinds of unrelated stuff, if anyone were to find a regression in there it'd be awkward to find out what's wrong

What is unrelated?

Other preference changes is only one person's opinion at this point.

I invite more opinions.

Trying to get the best possible motion plus feature alone is what should be the focus

Same point as previous comment.

nothing is ever going to get done here until you separate the emulated motion plus code form the rest of what you've done there

Same point as previous comment.

"Please separate the changes marked with * out into their own branch:

These changes match the commits in this branch.
The merge is per commit so the branch doesn't matter.

We can merge the ones marked with * at a later stage."

Same point as previous comment.

The point of splitting is to make it easier reviewable ...

Same point as previous comment.

too much unnecessary stuff in it ... Randomly commenting out code, adding unused parameters."

What comments and parameters?

... Don't ball it all up in to a single patch that has everything in it ...

Same point as previous comment.

There are tons of things that I could continue on about ...

What things?

@tommyhl2.SS

... they've all told you the same thing.

Told me what?

... what other opinions you're looking for.

About what?

@NeoBrainX

afdfeaeebe34, ... keep stuff in separate commits

What should be in a separate commit?

given your past contributions to Dolphin I'm kinda opposed against merging ... your changes

What contributions?

it's ... not that hard to grasp what I'm talking about

Talking about what?

this is just dumb

What's dumb?

... if you ... weren't understanding stuff you'd ... ask

Ask about what?

... this deserves no further attention.

What deserves no attention?

john-peterson · 2013-05-31T22:48:08Z

Adding stick radius setting

Problem

The stick hardware max range is around 0,65 of the the max value

// GC main stick
max = (87.0 / 127.0) * 100;
diagonal = (55.0 / 127.0) * 100.0;
// GC C-stick
max = (74.0 / 127.0) * 100;
diagonal = (46.0 / 127.0) * 100;
// Nunchuk, Classic Controller
max = (82.0 / 127.0) * 100;
diagonal = (58.0 / 127.0) * 100;

according to

documentation
test in Zelda TP and SMS that the character move at full speed at this range

The optimal radius is directly outside PAD_Clamp because only values inside it is considered

Solution

Adding stick radius setting because

that makes it easier to adjust it

Adding visual aid for the hardware range because

that makes it easier to adjust the radius relative to it

The default value is 0.7f because

that's the radius for the stick with the largest range (87.0 / 127.0)

The radius is applies after the deadzone because

the deadzone shouldn't be dependent on the radius because that's not meaningful and harder to understand

This should be added even when considering that the range setting for the axes allow a similar effect because

the range doesn't have a circular effect, exemplified in this coordinate inequality for the range 0,65

[x*0,65, y*0,65] ≠ [cos(atan2(y,x))*sqrt(x*x+y*y)*0,65, sin(atan2(y,x))*sqrt(x*x+y*y)*0,65]

the range setting require four (one per axis) changes rather than one

Discussion

Reference

https://code.google.com/p/dolphin-emu/issues/detail?id=4980

JPeterson, stick radius is ok to merge

If there are no objections for one day I'll merge the stick radius, because your approval is adequate

[15:16] yeh, that should have been merged a long time ago.. i don't even remember the arguments against that patch were

There

Refactor

I want to change dist to rad in AnalogStick::GetState because it's a better name for radius

john-peterson · 2013-05-31T22:51:48Z

Fixing gyro emulation

Patch

The commit in this branch with the same title as this post

Discussion

[08:17] <+[SS]> Like emulated motion plus not being detected at times unless you unselect and reselect the motion plus box while the game is running.

This occur around 1/20 calibrations at the Zelda SS start screen

The solution is

reattachment M+ and/or Nunhuk

Rotation g-force

Red Steel 2 require accelerometer values that indicate g-forces in addition to gyro values to swing the sword

Rotation fast mode yaw and pitch swing the sword

Adding deceleration to force

A deceleration is added after acceleration to force because

it fixes Zelda SS shield, stab attack, spin attack, pull out sword of stone

Merge status

@tommyhl2.SS

<+[SS]> It's nice to have around for people too lazy to buy hardware.
[10:10] <+[SS]> Not to mention the thing will never connect/stay connected/be playable with real hardware anyway.

This comment approve the purpose of the patch

Refactor

"Tilt" should be changed to "rotate" because

it's a more general motion that can include yaw

"Force" (also called swing) should be changed to "thrust" because

acceleration followed by deceleration is

closer to the word thrust than swing
more precisely described by thrust than force

This is the refactor string table

Tilt                            Rotate
m_tilt                          m_rotate
tilt_group                      rotate_group
EmulateTilt                     EmulateRotate
GROUP_TYPE_TILT                 GROUP_TYPE_ROTATE
tilt                            rotation

Swing                           Thrust
m_swing                         m_thrust
swing_group                     thrust_group
EmulateSwing                    EmulateThrust
swing                           thrust
SWING_INTENSITY                 THRUST_INTENSITY
Force                           Thrust
GROUP_TYPE_FORCE                GROUP_TYPE_THRUST

john-peterson · 2013-05-31T23:09:26Z

Adding option to save input settings to ISO ini

Patch

The commit in this branch with the same title as this post

Problem

Input settings are sometimes different between games because the optimal settings for one game is different from that for another game

Especially emulated Wii (compared to GC) input often use unique settings because the accelerometer and gyro input allow many input setting combinations

It's slow to manually load game settings (through profiles)

Solution

Adding an option to save input settings to the ISO ini because

it can be faster than profiles to change between input settings

Function

When emulation is started input settings are read from the ISO ini (and the input dialogs are updated if they are open)

When emulation is stopped the settings are restored to the the default settings

During emulation the buttons "User Default"

Load
Save

read and write to User/Config/WiimoteNew.ini or User/Config/GCPadNew.ini because

that allow saving and loading the default settings during emulation when bInputSettingsISO is enabled

If the ISO ini lack input settings they are copied from the default input settings files the first time a game is booted

Profile

The profile ISO ini keys are changed from

[Controls]
GCProfile1 = name
WiiProfile1 = name

to

[Controls]
GCPad1Profile = name
Wiimote1Profile = name

because that's better organisations because that reduce string duplication because the strings GCPad1 and Wiimote1 exist

The [Controls] options has precedence over the Wiimote1 options because it would otherwise not be used

This is a description of the profile function

in b8691df it read /Config/Dolphin.ini

[Core]
GCProfile = name
WiiProfile = name

User/Config/name.ini

in e32b152 it read /GameConfig/ID.ini

GCProfile# = name
WiiProfile# = name

User/Config/Profiles/GCPad/name.ini
User/Config/Profiles/Wiimote/name.ini

It doesn't write the section, it's added manually by the user

This solution isn't altered because it doesn't conflict with the entries added in this patch, f.e.

[GCPad1]
Buttons/A = S

[Wiimote1]
Buttons/A = Q

Test

The patch is tested with

User/Config/Dolphin.ini

InputSettingsISO = False
InputSettingsISO = True

User/GameConfig/RSPE01.ini

GCPad1Profile = test
Wiimote1Profile = test

Discussion

@skidau

I am not keen on the game ini changes. The rest of the changes are fine.

The following communcation is about an abandoned idea to only load non-set settings

@LPFaint99

IniFile::Section::Copy loads the existing baseini settings (currently only IR settings) on first use.

Saving input settings to the isoini is made optional because the function has been challenged

The loading of only non-set keys (rather than all as in the usual profile function) is removed in favor of IniFile::Section::Copy that copy f.e. the GCPadNew.ini settings to the ISO ini if they aren't present

wxNotebook drawing bug

The drawing bug described here is

noticeable when UpdateGUI is called from f.e. OnPlay
not solved beacause the best known solution has a disadvantage

Unhandled exception

Problem

controllers is empty at this code when there's no GCPadNew.ini

bool InputPlugin::LoadConfig(bool def)
controllers[0]

Solution

Moving LoadConfig to the controllers iteration

Discussion

[04:32] @skid_au JPeterson, your patches look ok to me

john-peterson · 2013-05-31T23:12:47Z

Changing the input dialog from modal to non-modal

Patch

The commit in this branch with the same title as this post

Problem

The input configuration is modal

This is a problem because

the window with focus receive keyboard and mouse input
it's useful to test input settings
re-opening the dialog (to test another setting) use time
the emulated Wii accelerometer and gyro input allow many combinations and it's not obvious how the game respond to different input values

Solution

Making the input dialogs non-modal because

that solve the problem
non-modal (compared to modal) dialogs don't have a significant disadvantage

Thread safe controller interface

The CI is made thread safe because it's possible to call ControllerInterface::Initialize twice simultaneously by opening the GC and Wii input dialog simultaneously

Moving DInput HWND variable

The DInput HWND variable is moved because that's better organisation

Discussion

[04:32] @skid_au JPeterson, your patches look ok to me

john-peterson · 2013-06-02T09:24:01Z

Default DInput mouse

Problem

The DInput axis (relative) mouse input is less accurate than cursor (absolute) mouse input because

it appear to have a lower update frequency because it's harder to control with precision
its update frequency varies with emulation frame rate

Solution

Chaging the default DInput mouse input from axis to cursor because it's more accurate

This has an effect when CIFACE_USE_RINPUT is undefined so that both axis and cursor is present

john-peterson · 2013-06-02T19:05:27Z

Fixing input dialog rounding

Patch

The commit is in this branch with the same title as this post

Problem

The wrong value is sometimes saved because of rounding

The User/Config/WiimoteNew.ini value

Tilt/Angle = 117
Tilt/Angle = 117.000000
Tilt/Angle = 116.999992

is rounded to 116 when cast to int

((wxSpinCtrl*)wxcontrol)->SetValue((int)(value * 100))

Test

This is tested with

Visual Studio 12

The minimal code to produce the problem is

float f = 117;
f *= 0.01;
f *= 100;
printf("%d\n", int(f));
116

Solution

Rounding the non-exact float representation of and integer (f.e. 117) to an exact float representation before casting to int because

that cast to the expected int, f.e. 117 instead of 116

Alternative solution

wxSpinCtrl::GetValue 117 is saved as

Tilt/Angle = 116.999992

it's rounded to 116 when cast to int

((wxSpinCtrl*)wxcontrol)->SetValue((int)(value * 100))

Saving as this instead

Tilt/Angle = 117

doesn't solve the problem because

this value is also rounded to 116 when cast to int in

((wxSpinCtrl*)wxcontrol)->SetValue((int)(value * 100))

Discussion

[07:20] <_Jasper> [paraphrased] how does "117.000000" become 116? because

all integers in the range of 2^23 (for floats) can be represented exactly

Because of this transformation (betweeen IniFile::Load and wxSpinCtrl::SetValue)

f *= 0.01;
f *= 100;

[11:24] <_Jasper> [paraphrased]
what's "value"?

It's in InputConfigDiag.cpp

where does it come from?

It's loaded at ControllerEmu.cpp

sec->Get((group+(*si)->name).c_str(), &(*si)->value, (*si)->default_value*100);

[11:49] <_Jasper> [paraphrased] Should value be the integer 117 instead of the float 1.17?

No, PadSetting::value should have the type ControlState (float) rather than int because

it should be the same type as the value its multiplied with
it's multiplied with ControlState ControllerInterface::InputReference::State

Saving

Patch

This problem isn't solved in this branch

Problem

The value is saved with limited precision, f.e. 117 is saved

Set(key, StringFromFormat("%f", newValue).c_str())

as this value in vsnprintf because of limited float precision

Tilt/Angle = 116.999992

This is a problem because

the text " 116.999992" use more disk space than the text 117
when changing User/Config/WiimoteNew.ini in a text editor it's less clear if the decimals in 116.999992 have meaning (rather than not have meaning)

john-peterson · 2013-06-02T22:08:08Z

Changing the device combo box text to read only

Patch

The commit in this branch with the same title as this post

Problem

The profile combo box test is cleared when pressing "Save" which is a problem because

the next time Save" is used it's sometimes for the same profile as the previous

The device combo box text

accept unput which it shouldn't do because the it's not useful
can be empty

Solution

Fixing this by

changing the device combox box to a read only box
not clearing the profile text when pressing "Save"

john-peterson · 2013-06-05T14:12:19Z

Adding capture cursor function

Patch

The commit in this branch with the same title as this post

Problem

When using a non-fullscreen window it's benefical to capture the cursor in the window

Solution

Adding a capture cursor

function
hotkey
menu option

Compatibility with a separate device per mouse

This patch is compatible with "Creating a separate device per mouse" in the sense that ClipCursor confine the system cursor regardless of which device control it

X Window implementation

The HAVE_X11 code is

referenced from

tested with

vmmouse.present = "FALSE"

It use XGrabPointer rather than

XIGrabDevice because it lack confine_to
gdk_pointer_grab because with __WXGTK3__ it uses gdk_x11_device_xi2_grab that lack confine_to
XIGrabDeviceWithConfine because it hasn't been accepted

wxWidgets

There isn't a wxWidgets function to confine the cursor

wxWindow::CaptureMouse capture input but doesn't confine the cursor

Test

Holding the hotkey and running this in CFrame::CaptureCursor

static int i = 0;
i++;
wxASSERT(i % 2 == free);

i.o.w. observing

that every call after a capture uncapture and vice-versa

Discussion

[20:02] @Billiard JPeterson: only implemented on windows?

Yes because I don't know the Linux code for it

Update: a X Window implementation is added

… also yeh, is there a linux/osx implementation?

No

[20:02] @Billiard looks like it's in the gui on *nix though?

Do you mean this function exist in the Linux build? Where?

[16:38] <_Sonicadvance1> I'm not one to ask about that, I don't use mouse

[15:46] JPeterson, also, it seems you want to add multiple pointer support. Why aren't you using XI2, then?

It's not meaningful to use XIGrabDevice instead of XGrabPointer even if there's a separate device per pointer because

XGrabPointer confine the system cursor regardless of which device control it

Hotkey "Action, Key" header

Problem

The "Hotkey Configuration" dialog header line "Action, Key" is a problem because it's apparent

from the text left of the button that it's an action description
from the fact that the button labels are key names that they represent a key

Solution

Removing the "Hotkey Configuration" dialog header line because

it's not meaningful
it's distracting because of its limited meaning
it use screen space

wxWidgets' grab use

This information about wxWidgets' grab use can be of interest

wx use grab in these functions

Menu option

[01:15] @skid_au JPeterson, capture mouse should also have a menu equivalent..

What do you mean with a menu equivalent?

[13:28] @skid_au a menu equivalent to the hotkey, JPeterson

Do you mean that capture mouse should be a menu option?

That is only useful for capturing the cursor because when captured it can't be moved to the menu to release it

[13:34] @skid_au you can use the keyboard to access menus
[13:35] @skid_au makes as much sense as to make it a hotkey

A menu item is added

[15:30] @skid_au JPeterson, looks ok now

[02:40] @Sonicadvance1 JPeterson, I personally don't want the grab cursor code to be merged

[02:41] @Sonicadvance1 Because I don't think it is worth adding another option in the menus

This is in opposition to skid_au's opinion

Track grabbed state in X server rather than in client

Summary

Tracking the grabbed state in the X server (with X11Utils::IsPointerGrabbed) has been opposed in favor of

tracking it in the X client (in a variable called f.e. CFrame::m_bPointerGrabbed)

because

it's possible for another process to grab or ungrab the pointer between reading IsPointerGrabbed and acting on the information
this is a problem if such an occurence is a significant problem

However,

for the "Capture Cursor" function such an occurence isn't a significant problem (because of the reason in the topic "The implementation is thread safe")
tracking the grabbed state in a variable
- doesn't solve this problem because another process can grab the pointer without updating the pointer grab state variable (called f.e. CFrame::m_bPointerGrabbed)
- is an inferior solution because if another process release the pointer grab without updating the X client grab state variable (called f.e. CFrame::m_bPointerGrabbed) the user will be required to use the grab hotkey twice (ungrab an grab) to grab the pointer again

IsPointerGrabbed doesn't remove a grab

[15:38] <_Jasper> [paraphrased] Don't use IsPointerGrabbed because

if the user has a wx menu open it return true and the existing grab is removed

It's true that IsPointerGrabbed return true if a menu is open (because that grab the cursor for the menu with wxWindow::CaptureMouse because the menu should be closed if it lose focus)

It's not true that this is a problem for the hotkey because

Dolphin doesn't listen to a hotkey if a menu is open or a dialog has focus

[16:33] <_Jasper> [paraphrased] Is it true that Dolphin hotkeys are ignored if a menu is open?

Yes it's true

[16:37] Hotkeys aren't disabled by grab. It grabs with owner_events set to true and doesn't filter out key press / release events when a component has a grab. and then you'll drop wx's grab.

It's true that a grab doesn't disable hotkeys, but an open menu or dialog does

The implementation is thread safe

[15:45] <_Jasper> [paraphrased] It's not possible to thread safely check if another client has grabbed the pointer because

It's possible for another client to queue a GrabPointer request after you've already done the GrabPointer/UngrabPointer to test

It's not possible for dolphin-emu to queue a grab between IsPointerGrabbed and XGrabPointer because

the GUI doesn't run other code during running CFrame::CaptureCursor (because it doesn't run CFrame::CaptureCursor in parallel)

[02:30] <_Jasper> The thread unsafety is for other processes rather than the Dolphin GUI thread. X is asynchronous. You call into X by writing to a socket, and every once in a while X reads from every client's socket in order.

[06:08] Thread unsafety shouldn't be disregarded if there's a small probability for the occurence because that's an inappopriately high probability to allow thread unsafe occurences

[16:33] the estimated occurence probability can be lower than the actual occurence sampled from when the program is used

[06:08] this scenario is a problem

IsPointerGrabbed = false

other process grab the pointer

grab the pointer and release the existing grab that occured between IsPointerGrabbed and XGrabPointer

It's true that another process can grab or ungrab between IsPointerGrabbed and XGrabPointer

It's true that "Capture Cursor" release the existing pointer grab and confine the cursor to the window in this scenario

This isn't a problem because

the user probably expected the cursor to be confined to the window when this occurs

the user is unlikely to intentionally request another program to queue a grab in that time because it's short (around 3 ms according to a log message at the start and end of CFrame::CaptureCursor)

the result from possible scenarios aren't important problems

f.e. the scenario

the user grab the pointer for a menu by opening a menu between IsPointerGrabbed and XGrabPointer
XGrabPointer override the grab for the menu and confine the cursor to the video window
the menu is closed because it lose focus

the opposite scenario

IsPointerGrabbed = true
other process release grab between IsPointerGrabbed and XGrabPointer
CFrame::CaptureCursor release grab

isn't a problem because

the outcome of releasing the grab is the expected outcome

The alternative of tracking the pointer grab state in a variable (called f.e. CFrame::m_bPointerGrabbed) doesn't handle this better because

another process can grab the pointer without updating the pointer grab state variable (by writing false to a variable called f.e. CFrame::m_bPointerGrabbed)

f.e. in the scenario

CFrame is initiated with CFrame::m_bPointerGrabbed = false
another process capture the pointer without updating CFrame::m_bPointerGrabbed
the user call CFrame::CaptureCursor
CFrame::CaptureCursor read CFrame::m_bPointerGrabbed = false and capture the cursor, which release the existing capture

which isn't a problem because

the user expect the cursor to be confined to a window

and

CFrame::CaptureCursor set CFrame::m_bPointerGrabbed = true
another process release the pointer without updating CFrame::m_bPointerGrabbed
the user call CFrame::CaptureCursor
CFrame::CaptureCursor read CFrame::m_bPointerGrabbed = true and release the cursor instead of confining it

which is a problem because

the user expect the cursor to be confined to a window again (rather than it still being ungrabbed)

[16:39] The grabbed state should be tracked in the X client rather than the X server because

the X server fuction calls in sPointerGrabbed use many process cycles

it's better to track it in the X client (rather than the X server) because it knows when XGrabPointer/XUngrabPointer is called
The X server state isn't more reliable than the X client state
I've changed code in a window manager related to the focus state because it wasn't thread safe and the estimated problem occurence was lower than the actual problem occurence sampled from use of the program

This isn't true because

it hasn't been described that tracking the grab state in a variable solve the described problem
it's necessary to describe a scenario where it's a problem for this argument to be important

XGrabServer isn't an appropriate solution

[02:32] <_Jasper> [paraphrased] CFrame::CaptureCursor can be made thread safe by with

XGrabServer, then GrabPointer, then UngrabPointer, then GrabPointer, then XUngrabServer

but this is more complex than saving the pointer grab state in a CFrame variable in CFrame::CaptureCursor

XGrabServer isn't used because

preventing a grab or ungrab between IsPointerGrabbed and XGrabPointer has a small meaning because of the reason described above
XGrabServer can't be used before IsPointerGrabbed because it hang XOpenDisplay (that's opened to test if there's a grab in another display)

wxWidgets doesn't track all its grabs

[02:34] [paraphrased] The pointer grab state should be tracked in the X client because

wxWidgets tracks its pointer grab

Xlib doesn't track pointer grab

*The X server track pointer grab state but it shouldn't be asked for the grab state because of the arguments above

It's true that wx doesn't run XGrabPointer to test if there's a grab, but it's not true that it track a grab in a variable

F.e. wxPopupTransientWindow doesn't consider if there's an existing grab or the return value from XGrabPointer because

it's unlikely that there's an existing grab when the user open a wxPopupTransientWindow
removing an existing grab isn't an important problem

It's more reliable than the alternative

[15:43] JPeterson, just track whether you have a pointer grab in a member variable.

It's better to retrieve this information from the X server because

a function in wx or Dolphin can potentially release the grab for the m_RenderParent Display without updating the member variable
this is a problem becase selecting "Capture Cursor" again won't grab the cursor as the user expect

[02:33] If this happens, the code as written will break anyway.

No this doesn't result in a problem with this patch because

when CaptureCursor is called

it will grab the cursor again as the user expect (because it's ungrabbed) because

it'll receive information from XGrabPointer that there isn't a grab grab rather than information from CFrame::m_bPointerGrabbed that it's grabbed

It's good organisation

It's better organisation to track the GUI state in the GUI library than in the GUI client

[02:34] <_Jasper> [paraphrased] This communication isn't about a GUI library

With GUI library I mean the X server function IsPointerGrabbed call

Automatic capture

[02:46] <_Jasper> [paraphrased] Does CFrame::CaptureCursor capture the pointer only if at least one mouse input is bound in input configuration?

"Capture Cursor" doesn't consider input configuration because

the problem of capturing the cursor despite it having no benefit because mouse input is unused can also be solved by not using "Capture Cursor"
there's a complexity in iterating the input configuration to determine if the mouse is used

[05:59] <_Jasper> [paraphrased] It's not complex

[05:59] <_Jasper> "Capture Mouse" should be enabled automatically rather than non-automatically
by

if you click on the gameplay area while there's no grab, and you have mouse configured, it grabs the pointer within the gameplay area

because

that's an easier method to grab the cursor

Despite an automatic pointer grab there's a use for a hotkey because

it's useful for releasing the pointer grab
the key to release the grab should be configurable rather than non-configurable

john-peterson · 2013-06-15T22:51:03Z

Delay ControllerInterface and InputConfigDiag changes

Problem

The problem is

the probability of a merge conflict increase with time
ControllerInterface is refactored in https://github.com/magcius/dolphin-emu/commits/master
this branch has significant changes to ControllerInterface
if the refactoring is merged before this branch it creates a merge conflict

Solution

Delay refactoring

A solution is

delay ControllerInterface refactoring until the significant changes to ControllerInterface in this branch is merged

Merge this branch

A solution is

merge this branch with master in a reasonable time

@magcius

Your

approval of patches in this branch has value because

you have knowledge about ControllerInterface and InputConfigDiag becuse you've changed it
jordan-woyak, the author of most of the code changed in this branch, has made few comments about this branch

commit of the patches has value becuse

I don't have commit access to origin (since the change described in Travis CI)

Commit unmodified commit or branch

However

don't commit a modified commit or branch because of the reason described in Commit unmodified commit or branch
if there's a merge conflict ask me to resolve it because there can exist different opinions about the solution to a merge conflict

Discussion

[00:53] JPeterson, go ahead and land your thing
[00:53] JPeterson, I'll clean up the merge conflicts

[22:12] JPeterson, so are you going to merge your controller interface stuff? I can deal with the merge conflicts.

The reason the ControllerInterface change in input@john-peterson isn't merged is because it isn't approved

[06:36] @Jasper JPeterson, OK, what exactly do you want me to do? What changes do you want me to pull?

I want you to

approve patches in this branch
commit them if they merge automatically rather than fix a merge conflict because of the argument in the topic "Commit unmodified commit or branch"

[07:15] @Jasper You're asking me to merge in your changes, resolve tricky merge conflicts with commits with 60000 unrelated changes, and you won't accept my simple request of not speaking like a moon being?

That's wrong because

it's requested that you don't fix a merge conflict because of the argment in the topic "Commit unmodified commit or branch"

Status

Since 2011-11 around 30 hours have been used to resolve merge conflicts when rebasing this branch with master

There are changes to ControllerInterface and InputConfigDiag that create a merge conflict with this branch in

magcius

They will take 1 - 10 hours to resolve

john-peterson · 2013-06-15T23:37:14Z

Adding a device per mouse

Commit

In this branch with the same title as this post

Commit message

because

that allow using the mice for different controllers

Problem

Multiple mice can't be used because they're combined in one (DInput) device

Multiple mouse input is useful in multi player games that use IR input

Solution

Creating a separate device per mouse because

that allow using the mice for different controllers

Backend selection

The Windows mouse backend is RInput because

it's easy to use because there exist reference code (used in f.e. nuvee)

System mouse

The system mouse is enabled because

it's sometimes useful to control the same device input from all devices

DInput conflict

CIFACE_USE_RINPUT disable the DInput mouse axis (the cursor is not disabled) because

they can't poll the mouse at the same time
this isn't a disadvantage because the DInput and RInput axis input is equivalent

john-peterson · 2013-06-16T22:06:03Z

Adding technical name for ISO region

Discussion

[23:17] @Matt_P JPeterson: please rework this commit: john-peterson@7df42d7
[23:17] @Matt_P move GetRegion to the same location as GetCountry() etc

[00:52] @Matt_P JPeterson: m_strRegion = GetRegion(pVolume->GetCountry());
[00:52] @Matt_P should be reworked so that it is: m_strRegion = pVolume->GetRegion();

Write the filename and row of where it should be moved because it's not clear

Do you mean GetRegion should be in IVolume (in Volume.h) instead of SCoreStartupParameter?

[01:01] @Matt_P if those were your requirements
[01:02] @Matt_P how would you do it?

As the interpretation described in my question

[01:02] @Matt_P where is GetCountry currently ?

In IVolume

because a build test is useful

john-peterson · 2013-06-23T10:12:29Z

Adding gyro controls

IR and gyro synchronisation

IR isn't synchronised with gyro because of these problems with doing so

Horizontal cursor

Problem

The Zelda SS horizontal cursor is stuck when IR is visible

Solution

Hiding IR for < -0,25 and > 0,25 when gyro input is active reduce this problem

but

it might be beneficial to use the whole gyro range instead
the problem can be solved by hiding IR entirely

Pitch implied from IR

Problem

The Zelda SS vertical cursor is stuck without gyro input which is a problem if mouse input isn't bound to gyro input

Solution

Apply accelerometer pitch when IR isn't centered

Problem

if gyro and IR input is controlled by the same key the absolute IR position will become desynchronised and incorrectly apply a constant pitch

Fast mode

The fast gyro mode control is manual rather than

automatically activated when gyro input is large

because

for precision movement the full gyro range without fast mode is required

Settings

The accelerometer and gyro range is separate because it should sometimes be different

IR

"IR → IR Sensitivity" adjust

the IR input (both relative and absolute)
the pitch implied by IR (when M+ is active)

"IR → Hide" return 0 from GetState
"IR → Show" override "Hide IR" in GetIRData
"IR → Show" doesn't undo "IR → Hide" or vice versa

[08:16] <+[SS]> JPeterson: The fact that some things don't work very well, like moving the cursor around at various points when trying to select options at the start of the game.

The cursor works well if IR is hidden, i.o.w. "Hide IR" should be bound to a key because it allow efficient cursor movement

This is described in the issue Gyro emulation

Toggle keys for binary settings

[03:28] <_RachelB> JPeterson: hey, any interest in pulling out the options to toggle sideways/upright wiimote from john-peterson@7b256a5 and making a new commit with just those changes?

Yes

Name

"IR Range" and "Gyro Range" use the word range instead of sensitivity despite

that it applies to relative (rather than absolute) input
the word "IR Sensitivity" appear elsewhere in dialogs

because

range is an acceptable name for relative input too
it's shorter
it's consistent

Names for numbers

[06:46] <_Jasper> JPeterson, going through your branch now. You make unrelated and strange changes (like adding constants for completely other stuff) in "Adding gyro controls". Please split out your patches and squash later fixes.

Names are added for numbers in ControllerEmu.h because

that makes it easier to understand what the number mean

Describe why this should

be in a separate commit
not be commited

john-peterson · 2013-07-11T22:08:47Z

Changing extension during WM_SPEAKER_DATA

Problem

Disconnecting the Nunchuk during

the Wii Sports intro audio at 60 FPS

leave the Wiimote in a permanently unconnectable state with the output

06:43:172 Com[R] WM_SPEAKER_DATA: 52 18 a0 80 80 80 80 08 80 08 08 00 88 00 88 08 08 88 08 88 88 89 88 
06:43:187 Data[R] | id  | a1 37  | c  | a | -0.46 -0.46  0.50 | ir 1023 1023 1023 1023 | ext 
06:43:188 Com[R] WM_SPEAKER_DATA: 52 18 a0 88 88 08 80 80 80 00 08 00 00 08 08 00 80 00 80 80 00 80 00 
06:43:194 Data[R] | id  | a1 37  | c  | a | -0.42 -0.46  0.54 | ir 1023 1023 1023 1023 | ext 
06:43:204 Com[R] WM_SPEAKER_DATA: 52 18 a0 80 00 80 00 08 00 08 80 00 88 80 88 80 88 80 80 88 00 88 00 
06:43:217 Data[R] | id  | a1 37  | c  | a | -0.35 -0.46  0.54 | ir 1023 1023 1023 1023 | ext 
06:43:224 Com[R] WM_SPEAKER_DATA: 52 18 a0 08 00 00 80 00 80 08 88 00 88 88 08 88 88 88 08 88 88 88 08 
06:43:239 Data[R] | id  | a1 37  | c  | a | -0.35 -0.35  0.58 | ir 1023 1023 1023 1023 | ext 
06:43:245 Com[R] WM_SPEAKER_DATA: 52 18 a0 80 80 88 08 08 08 08 08 08 08 08 08 80 08 08 00 80 00 80 00 
06:43:261 Data[R] | id  | a1 37  | c  | a | -0.38 -0.58  0.62 | ir 1023 1023 1023 1023 | ext 
06:43:262 Com[R] WM_STATUS_REPORT
extension: 0: a1 20 00 00 1c 00 00 43 
06:44:264 Com[R] WM_SPEAKER_DATA: 52 18 a0 00 80 00 80 00 80 00 80 00 08 00 80 80 08 08 00 80 08 08 08 
06:45:276 Com[R] WM_SPEAKER_DATA: 52 18 a0 08 80 88 88 08 88 88 08 88 88 80 88 08 08 00 80 80 80 08 80 
06:46:286 Com[R] WM_SPEAKER_DATA: 52 18 a0 88 08 80 88 80 88 08 80 88 80 80 80 00 80 08 00 80 80 80 80 
06:47:295 Com[R] WM_SPEAKER_DATA: 52 18 a0 80 80 80 00 80 00 80 00 80 00 00 00 00 00 00 80 08 08 08 08 
06:48:309 Com[R] WM_SPEAKER_DATA: 52 18 a0 08 80 88 88 08 88 88 08 80 88 08 08 08 80 88 08 08 88 08 08 
06:49:335 Com[R] WM_SPEAKER_DATA: 52 18 a0 88 88 88 08 89 08 80 80 80 80 80 80 80 80 80 08 00 08 00 00 
06:50:347 Com[R] WM_SPEAKER_DATA: 52 18 a0 80 00 00 00 08 10 00 00 08 00 80 80 80 88 08 08 88 08 88 88 
06:51:364 Com[R] WM_SPEAKER_DATA: 52 18 a0 80 88 08 00 80 00 08 80 00 80 80 80 80 88 80 88 88 08 88 88 
06:52:375 Com[R] WM_SPEAKER_DATA: 52 18 a0 80 80 88 80 88 88 08 88 80 88 80 08 08 00 80 08 08 00 00 00 
06:53:387 Com[R] WM_SPEAKER_DATA: 52 18 a0 00 00 08 00 00 80 00 08 00 08 08 08 08 08 88 08 08 80 80 88 
06:54:396 Com[R] WM_SPEAKER_DATA: 52 18 a0 08 08 08 08 80 80 80 80 88 08 08 08 80 80 80 80 80 80 9a a8 
06:55:408 Com[R] WM_SPEAKER_DATA: 52 18 a0 11 7f 97 af 8f 29 71 10 0c ba 98 15 31 28 bd ba 80 36 22 09 
06:56:419 Com[R] WM_SPEAKER_DATA: 52 18 a0 cb ba 02 54 21 8b cc a8 03 53 20 9c cb a0 25 33 18 cb da 80 
06:57:432 Com[R] WM_SPEAKER_DATA: 52 18 a0 35 22 0a bd a9 82 44 21 9a db a8 14 42 20 ac cb 90 25 33 19 
06:58:443 Com[R] WM_SPEAKER_DATA: 52 18 a0 be aa 81 34 41 0a bd b9 02 53 21 9b db a8 14 43 28 ac cb a0 
06:59:456 Com[R] WM_SPEAKER_DATA: 52 18 a0 35 33 18 cc bb 81 45 21 09 cb b9 02 54 20 8b cb b8 14 52 28 
07:00:471 Com[R] WM_SPEAKER_DATA: 52 18 a0 9c bb a0 35 42 08 ad aa 81 34 32 0a db b9 83 54 11 8b cb a9 
07:01:483 Com[R] WM_SPEAKER_DATA: 52 18 a0 24 43 20 ad bb 98 35 33 19 be aa 81 35 21 0a bd a8 82 43 30 
07:02:495 Com[R] WM_SPEAKER_DATA: 52 18 a0 8c cb a8 23 62 10 ab cb 90 34 43 09 bc ca 82 35 31 8a cb ba 
07:03:506 Com[R] WM_SPEAKER_DATA: 52 18 a0 04 44 20 9a da a8 13 52 10 ab cb 90 34 42 18 cb ff b8 12 45 
07:04:267 Src\Main.cpp:769 N[Wiimote] Not connected
07:04:525 Com[R] WM_SPEAKER_DATA: 52 18 a0 10 89 98 88 8a 9c b4 62 00 8a 98 32 af ab 93 45 20 99 01 af 
07:05:540 Com[R] WM_SPEAKER_DATA: 52 18 a0 b8 01 43 18 aa 03 18 cc bc 85 40 80 08 91 30 cb ce a0 27 21 
07:06:549 Com[R] WM_SPEAKER_DATA: 52 18 a0 99 08 ab a9 98 54 30 98 81 10 cf ca 15 30 89 89 13 0a ea 89 
07:07:562 Com[R] WM_SPEAKER_DATA: 52 18 a0 81 37 20 88 9a cb 01 00 12 34 50 aa b0 8c f9 04 31 09 aa 16 
07:08:571 Com[R] WM_SPEAKER_DATA: 52 18 a0 29 bd aa 03 62 19 90 18 cc a0 02 10 03 72 89 ba ca 90 35 18

Settings

This occur

with

[Core]
WiimoteEnableSpeaker = True

because otherwise there aren't any WM_WRITE_SPEAKER_DATA

regardless of the code

Wiimote::InterruptChannel 
|| (!wm->m_status.speaker || wm->m_speaker_mute)

that change WM_WRITE_SPEAKER_DATA to WM_RUMBLE when the speaker is muted

because that information sometimes not meaningful

because * color makes it faster to identify elements of a log message

because color can indicate type of message

because it doesn't have meaning

because it's sometimes useful to copy sections between files

because it describe the technical difference between ISO regions more directly than the region name

because it's useful for cursor input

because it's not meaningful to change it

because that remove empty space

because that allow adjusting the force

because to test settings during emulation it's useful to have it open without blocking the main window

because that automate saving and loading ISO adjusted input settings

because * that allow using the mice for different controllers

because * it's sometimes more efficient to use an emulated rather than real gyro

because it's expected by some programs

because that's expected by some programs

* range setting for tilt and force * fast gyro toggle * IR toggle option because it's useful with gyro input * toggle keys for binary settings

Adding Travis CI script

6fadf5b

because a build test is useful

This was referenced Jun 28, 2013

gyro emulation #19

Open

CLI mode, benchmark from movie etc. #1

Open

john-peterson and others added 13 commits July 16, 2013 12:03

Adding option to log without file or type

95767b0

because that information sometimes not meaningful

Adding log colors

3dd00d2

because * color makes it faster to identify elements of a log message

Adding color option to OSD::DrawMessages

392f4aa

because color can indicate type of message

Adding math functions

fae44ce

Removing newline EOF requirement from ini

e350a62

because it doesn't have meaning

Adding copy ini section function

fc4e702

because it's sometimes useful to copy sections between files

Adding technical name for ISO region

4c149c6

because it describe the technical difference between ISO regions more directly than the region name

Adding capture cursor function

adb636b

because it's useful for cursor input

Changing the device combo box text to read only

e50d01e

because it's not meaningful to change it

Changing the input dialog range slider position

55a41a6

because that remove empty space

Fixing input dialog rounding

bcf8486

Adding range option to shake

fb626b9

because that allow adjusting the force

Changing the input dialog from modal to non-modal

1cd8c42

because to test settings during emulation it's useful to have it open without blocking the main window

john-peterson added 6 commits July 20, 2013 13:39

Adding option to save input settings to ISO ini

d6fbbcf

because that automate saving and loading ISO adjusted input settings

Adding a device per mouse

7b99fed

because * that allow using the mice for different controllers

Fixing gyro emulation

3d57b21

because * it's sometimes more efficient to use an emulated rather than real gyro

Adding deceleration after acceleration to force

9f2e902

because it's expected by some programs

Emulating acc g-forces from gyro values

2aceed9

because that's expected by some programs

Adding gyro controls

7b256a5

* range setting for tilt and force * fast gyro toggle * IR toggle option because it's useful with gyro input * toggle keys for binary settings

john-peterson mentioned this pull request Jan 29, 2014

Repo workflow #38

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RInput, gyro emulation #3

RInput, gyro emulation #3

john-peterson commented Dec 5, 2012 •

edited

Loading

john-peterson commented May 30, 2013 •

edited

Loading

john-peterson commented May 31, 2013

john-peterson commented May 31, 2013

john-peterson commented May 31, 2013

john-peterson commented May 31, 2013

john-peterson commented Jun 2, 2013

john-peterson commented Jun 2, 2013

john-peterson commented Jun 2, 2013

john-peterson commented Jun 5, 2013

john-peterson commented Jun 15, 2013

john-peterson commented Jun 15, 2013

john-peterson commented Jun 16, 2013

john-peterson commented Jun 23, 2013

john-peterson commented Jul 11, 2013

RInput, gyro emulation #3

Are you sure you want to change the base?

RInput, gyro emulation #3

Conversation

john-peterson commented Dec 5, 2012 • edited Loading

gyro emulation

files

gyro log

emulated input

gyroscope and accelerometer input

Settings

Axis mouse input

Range

gyro settle

Mouse and keyboard

Gamepad

Zelda SS

Moves

Key binding

FAQ

Red Steel 2

Moves

john-peterson commented May 30, 2013 • edited Loading

old discussions

Commit organisation

john-peterson commented May 31, 2013

Adding stick radius setting

Problem

Solution

Discussion

Refactor

john-peterson commented May 31, 2013

Fixing gyro emulation

Patch

Discussion

Rotation g-force

Adding deceleration to force

Merge status

Refactor

john-peterson commented May 31, 2013

Adding option to save input settings to ISO ini

Patch

Problem

Solution

Function

Profile

Test

Discussion

wxNotebook drawing bug

Unhandled exception

Problem

Solution

Discussion

john-peterson commented May 31, 2013

Changing the input dialog from modal to non-modal

Patch

Problem

Solution

Thread safe controller interface

Moving DInput HWND variable

Discussion

john-peterson commented Jun 2, 2013

Default DInput mouse

Problem

Solution

john-peterson commented Jun 2, 2013

Fixing input dialog rounding

Patch

Problem

Test

Solution

Alternative solution

Discussion

Saving

Patch

Problem

john-peterson commented Jun 2, 2013

Changing the device combo box text to read only

Patch

Problem

john-peterson commented Dec 5, 2012 •

edited

Loading

john-peterson commented May 30, 2013 •

edited

Loading