Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

[Feature] Add GazeInput.SetCursorBrush method to change the default GazeCursor color #3210

Closed
niels9001 opened this issue Apr 1, 2020 · 12 comments
Labels
feature request 📬 A request for new changes to improve functionality gaze 👀 good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities

Comments

@niels9001
Copy link

niels9001 commented Apr 1, 2020

Describe the problem this feature would solve

The current Toolkit implementation allows for setting the visibility of the gaze cursor and the radius of the ellipse.

The brush however, can not be changed - which is unfortunate because the default color does look great and having the flexibility to change it (for e.g. dark vs light themed UIs) to create more accessible apps would be great!

Describe the solution

  • Add a method that allows for setting the gaze cursor brush.

The current available methods are:
public static void SetIsCursorVisible(UIElement element, bool value);
public static void SetCursorRadius(UIElement element, int value);

The proposed method would be:
public static void SetCursorBrush(UIElement element, Brush value);

Additional context & Screenshots

@niels9001 niels9001 added the feature request 📬 A request for new changes to improve functionality label Apr 1, 2020
@ghost
Copy link

ghost commented Apr 1, 2020

Thanks for submitting a new feature request! I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

@michael-hawker
Copy link
Member

@niels9001 looks like you copied the signature, did you mean it to be?

public static void SetCursorBrush(UIElement, Brush value);

I don't know much about the internals of the Gaze helpers, @peteams thoughts? Is this easy to do?

@niels9001
Copy link
Author

@niels9001 looks like you copied the signature, did you mean it to be?

public static void SetCursorBrush(UIElement, Brush value);

I don't know much about the internals of the Gaze helpers, @peteams thoughts? Is this easy to do?

Oh yeah, sorry about that.. fixed!

I have true lack of C++ knowledge, else I would have put in a PR myself :)! I see in the code that an Ellipse is drawn and the fill is set to IndianRed. Not sure if we could create a simple method that overrides that (like the SetCursorRadius already does)?

@michael-hawker michael-hawker added this to the 6.1 milestone Apr 2, 2020
@michael-hawker michael-hawker added good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities labels Apr 2, 2020
@deanchalk
Copy link

Id really like an opportunity to work on this one if thats OK with everyone else

@niels9001
Copy link
Author

Id really like an opportunity to work on this one if thats OK with everyone else

That would be very much appreciated :)

@deanchalk
Copy link

Does this need to be assigned to me? Or shall I just start work on it :)

@azchohfi
Copy link
Contributor

azchohfi commented Apr 3, 2020

@deanchalk I just assigned it to you. Thanks for the help!

deanchalk pushed a commit to deanchalk/WindowsCommunityToolkit that referenced this issue Apr 4, 2020
@deanchalk
Copy link

OK, I have had a chance to look at this. The C++ control code is fairly straightforward, but we can achieve the desired result using the current GazeInteraction APIs.
Essentially, there is a GazeInput.GazeCursor.CursorElement property, which is of type UIElement, and represent the cursor. When you instantiate the GazeCursor, the C++ code simply sets the CursorElement as an Ellipse with a Fill of IndianRed (this is hard-coded in the constructor).
I have made changes to one of the sample pages (GazeInteraction) to include a color picker. When a color is picked, the sample page creates a new Ellipse (with the new color) and sets it to the CursorElement property .

HOWEVER....

I do not have a gaze tracking device, so cannot actually test this. The code change seems like it should do the job, when you delve into the C++ it seems very clear that this is what you have to do, but who knows - I may have coded something wrong.
I have created a pull request so other people (with an eye tracker) can check it out. If someone is willing to send the hardware to me on loan, I'd be happy to test the code and make sure it all works OK.

@niels9001
Copy link
Author

OK, I have had a chance to look at this. The C++ control code is fairly straightforward, but we can achieve the desired result using the current GazeInteraction APIs.
Essentially, there is a GazeInput.GazeCursor.CursorElement property, which is of type UIElement, and represent the cursor. When you instantiate the GazeCursor, the C++ code simply sets the CursorElement as an Ellipse with a Fill of IndianRed (this is hard-coded in the constructor).
I have made changes to one of the sample pages (GazeInteraction) to include a color picker. When a color is picked, the sample page creates a new Ellipse (with the new color) and sets it to the CursorElement property .

HOWEVER....

I do not have a gaze tracking device, so cannot actually test this. The code change seems like it should do the job, when you delve into the C++ it seems very clear that this is what you have to do, but who knows - I may have coded something wrong.
I have created a pull request so other people (with an eye tracker) can check it out. If someone is willing to send the hardware to me on loan, I'd be happy to test the code and make sure it all works OK.

Thanks for the clarification! What's the PR no.? I can try it out to see if it works.

@deanchalk
Copy link

#3221

@deanchalk
Copy link

To be honest, the c++ code could do with some improvements regarding it's API surface, plus ideally it would be upgraded from C++/CX to C++/WinRT. If I ever invest in an eye tracker I'd be happy to re-work this set of controls. Hopefully the PR I have submitted offers a working solution :)

@Kyaa-dost Kyaa-dost modified the milestones: 6.1, 7.0 Jun 22, 2020
@ghost ghost removed the help wanted Issues identified as good community contribution opportunities label Jun 22, 2020
@ghost ghost removed the In-PR 🚀 label Aug 5, 2020
@ghost ghost added the In-PR 🚀 label Aug 13, 2020
@ghost ghost added In-PR 🚀 and removed In-PR 🚀 labels Nov 17, 2020
@ghost ghost added the in progress 🚧 label Dec 9, 2020
@michael-hawker michael-hawker modified the milestones: 7.0, 7.1 Mar 1, 2021
@ghost ghost removed the in progress 🚧 label Mar 1, 2021
@michael-hawker michael-hawker added good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities and removed good first issue Issues identified as good for first-time contributors labels Apr 30, 2021
@ghost ghost removed the help wanted Issues identified as good community contribution opportunities label Apr 30, 2021
@ghost ghost removed the In-PR 🚀 label May 18, 2021
@michael-hawker michael-hawker added the help wanted Issues identified as good community contribution opportunities label May 18, 2021
@michael-hawker
Copy link
Member

To be honest, the c++ code could do with some improvements regarding it's API surface, plus ideally it would be upgraded from C++/CX to C++/WinRT. If I ever invest in an eye tracker I'd be happy to re-work this set of controls. Hopefully the PR I have submitted offers a working solution :)

Haven't heard from @deanchalk lately, hope everything is alright. Later last year for 7.0.0 we ported the gaze code to C#, so it should be easier to modify directly in the future for improvements like this one.

In the meantime, if anyone wants to take on taking a look at adding this function, let us know.

@michael-hawker michael-hawker modified the milestones: 7.1, 7.2/8.0? Aug 31, 2021
@ghost ghost removed this from the 7.2/8.0? milestone Aug 31, 2021
@CommunityToolkit CommunityToolkit locked and limited conversation to collaborators Jul 29, 2022
@LalithaNadimpalli LalithaNadimpalli converted this issue into discussion #4662 Jul 29, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
feature request 📬 A request for new changes to improve functionality gaze 👀 good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants