Converted GazeInteraction project from C++ to C#. #3427

azchohfi · 2020-08-13T03:01:27Z

Unifies Toolkit language to be only C#.

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Refactoring (no functional changes, no api changes)

What is the current behavior?

C++, boo.

What is the new behavior?

All C# 🦙🎉

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Pull Request has been submitted to the documentation repository instructions. Link:
Sample in sample app has been added / updated (for bug fixes / features)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

This is not exporting WinMDs, so C++ devs will be stuck on 6.x, and this is expected.

ghost · 2020-08-13T03:01:29Z

Thanks azchohfi for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

Kyaa-dost

🚀 🚀

azchohfi · 2020-08-13T23:17:56Z

Two awesome things about this PR:

Build times reduced ~15%.
The old NuGet package had 7.8Mb, whereas the new NuGet package is 39KB!

michael-hawker · 2020-09-01T22:55:57Z

@azchohfi just tested in the sample app and was working great! For some reason the store app interaction traction wasn't working for me, but worked perfectly here with the new code! 🎉🎉🎉

michael-hawker

Hey @azchohfi some initial feedback on the port. I think there's a few things we could take the opportunity to improve with the change to C#. Feel free to just say that we should file issues for future improvements on things. There definitely did seem to be an issue with ComboBox though.

Haven't quite completed looking at everything, but figured this would get us started again on trying to wrap this up. Thanks!

Microsoft.Toolkit.Uwp.Input.GazeInteraction/DwellInvokedRoutedEventArgs.cs

Microsoft.Toolkit.Uwp.Input.GazeInteraction/DwellProgressEventArgs.cs

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeEventArgs.cs

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeFeedbackPopupFactory.cs

michael-hawker · 2020-09-01T23:29:06Z

Microsoft.Toolkit.Uwp.Input.GazeInteraction/ComboBoxItemGazeTargetItem.cs

