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

[UWP] Improvements 2 (Configs, Render, Input) #17952

Merged
merged 13 commits into from
Aug 26, 2023
Merged

[UWP] Improvements 2 (Configs, Render, Input) #17952

merged 13 commits into from
Aug 26, 2023

Conversation

basharast
Copy link
Contributor

@basharast basharast commented Aug 22, 2023

Hi, this is the second (phase2 of PR17350), almost last PR for UWP
(PR 17350 was not mentioned in the release log so I'm linking it here for future tracking)

this PR will address the following issues:

Initialize

  • Configs and App initialize now in better state
  • this change was important to handle user's custom configuration on startup
  • it shouldn't cause any further problems

Render device

  • I made some clean up on DeviceResources.cpp
  • Multiple adapters now possible
  • it will handle user's custom adapter selection
Screenshot

Post shaders

  • Tls problem for UWP was solved long time ago
  • it doesn't require any hacks at all

Read the it at TlsAlloc Remarks

Screenshot

Input keyboard

  • You can now use keyboard to write into text edit
  • On-Screen keyboard also works
  • It will help users to enter login data for RA
  • Keyboard check no longer always true, it has actual check now
Screenshot

Plus other minor fixes.

HWK & OSK

So now we can support Hardware keyboard and on-screen keyboard
the changes made on the UI was to add UI-Notification whenever:

  • TextEdit got focus, State: text_gotfocus
  • TextEdit lost focus, State: text_lostfocus
  • Popup closed, State: popup_closed

With those events the front end was able to manage Input pane and to forward chars to the NativeKey when required
on XBox it's a bit complicated because input could be DPad or Keyboard, but thankfully done.

Related issues

Fixes #17639

Hope you will find this PR helpful,

Many thanks.

@GABO1423
Copy link
Contributor

GABO1423 commented Aug 22, 2023

@basharast Please link issue #17639 to this PR so that when it is merged the issue is closed automatically. You can do this by just typing "Fixes #17639" into any part of the PR's description.

@GABO1423
Copy link
Contributor

Made a build to test this on Xbox, OSK does not seem to work still. But USB Keyboards now work.

The Render Device changes work perfectly, although you obviously don't want to change from the default on Xbox lol:
Unknown-2023_08_22-19_11_21

And shaders work perfectly:
Unknown-2023_08_22-19_12_46

@basharast
Copy link
Contributor Author

Ah nice thanks for the test, yeah as mention in the details at the end, I prepared the events to show/hide the OSK, but need to be linked with the UI text edit (focus state), so I hope this will be easy to be implemented

@hrydgard
Copy link
Owner

hrydgard commented Aug 23, 2023

Really good stuff!

I think at least on Xbox, we should hide Microsoft Basic Render Driver, though.

I think we can get this in even without the on-screen keyboard working on XBox, and fix that later. What do you think @basharast , or do you plan to make an attempt right away?

@hrydgard hrydgard added this to the v1.16.0 milestone Aug 23, 2023
@basharast
Copy link
Contributor Author

basharast commented Aug 23, 2023

The only reason I want to implement the focus got/lost event for text edit is to avoid OnCharacterReceived and OnKeyDown, OnKeyUp to send the same call at once (NativeKey)
and to prevent OnCharacterReceived from sending any call when no text edit has focus.

I will try to do this, and see how it can be better asap
as for Basic render, yeah as it's useless I will skip that from XBox

@basharast
Copy link
Contributor Author

I just updated the code to support the OSK case with few improvements, I hope I covered most cases, @GABO1423 could you please test this if it works fine on Xbox.

@GABO1423
Copy link
Contributor

Sorry for the late response, I'll make a build and test this in a bit, until then please hold off from merging this PR.

@hrydgard
Copy link
Owner

I won't merge until I get green light from both of you, don't worry :)

@GABO1423
Copy link
Contributor

Sad news, OSK is not working. In fact, trying to use it only causes the app to freeze, softlocking the app and requiring the user to close it. Another issue I found is Menu sounds do not work anymore, but I would have to test further to see if this PR caused the issue or if it was a regression from another commit.

@basharast
Copy link
Contributor Author

No problem, probably need a few changes.. it's almost done, I will try to manage to test this in someway directly, will check the menu sounds if there is something wrong soon.

@GABO1423
Copy link
Contributor

No problem, probably need a few changes.. it's almost done, I will try to manage to test this in someway directly, will check the menu sounds if there is something wrong soon.

I did some quick bisecting, and it seems that the issue is indeed caused by this PR. So at the very least it was not a regression from another commit. Should make narrowing down the issue a bit easier.

@basharast
Copy link
Contributor Author

basharast commented Aug 23, 2023

True, actually this line should be called before configs load

// Get install location
auto packageDirectory = Package::Current->InstalledPath;
const Path& exePath = Path(FromPlatformString(packageDirectory));
g_VFS.Register("", new DirectoryReader(exePath / "Content"));
g_VFS.Register("", new DirectoryReader(exePath));

// Mount a filesystem
g_Config.flash0Directory = exePath / "assets/flash0";

I didn't move it to the new initial function
this was fixed now, will try to solve the OSK and will push another update.

+UI sounds fix
@basharast
Copy link
Contributor Author

basharast commented Aug 23, 2023

I think it freeze because of this line:

if (IsXBox()) {
   return InputPane::GetForCurrentView()->Visible;
}

this may need to be called from UI thread,
it can be fixed but first need to be tested, let's hope, it's the problem.

@GABO1423
Copy link
Contributor

this may need to be called from UI thread, it can be fixed but first need to be tested, let's hope, it's the problem.

Making a build to do just that right now.

@GABO1423
Copy link
Contributor

After a lot of work over at Discord, we finally made the OSK work! So when the relevant commits are pushed, this PR is ready to go.

- Renamed few functions to avoid confusion
- Now UI will report text (gotfocus, lostfocus), popup_closed to the frontend
- Both cases (DPad, Keyboard) covered
@basharast
Copy link
Contributor Author

As mentioned we reached a stable state, many thanks to @GABO1423 for his time it wasn't possible without testing on real XBox, PR descriptions updated.

Removing unused include and cleaned the Tls TODO notice
@hrydgard
Copy link
Owner

Very cool, nice work guys!

@hrydgard hrydgard merged commit d35529e into hrydgard:master Aug 26, 2023
18 checks passed
@basharast basharast deleted the configs-loading branch August 26, 2023 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Xbox UWP) On-screen Keyboard is not working
3 participants