Skip to content

Windows: text input #161

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

Merged
merged 25 commits into from
Jan 1, 2019
Merged

Conversation

clarkezone
Copy link
Collaborator

Bring the Linux embedder keyboard input support over to the Windows embedder.

clarkezone and others added 6 commits December 9, 2018 13:34
… remove binary dependency on jsoncpp and compile from source instead.
…r functions, text input plugin headers / source into a glfw director
Collapses common/src/ to just common/ since there's no longer another
directory in common/.

Also moves linux embedder.h into include/linux to unify include dir.
@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Dec 11, 2018

Pushed changes that fix Linux. It ended up being kind of ugly since making my GN include/ aggregation not flatten the structure was non-trivial, so there are a few conditional includes in the Linux code now. Once build systems is settled I can improve the situation.

Some things to note:

  • I moved the files some more; common/include/ is now just include/, and common/src/ is now just common/, since having include/ be right under library/ makes more sense in the Make-style build where clients are pointing a tree locations for includes. I edited the VS projects manually on Linux to account for that, but I expect I missed something and you'll need to clean that up.
  • I merged with master to resolve conflicts with the changes from adding key events on Linux. Feel free to leave key events for Windows out of this patch, but it might be worth adding that file and seeing if it Just Works given that it's all GLFW.

@stuartmorgan-g
Copy link
Collaborator

Feel free to leave key events for Windows out of this patch, but it might be worth adding that file and seeing if it Just Works given that it's all GLFW.

I decided to test this branch out on Windows to see if I broke anything, and then thought I'd check this. It does work, as expected: https://github.com/stuartmorgan/flutter-desktop-embedding/tree/windows-key-events

I'll land that as a follow-up patch to avoid adding even more things to this patch though.

@clarkezone
Copy link
Collaborator Author

Feel free to leave key events for Windows out of this patch, but it might be worth adding that file and seeing if it Just Works given that it's all GLFW. ...

@stuartmorgan will do

Sent with GitHawk

@clarkezone
Copy link
Collaborator Author

All done, awaiting input.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Looking good! Just a few minor things (I'd fix them myself, but I don't have my Windows machine at the moment and don't want to accidentally break something with an untested change).

@stuartmorgan-g
Copy link
Collaborator

Oh, and if you want to do the honors you could remove the final caveat from the main README in this patch :)

@clarkezone
Copy link
Collaborator Author

OK, all done here!

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

🎉

@stuartmorgan-g stuartmorgan-g merged commit 5c77651 into google:master Jan 1, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants