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

Swappy and GameActivity #1172

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HakimHauston
Copy link

Integration to 2 of the projects:
-. Deferred Multi Sampling (base for the initial work)
-. GLTFSceneRendering (patched the work with diff and some tuning)

-. Integrated Swappy and GameActivity
-. for GLTFSceneRendering project
@HakimHauston HakimHauston marked this pull request as ready for review December 16, 2024 06:37
@HakimHauston
Copy link
Author

HakimHauston commented Dec 16, 2024

Hi @SaschaWillems ,

Thanks for your amazing work on this Vulkan Sample repository!

I noticed that there are some possible performance gain for the Android platform, the largest of which can be gain through integrating Swappy (Android's Frame Pacing API) and GameActivity.

My comparison test on deferredmultisampling project shows average FPS increase from ~8fps to ~12fps after the change.

I've based my work on your deferredmultisampling sample and plan to patch the rest of the projects. I've patched gltfscenerendering as a separate commit cc27517 but figured some minor manual modification may be required for each of the projects.

Before I do the manual patch for each of the projects, would you please:

  1. let me know if there is an easier way to patch the same changes to all examples ? I saw your _template directory but looks like it is for creating a new sample rather than patching existing ones
  2. review my modification for deferredmultisampling at commit 6e58227 to ensure that it is acceptable? I'll base the patch for the rest of the examples on this commit.

@SaschaWillems
Copy link
Owner

SaschaWillems commented Dec 20, 2024

I'll probably need some more time for a proper review, but in general changes like this need to work an all platforms, so changes like in https://github.com/SaschaWillems/Vulkan/pull/1172/files#diff-5b12913c531aefa5a62f2e7cf59ce9479ee5491c67cca7c898baa8d45f38b960L45 need to be properly guarded so they only apply to Adnroid without touch other platforms.

And if you need to have changes applied to all or many samples, such changes need to be implemented in the example base class..

I also need to read up on swappy first. It looks like integrating that would require a lot of changes and I'm not sure about that. There are things I'm skeptical, kike having to up the min SDK version (which would reduce the target audience). And who would maintain that part of the code in the futurte?

Can you write a short proposal so I can consider if those changes my sense?

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.

2 participants