Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge quest-pro into main ready for release #122

Merged
merged 145 commits into from Mar 25, 2023
Merged

Merge quest-pro into main ready for release #122

merged 145 commits into from Mar 25, 2023

Conversation

benaclejames
Copy link
Owner

@benaclejames benaclejames commented Mar 10, 2023

Pretty self explanatory, merge conflicts need fixing but a final go-ahead from the main contributors would be wonderful in the meantime

Closes #106

Adjerry91 and others added 30 commits November 5, 2022 15:46
Removed parameter scaling on the brows as it was saturating the parameters on my headset. Updated the tracking angle limits to normalize to the quest pro eye angles. Testing in VRChat is way closer to SRanipal eye tracking.
Squint was separated out as was causing problems with mapping to squeeze with the combined eye lid expanded squeeze.
special Quest Pro tracking parameters should be handled from the extTrackingInterface to make the system modular for VRCFT. Currently the extTrackingInterface is embedded in the VRCFT exe this needs to be removed from the main code.  Added Jaw Drop to work with MouthTowards for testing purposes instead compensating in code.
Enhance MouthApeShape, EyeClose, and CheekPuff controls and removed mouthtowards and jawdrop to be unified to shapes.
…vacancies. Removed SRanipal dependencies. Updated included tracking modules appropriately.
…edTracking and made the structuring of the class much clearer. Updated modules.
Comment on lines +66 to +72
public struct UnifiedExpressionShape
{
/// <summary>
/// Value that contains the specified Unified Expression raw value.
/// </summary>
public float Weight;
}
Copy link
Collaborator

@regzo2 regzo2 Mar 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the weight in a struct early on in the UE builds, and they used to contain all the Mutator data as well, but after moving those out of the Shape structs there only exists Weight now. I'm not entirely sure what else would be contained here in the future but it would only be data that is meant to directly affect the shapes (maybe something along the line sof an optional Curve modifier shape in the future for example?).

Comment on lines +18 to +22
public float Ceil; // The maximum that the parameter reaches.
public float Floor; // the minimum that the parameter reaches.
//public float SigmoidMult; // How much should this parameter be affected by the sigmoid function. This makes the parameter act more like a toggle.
//public float LogitMult; // How much should this parameter be affected by the logit (inverse of sigmoid) function. This makes the parameter act more within the normalized range.
public float SmoothnessMult; // How much should this parameter be affected by the smoothing function.
Copy link
Collaborator

@regzo2 regzo2 Mar 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the calibrator Mutation includes only a ceiling and floor calibration for each parameter, and I included some commented out ones for adjusting the profile of each parameter (one that makes it more toggle like and one that makes it more curve like). Basically the Mutator class was really intended on VRCFT itself allowing users to modify how expressions appear or feel like on a user level, though how this class could be modified by modules isn't yet entirely clear.