+
+namespace Microsoft.Toolkit.Uwp.Input.GazeInteraction
+{
+    internal class ComboBoxItemGazeTargetItem : GazeTargetItem


I added a ComboBox to the example:

<ComboBox SelectedIndex="0" Margin="100,100"> <x:String>One Thing</x:String> <x:String>Two Things</x:String> <x:String>Three Things</x:String> <x:String>Four Things</x:String> </ComboBox>

However, while it opens the combobox, it doesn't show the cursor or allow me to invoke item selection with it. Is that because we're missing the XamlRoot now or something else?

I don't think this is related to XamlRoot. Not sure what is happening. What was the behavior on the old version?

The old version wasn't working for me at all, so I couldn't compare... @peteams was the cursor supposed to move to the pop-up element and show in the drop-down?

Alright, so investigated this a bit. The old code did a better job with the highlight rectangles on the ComboBox, even though I've fixed that now in the C#, the code is the same within the GazeTargetItem.RaiseProgressEvent method, so I'm not sure why it behaves differently, though maybe has to do with how the visual tree is searched in that logic? Doesn't seem to make a difference using x:String elements vs ComboBoxItem elements directly in my example above.

I am basically noticing that sometimes it does find the ComboBoxItem and can select it, other times it shows the highlight rectangle briefly, but most times there's no visual indications (the cursor is behind the pop-up too), and it seems like the UIElement selector is grabbing the 'TextBlock' element within the ComboBoxItem instead of looking up and grabbing the ComboBoxItem itself, this I think is the main thing breaking the logic here. Will try and poke a bit more, but may be easier to have someone from @peteams group take a look and help us with this fix.

Since this also wasn't working great in the C++ version, I think we should just add it to our #3497 list or a separate issue, and not block this PR. I think the PR should be good to go otherwise.

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazePointer.cs

michael-hawker · 2020-09-01T23:59:20Z

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeSettingsHelper.cs

+            // Setup a new app service connection
+            AppServiceConnection connection = new AppServiceConnection();
+            connection.AppServiceName = "com.microsoft.ectksettings";
+            connection.PackageFamilyName = "Microsoft.EyeControlToolkitSettings_s9y1p3hwd5qda";


Is this the system app? Is this documented somewhere we should probably have a comment? FYI @peteams

I couldn't find docs for this anywhere.

From an internet search, it almost appears like it's grabbing from an app service that's only from a sample here:

<Extensions> <uap:Extension Category="windows.appService"> <uap3:AppService Name="com.microsoft.ectksettings" SupportsRemoteSystems="true" /> </uap:Extension> </Extensions>

@joncamp do you remember what this is about? It's unlikely for someone to compile and install this other sample app locally. Should we bother having this extra code in the Toolkit? It seems like it's not referenced anywhere within the codebase itself?

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeTargetItem.cs

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazePointer.cs

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeStats.cs

vgromfeld

michael-hawker · 2020-09-10T16:48:49Z

@azchohfi since this is already working better than what was in the Toolkit, should we merge this and open an issue to track the ComboBox problem separately?

azchohfi · 2020-09-10T16:56:29Z

I believe we should, yes.

As an extra precaution against accidental merge

michael-hawker · 2020-09-22T20:45:39Z

@azchohfi any idea why I'm seeing the output directories in this project?

Is this if I don't clean when switching from the C++ project?

azchohfi · 2020-09-22T20:47:04Z

It might be, but it's fine to see those files there, they won't be built or anything.

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeTargetItem.cs

michael-hawker · 2020-09-22T23:32:48Z

Think all the minor details are smoothed out enough for this PR. Should follow-up on the Settings with the Gaze team as well as a general review, and later as another PR we should fix the ComboBox behavior for #3497, and investigate tidying up some code and performance with our VisualTree investigation as part of #3487.

It'd also be good to know if we can mimic these Gaze events with WinAppDriver or other tooling so that it's easier to try and reply specific scenarios for debugging and testing.

michael-hawker · 2020-09-28T17:29:32Z

@azchohfi package step failed now?

azchohfi · 2020-09-28T17:37:46Z

Oh, that is the design package that is not updated properly. I can fix that.

Seems to be 00:00:00 a lot?

…mmunityToolkit into gazeCS

…yncAction, to keep this from being a breaking change.

michael-hawker

Let's move ahead and we can fix anything else in the future, but pretty sure we're in the same space we should have been with the C++ version.

ghost · 2020-10-06T17:50:40Z

Hello @michael-hawker!

Because this pull request has the auto merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

ghost requested review from michael-hawker and Kyaa-dost August 13, 2020 03:01

azchohfi changed the title ~~Converted GazeInteraction project for C++ to C#.~~ Converted GazeInteraction project from C++ to C#. Aug 13, 2020

azchohfi force-pushed the gazeCS branch from 22b8fc1 to 5cac2bd Compare August 13, 2020 03:02

Converted GazeInteraction project for C++ to C#.

910de59

azchohfi force-pushed the gazeCS branch from 5cac2bd to 910de59 Compare August 13, 2020 03:02

Kyaa-dost previously approved these changes Aug 13, 2020

View reviewed changes

azchohfi mentioned this pull request Aug 13, 2020

Add C++ workload to the dependencies list #3300

Closed

7 tasks

This was referenced Aug 18, 2020

#3210 Add GazeInput.SetCursorBrush method #3221

Closed

Add a Vertical Text Block to the Windows Community Toolkit #2544

Closed

azchohfi force-pushed the gazeCS branch 2 times, most recently from 3d5d324 to eff69f8 Compare August 24, 2020 18:12

Merge branch 'master' into gazeCS

2a1ab20

azchohfi force-pushed the gazeCS branch from eff69f8 to 2a1ab20 Compare August 24, 2020 18:21

Kyaa-dost added this to the 7.0 milestone Aug 25, 2020

michael-hawker added reviewers wanted ✋ for-review 📖 To evaluate and validate the Issues or PR labels Sep 1, 2020

Merge branch 'master' into gazeCS

934c810

michael-hawker reviewed Sep 2, 2020

View reviewed changes

vgromfeld reviewed Sep 2, 2020

View reviewed changes

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeStats.cs Show resolved Hide resolved

Addressed PR feedback.

dcdf649

azchohfi requested review from michael-hawker and vgromfeld September 2, 2020 18:00

vgromfeld approved these changes Sep 3, 2020

View reviewed changes

Merge branch 'master' into gazeCS

83fe4dd

michael-hawker reviewed Sep 22, 2020

View reviewed changes

Microsoft.Toolkit.Uwp.Input.GazeInteraction/GazeTargetItem.cs Outdated Show resolved Hide resolved

michael-hawker added 2 commits September 22, 2020 14:17

Add text for Gaze sample Button to make it easier to focus on.

5e4f30e

Fix logic that was preventing progress reporting on Gaze events

a9a391c

This was referenced Sep 22, 2020

GazeElement StateChanged not working on TextBox? #3100

Closed

[Feature] Improve Toolkit controls support for Gaze #3497

Closed

Fix null exception crash when removing button from Gaze sample page

37639f8

azchohfi added 2 commits September 24, 2020 11:09

Merge branch 'master' into gazeCS

bc37162

Merge branch 'master' into gazeCS

6b5ce95

Bump gaze interaction target version.

4fa869b

azchohfi force-pushed the gazeCS branch from b8a4b9e to 4fa869b Compare September 28, 2020 19:17

michael-hawker and others added 5 commits September 28, 2020 14:12

Comment extra debug line

bfdc1f6

Seems to be 00:00:00 a lot?

Fixed initialization order of GazeInput's static properties.

05950af

Merge branch 'gazeCS' of https://github.com/windows-toolkit/WindowsCo…

a6bf4b7

…mmunityToolkit into gazeCS

Merge branch 'master' into gazeCS

19722d8

Changing back GazeSettingsHelper.RetrieveSharedSettings to return IAs…

6119b93

…yncAction, to keep this from being a breaking change.

michael-hawker approved these changes Oct 6, 2020

View reviewed changes

Merge branch 'master' into gazeCS

91c8704

michael-hawker added the auto merge ⚡ label Oct 6, 2020

Merge branch 'master' into gazeCS

1a28ca7

ghost merged commit 6583c81 into master Oct 6, 2020

ghost deleted the gazeCS branch October 6, 2020 22:05

michael-hawker mentioned this pull request Oct 13, 2020

[Future] 7.0 Release Plan #3062

Closed

70 tasks

michael-hawker mentioned this pull request Mar 2, 2021

[Add Test] Unhandled exception when gazing at Flyout #3525

Closed

This pull request was closed.

Converted GazeInteraction project from C++ to C#. #3427

Converted GazeInteraction project from C++ to C#. #3427

Uh oh!

Conversation

azchohfi commented Aug 13, 2020

Unifies Toolkit language to be only C#.

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Uh oh!

ghost commented Aug 13, 2020

Uh oh!

Kyaa-dost left a comment

Choose a reason for hiding this comment

Uh oh!

azchohfi commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michael-hawker commented Sep 1, 2020

Uh oh!

michael-hawker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michael-hawker Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

azchohfi Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michael-hawker Sep 1, 2020

Choose a reason for hiding this comment

Uh oh!

azchohfi Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

michael-hawker Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vgromfeld left a comment

Choose a reason for hiding this comment

Uh oh!

michael-hawker commented Sep 10, 2020

Uh oh!

azchohfi commented Sep 10, 2020

Uh oh!

michael-hawker commented Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azchohfi commented Sep 22, 2020

Uh oh!

Uh oh!

michael-hawker commented Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michael-hawker commented Sep 28, 2020

Uh oh!

azchohfi commented Sep 28, 2020

Uh oh!

michael-hawker left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 6, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

azchohfi commented Aug 13, 2020 •

edited

Loading

michael-hawker Sep 22, 2020 •

edited

Loading

michael-hawker commented Sep 22, 2020 •

edited

Loading

michael-hawker commented Sep 22, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.