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

Event loop implementation for Android. #132

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

hysw
Copy link
Collaborator

@hysw hysw commented Apr 25, 2023

I got a bit annoyed at #if defined statements, and trying to figure out what is the exact control flow, so I pulled out the part that are glfw or android specific

The new stuff

  • Actually have a proper Android event loop
  • Use Android screen size, instead of hardcoded 720p horizontal resolution
  • ImGui input

@hysw hysw requested review from Keenuts and chaoticbob April 25, 2023 15:57
@hysw hysw force-pushed the tianc/android-event-loop branch 2 times, most recently from b537af7 to f2eeba8 Compare April 25, 2023 18:18
Copy link
Contributor

@chaoticbob chaoticbob left a comment

Choose a reason for hiding this comment

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

Some minor comments.

include/ppx/window.h Outdated Show resolved Hide resolved
include/ppx/window.h Outdated Show resolved Hide resolved
include/ppx/application.h Outdated Show resolved Hide resolved
src/android/main.cpp Show resolved Hide resolved
include/ppx/window.h Outdated Show resolved Hide resolved
@hysw hysw force-pushed the tianc/android-event-loop branch 2 times, most recently from cdf668b to 4976d67 Compare April 25, 2023 21:08
@hysw
Copy link
Collaborator Author

hysw commented Apr 25, 2023

Note, commit d17a434 and dfd2ccd belong to #136

Copy link
Contributor

@chaoticbob chaoticbob left a comment

Choose a reason for hiding this comment

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

Just some nits. Will wait on @Keenuts feedback as well.

include/ppx/window.h Outdated Show resolved Hide resolved
include/ppx/window.h Outdated Show resolved Hide resolved
include/ppx/window.h Outdated Show resolved Hide resolved
include/ppx/window.h Outdated Show resolved Hide resolved
include/ppx/window.h Outdated Show resolved Hide resolved
src/ppx/window.cpp Outdated Show resolved Hide resolved
src/ppx/window.cpp Outdated Show resolved Hide resolved
src/ppx/window_android.cpp Outdated Show resolved Hide resolved
src/ppx/window_android.cpp Outdated Show resolved Hide resolved
include/ppx/application.h Outdated Show resolved Hide resolved
@hysw hysw force-pushed the tianc/android-event-loop branch 2 times, most recently from e6f2691 to f339316 Compare April 26, 2023 20:31
@hysw hysw marked this pull request as ready for review April 26, 2023 20:36
Copy link
Member

@Keenuts Keenuts left a comment

Choose a reason for hiding this comment

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

Sorry for the review delay!
Just some nits.
One thing I don't know yet is how Android handles application lifecycles with native activities (regarding the global/local app variable). But I'd guess we'll find out sooner or later.

src/android/main.cpp Outdated Show resolved Hide resolved
include/ppx/application.h Outdated Show resolved Hide resolved
src/ppx/window_android.cpp Show resolved Hide resolved
mAndroidApp->userData = this;
}

Result WindowImplAndroid::Create(const char* title)
Copy link
Member

Choose a reason for hiding this comment

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

nit pTitle?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a C style string, pTitle sounds like const char**

- Add event process to the application main loop.
- Read screen resolution from Android window.
- Pass touch input to ImGui.
Copy link
Contributor

@chaoticbob chaoticbob left a comment

Choose a reason for hiding this comment

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

LGTM

@hysw hysw merged commit 487743a into google:main Apr 28, 2023
@at-ninja
Copy link
Contributor

at-ninja commented May 1, 2023

This PR seems to have messed with the rendered image on the two XR projects.

@hysw
Copy link
Collaborator Author

hysw commented May 1, 2023

closes #126

@hysw hysw deleted the tianc/android-event-loop branch May 4, 2023 19:53
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.

4 participants