var paramsToCreate = new Dictionary<string, int>();
foreach (var param in boolParams)
{
// Cut the parameter name to get the index
if (!int.TryParse(param.name.Substring(_paramName.Length), out var index)) continue;
if (!int.TryParse(String.Concat(param.name.ToArray().Reverse().TakeWhile(char.IsNumber).Reverse()), out var index)) continue;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's are some good test inputs for this? There might be a simpler way to do it but I want to see what it should work on

Copy link
Collaborator

@regzo2 regzo2 Mar 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part specifically is for parsing out the int from a binary parameter (eg. v2/JawOpen4, v2/JawOpen2, v2/JawOpen1...), it basically counts from end to start and gets the first number it encounters and reverses it back. So v2/JawOpen16 would give int 16 for example. This is to work around the possibility that a loaded param won't strictly contain the base param name (eg. JawOpen) from the regex parsing (which could be like v2/JawOpen).

Basically any sets of binary params would be great to test, even testing out different nested patterns as well, like:

gamer/gunk/v2/JawOpen16
test/v2/JawOpen8
123123123/v2/JawOpen4
w/t/f/v2/JawOpen2
v2/JawOpen1

v2/JawOpen being the base parameter.

using VRCFaceTracking;
using VRCFaceTracking.Params;

namespace ALVRTrackingInterface
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to all ExtTrackingModules that are included with the VRCFaceTracking repo (ALVR module and SRanipal module respectively):

What do you all think of having the included external modules in separate repos from VRCFaceTracking? I feel that it would help better communicate that this repo is fully encapsulated by the VRCFaceTracking namespace alone, and any external modules are outside of that. It would also help keep commits done to the external modules separate from commits done to the actual VRCFaceTracking namespace? The installer would still include them however.

…stent with other names for Expression. Removed convergence from Focus 3 / Droolon eye fix (will implement other workarounds). Renamed ExternalExtTrackingModule to ExampleExtTrackingModule to better match it's purpose.

// Eye dilation code, automated process maybe?
eye.Left.PupilDiameter_MM = 0.0035f;
eye.Right.PupilDiameter_MM = 0.0035f;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EyesDilation on legacy parameters is being driven to 0 with input from ALVR, users must disable eye dilation to go back to default. I believe the mutators zeroing out the pupil diameter with no input. Previous QuestPro controls would just hold default value of 0.73 on the float value.

Copy link
Collaborator

@regzo2 regzo2 Mar 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .73 value you're talking about was controlling the old EyesDilation parameter directly (which just represented the value between 0-1), currently the value that is expected is the exact MM value from an interface, which then on runtime is used to create the old EyesDilation ranges.

I can just have it so that this interface sets the dilation normalization ranges from 0 - 10, and then the pupil diameter can be set to whatever it needs to be (which we would expect to be between 5 - 6 MM for normal pupil diameter ranges)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See if 74f3b62 resolves your issue!

@benaclejames
Copy link
Owner Author

image
Not sure what's going on here. This error is thrown when using vrcft on a fresh machine with an empty appdata folder

@regzo2
Copy link
Collaborator

regzo2 commented Mar 11, 2023

image Not sure what's going on here. This error is thrown when using vrcft on a fresh machine with an empty appdata folder.

SRanipal SDK libs need to be in the ModuleLibs folder, though I was wondering if embedding them in the dll would be more ideal? The post-build events should add the SDK dlls there tho 🤔

@PinballsWizard
Copy link
Contributor

PinballsWizard commented Mar 11, 2023

There are a few things that we need to make sure are retained as the conflicts get merged that are already a part of main, and might not necessarily show up as conflicts b/c of the file restructure (as of Nov 5, 2022):

With regards to points 1 and 3:
1 - fc76bf9 - disable-args

3 - 0865da9 - possibly these changes to UnifiedLibManager (from disable-args changes)?

Both the initial changes for disable-args and the related changes were recreated/present in this PR in 2135e8c

I'm sure it will need to be touched again for refactor/args later as well

@korejan
Copy link

korejan commented Mar 11, 2023

Please rename the ALVR module to ALXR, it is the ALXR client that the module recieves data from, not the alvr server. For Quest Pro, the alxr client supports both (Air)Link users without the ALVR server and ALVR-server users.

It will make less sense soon to call it ALVR as I'm currently working on adding more facial & eye tracking OpenXR extensions (for example XR_HTC_facial_tracking is done) to ALXR and updaing the vrcft module to support them. This is for both wired and standalone headsets. I am also planning to make a new option for the module to use the alxr-engine DLL directly (for OpenXR PC runtimes like Link).

@Adjerry91
Copy link
Contributor

Loading VRCFacetracking it does not load the module on start in the UI you have to click the reload modules.
image

@regzo2
Copy link
Collaborator

regzo2 commented Mar 11, 2023

Please rename the ALVR module to ALXR, it is the ALXR client that the module recieves data from, not the alvr server. For Quest Pro, the alxr client supports both (Air)Link users without the ALVR server and ALVR-server users.

It will make less sense soon to call it ALVR as I'm currently working on adding more facial & eye tracking OpenXR extensions (for example XR_HTC_facial_tracking is done) to ALXR and updaing the vrcft module to support them. This is for both wired and standalone headsets. I am also planning to make a new option for the module to use the alxr-engine DLL directly (for OpenXR PC runtimes like Link).

I personally think having the ALVR/ALXR external tracking module in it's own repo would be best so that you can make changes needed to the module (and I'm trying to get opinions from the other contributors on moving the other external tracking modules that are in this repo into their own repos as well). If you want to make a dedicated repo for the ALVR/ALXR module then feel free to do so!

@regzo2
Copy link
Collaborator

regzo2 commented Mar 11, 2023

Loading VRCFacetracking it does not load the module on start in the UI you have to click the reload modules. image

This is mostly a result of the main thread and UI threads not entirely initializing synchronously, and also having the UI listbox fetching module data on init. A lot of the initialization processes such as the module listbox try loading data on UI init, which is then reliant on main fully propogating the data and making the appropriate calls. We could look into having a finalized init on main finalizing or hooking into WPF events to propogate data to the UI to circumvent this, though if we plan on moving to a newer UI framework then it might be best to wait for that (and i'm not sure of the release window for this current build).

@regzo2 regzo2 linked an issue Mar 16, 2023 that may be closed by this pull request
@korejan
Copy link

korejan commented Mar 20, 2023

Please rename the ALVR module to ALXR, it is the ALXR client that the module recieves data from, not the alvr server. For Quest Pro, the alxr client supports both (Air)Link users without the ALVR server and ALVR-server users.
It will make less sense soon to call it ALVR as I'm currently working on adding more facial & eye tracking OpenXR extensions (for example XR_HTC_facial_tracking is done) to ALXR and updaing the vrcft module to support them. This is for both wired and standalone headsets. I am also planning to make a new option for the module to use the alxr-engine DLL directly (for OpenXR PC runtimes like Link).

I personally think having the ALVR/ALXR external tracking module in it's own repo would be best so that you can make changes needed to the module (and I'm trying to get opinions from the other contributors on moving the other external tracking modules that are in this repo into their own repos as well). If you want to make a dedicated repo for the ALVR/ALXR module then feel free to do so!

I want to apologize for being rude, sorry for being confrontational.

I'm totally fine with making a dedicated repo. It would have been what I would do but I wasn't originally involved on the vrcft module side of it until recently.

I have a fork of the quest-pro branch and have branches off of it with my work so I'm regularly rebasing them.

Let me know how/when you want do this and thank you for updating the module for the unified expression work.

@regzo2
Copy link
Collaborator

regzo2 commented Mar 25, 2023

Please rename the ALVR module to ALXR, it is the ALXR client that the module recieves data from, not the alvr server. For Quest Pro, the alxr client supports both (Air)Link users without the ALVR server and ALVR-server users.
It will make less sense soon to call it ALVR as I'm currently working on adding more facial & eye tracking OpenXR extensions (for example XR_HTC_facial_tracking is done) to ALXR and updaing the vrcft module to support them. This is for both wired and standalone headsets. I am also planning to make a new option for the module to use the alxr-engine DLL directly (for OpenXR PC runtimes like Link).

I personally think having the ALVR/ALXR external tracking module in it's own repo would be best so that you can make changes needed to the module (and I'm trying to get opinions from the other contributors on moving the other external tracking modules that are in this repo into their own repos as well). If you want to make a dedicated repo for the ALVR/ALXR module then feel free to do so!

I want to apologize for being rude, sorry for being confrontational.

I'm totally fine with making a dedicated repo. It would have been what I would do but I wasn't originally involved on the vrcft module side of it until recently.

I have a fork of the quest-pro branch and have branches off of it with my work so I'm regularly rebasing them.

Let me know how/when you want do this and thank you for updating the module for the unified expression work.

Here is an update: We have talked about what our plans are as far as rebasing the modules. Currently we are interested in working towards a rebase of all the modules into their own repositories, but nothing is completely concrete in when or how we are going to do so. We are planning on making this a focus at the very least after the next VRCFT release, and any suggestions to have it before then would be really helpful too!

@benaclejames benaclejames merged commit 63befcb into master Mar 25, 2023
1 check passed
@benaclejames benaclejames deleted the Quest-Pro branch August 6, 2023 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
6 participants