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

Android - Wrong FPS/update on high rate devices #2129

Closed
AlexandreK38 opened this issue Sep 5, 2024 · 15 comments · Fixed by #2162
Closed

Android - Wrong FPS/update on high rate devices #2129

AlexandreK38 opened this issue Sep 5, 2024 · 15 comments · Fixed by #2162
Labels
bug Something isn't working
Milestone

Comments

@AlexandreK38
Copy link
Contributor

Environment:

  • axmol version: Latest 2.1.5
  • devices test on: Pixel 6, Samsung S8

Issue:

Hi,
I use scheduleUpdate() so that my code is updated on each frame drawn. If I set the frame interval to 1/60s I expect the update() to be called once every 1/60s, but on my Pixel 6 it will be called twice.
So the issue occurs on high rate devices (120FPS) like the Pixel 6.
The code itself tells they are not handled:

@Override
    public void onDrawFrame(final GL10 gl) {
        /*
         * Fix 60fps limiting doesn't work when high-end device is working in 120fps mode.
         */
        if (AxmolRenderer.sAnimationInterval <= 1.0f / 1200.0f * AxmolRenderer.NANOSECONDSPERSECOND) {
            AxmolRenderer.nativeRender();
        } else {
            final long now = System.nanoTime();
            final long interval = now - this.mLastTickInNanoSeconds;

            /*
             * Render time MUST be counted in, or the FPS will slower than appointed.
            */
            this.mLastTickInNanoSeconds = now;
            AxmolRenderer.nativeRender();

            if (interval < AxmolRenderer.sAnimationInterval) {
                try {
                    Thread.sleep((AxmolRenderer.sAnimationInterval - interval) / AxmolRenderer.NANOSECONDSPERMICROSECOND);
                } catch (final Exception e) {
                }
            }
        }
    }

A possible fix to this is this following code:

@Override
    public void onDrawFrame(final GL10 gl) {
        /*
         * Fix 60fps limiting doesn't work when high-end device is working in 120fps mode.
         */
        if (AxmolRenderer.sAnimationInterval <= 1.0f / 1200.0f * AxmolRenderer.NANOSECONDSPERSECOND) {
            AxmolRenderer.nativeRender();
        } else {
            /*
             * Render time MUST be counted in, or the FPS will slower than appointed.
            */
            this.mLastTickInNanoSeconds = System.nanoTime();
            AxmolRenderer.nativeRender();

            final long interval = System.nanoTime() - this.mLastTickInNanoSeconds;

            if (interval < AxmolRenderer.sAnimationInterval) {
                try {
                    Thread.sleep((AxmolRenderer.sAnimationInterval - interval) / AxmolRenderer.NANOSECONDSPERMICROSECOND);
                } catch (final Exception e) {
                }
            }
        }
    }

It works, but in any case (current and possible fix) the UI rendering seems a bit laggy because the renderer is using RENDERMODE_CONTINUOUSLY mode meaning it's the java thread that triggers the call periodically and sleeping in the onDrawFrame() might cause troubles to it.

I explored this solution and a new one using our own thread to trigger manually the rendering (see PR #2125) and the thread solution was giving better performance, overall when you do much work at each frame.

@smilediver
Copy link
Contributor

I did some digging around, and found another way to sync the drawing. I think this would be a better solution than sleeping or triggering rendering from another thread, since it gives more precise control.

So this method would have 2 parts. First part is getting a callback when vsync happens, since this is the ideal time we want to start the frame update and rendering. There's Choreographer.FrameCallback that does this. In this callback we also get the time when vsync has started. Here we could check if we should render a frame now (in case app requests less FPS than actual refresh rate), and then request the manual frame render from GLSurfaceView.

The second part is to tell when we want to display our rendered frame. For this there's eglPresentationTimeANDROID(), that takes the time when the frame should be displayed. We would need to pass N+2 frame time. The only problem might be if it's impossible to get those other parameters that need to be passed in.

The advantage this method gives is that updates happen on vsync, and we control how many frames are in flight, thus minimizing the latency by not stuffing frame queue full.

Related resources:

@thomascastel
Copy link

I don't think it is necessary to go this low level, at least the proposed solution works, as opposed to the live one which doesn't.

@AlexandreK38
Copy link
Contributor Author

@smilediver I had a look at the solution you gave, and it’s a bit more complicated (in the sense you gave on the PR). What seems to be better is that the ui synchronization is right on the thread whereas my PR could occur slightly before or before. This is a good thing but what I’m wondering about is the parameters you talked about (maybe we could find them) and overall the « frame pattern » we should use: if we were to use a multiple of the device refresh rate we wouldn’t have issue, but android device refresh rate could be any from 1/60 to 1/120 or even more, so you end up computing the frames like in the example java file (32232 etc) and I’m not sure it would work well 🤔

@smilediver
Copy link
Contributor

I think it would be less complicated. Maybe you think so because of the Grafika example, but I added that Grafika link just as a reference we can use to get some ideas or how to implement certain things. We certainly don't need majority of stuff in there.

The vsync callback needs only a check if something should be rendered on this vsync or not, which is always true, except if user wants less FPS than the device refresh rate, and then request the rendering if needed. And after the rendering it's just a call to give the time when frame is supposed to be on screen. So it should be pretty easy to understand what's happening.

I was also thinking what should happen if user requests less FPS than device refresh rate. I think patterns like 32232 are not good, since you'll get stutters on rate changes (BTW, in Grafika I think those patterns are just for testing purposes, not for real usage). I think the best way would be to fall to the closest smaller stable possible FPS. So for example if device has 60 FPS, then it can do stable 60/30/20/15... etc., so if user requests 40 FPS we do 30 FPS to keep the frame rate stable. Btw, that Cocos "interval" notion is really unfortunate, I think it should have been something like "max FPS" instead.

It seems the parameters for eglPresentationTimeANDROID() can be got using eglGetCurrentDisplay() and eglGetCurrentSurface(int).

Btw, in your PR, I think, render start time can drift in the whole interval from one vsync to another, since it's not syncing, and sleep() is not precise. It might be causing stutters when it crosses certain points... but maybe it's hidden by buffers, so I'm not sure. I wonder if it would be possible to setup some kind of obvious tests for checking stutter problems...

@AlexandreK38
Copy link
Contributor Author

I think it would be less complicated. Maybe you think so because of the Grafika example, but I added that Grafika link just as a reference we can use to get some ideas or how to implement certain things. We certainly don't need majority of stuff in there.

The vsync callback needs only a check if something should be rendered on this vsync or not, which is always true, except if user wants less FPS than the device refresh rate, and then request the rendering if needed. And after the rendering it's just a call to give the time when frame is supposed to be on screen. So it should be pretty easy to understand what's happening.

It’s sure once we get rid of the useless stuff it should be more understandable

I was also thinking what should happen if user requests less FPS than device refresh rate. I think patterns like 32232 are not good, since you'll get stutters on rate changes (BTW, in Grafika I think those patterns are just for testing purposes, not for real usage). I think the best way would be to fall to the closest smaller stable possible FPS. So for example if device has 60 FPS, then it can do stable 60/30/20/15... etc., so if user requests 40 FPS we do 30 FPS to keep the frame rate stable. Btw, that Cocos "interval" notion is really unfortunate, I think it should have been something like "max FPS" instead.

That’s definitely the point I have doubt about. The fact is that the cocos interval should better be set as an enum (Max, 60,30,20,15) and not as a float so it would be sure your program runs at the desired frequency, but it would require every supported platform to be modified/adapted.
For now the nativeRender call in C++ (and then the update schedule call) is tied to this UI rendering so if we set 30fps as interval we expect the update to be called at the same interval… Or the ui rendering should no more be tied to the update call, who knows.

In the case the user wants 40fps and we do 30fps ofc it would run graphically but what if the user wants to compute something using update() every 30fps not 40? That’s my préoccupation about this frequency.

It seems the parameters for eglPresentationTimeANDROID() can be got using eglGetCurrentDisplay() and eglGetCurrentSurface(int).

Great

Btw, in your PR, I think, render start time can drift in the whole interval from one vsync to another, since it's not syncing, and sleep() is not precise. It might be causing stutters when it crosses certain points... but maybe it's hidden by buffers, so I'm not sure. I wonder if it would be possible to setup some kind of obvious tests for checking stutter problems...

yeah sleep() is more or less precise depending on the device but so far it seems better than the actual code. Maybe a test with trace could reveal the stutters but it would be device dependent. Anyway having a device with a strange rate like 90fps or 100fps would help see some issues.

@AlexandreK38
Copy link
Contributor Author

AlexandreK38 commented Sep 10, 2024

@smilediver I tried to use the FrameCallback and eglPresentationTimeANDROID() but somehow the params were not good (display error) and setting the time wasn't working; unfortunately I couldn't get deeper to see why.

In the meantime I saw that there was other ways to "set" a frame rate (not always working as wanted) and lately since Android 11 there is even a setFrameRate() method on the Surface or SurfaceControl.
This blog explained it all, maybe you saw it:
https://android-developers.googleblog.com/2020/04/high-refresh-rate-rendering-on-android.html

As side note, I tried to use setFrameRate(60, DEFAULT) on my Pixel 6 and it effectively run at 60FPS; I tried with 30 too but it runs at 60FPS anyway...

They also explain that using Choregrapher itself is a simple but not full solution, and since then a thing called AGDK is made to handle the rate, available since Android Api level 19 / NDK 18.

https://developer.android.com/games/sdk/frame-pacing

@smilediver
Copy link
Contributor

smilediver commented Sep 10, 2024

Nice find on that blog post, some good insights there.

setFrameRate() changes display refresh rate, maybe that display can't run in 30Hz mode. Also, it seems that it can coordinate with other apps, so something else could be limiting it. But we can't use this API, too many devices don't support it.

I was aware of frame pacing module in AGDK library, but it seems like an overkill to integrate it just for running the game loop.

Anyway, I'm also planning to take a look at this problem later this week, maybe I can get something working.

Edit: this is probably the reason why calls to eglPresentationTimeANDROID() have failed: https://stackoverflow.com/a/32598507/254101.

@AlexandreK38
Copy link
Contributor Author

setFrameRate() changes display refresh rate, maybe that display can't run in 30Hz mode. Also, it seems that it can coordinate with other apps, so something else could be limiting it. But we can't use this API, too many devices don't support it.

Yeah I agree it is too recent, and not working as we want (like for 30 FPS) so not a solution

I was aware of frame pacing module in AGDK library, but it seems like an overkill to integrate it just for running the game loop.

That's what I thought too, but I also saw Axmol support starts from Android level api 17 not 19, if I'm correct

Anyway, I'm also planning to take a look at this problem later this week, maybe I can get something working.

Great

Edit: this is probably the reason why calls to eglPresentationTimeANDROID() have failed: https://stackoverflow.com/a/32598507/254101.

Good point, but the thing is we have EGL10 in Axmol, mixing to have both seems a bit tricky and I'm not sure we can change EGL10 to EGL14 that easily

@solan-solan
Copy link
Contributor

I use scheduleUpdate() so that my code is updated on each frame drawn. If I set the frame interval to 1/60s I expect the update() to be called once every 1/60s, but on my Pixel 6 it will be called twice.

update method relies on delta time between two frames. If this time is true, do not think that it is good idea to link this method to draw fps.

@smilediver
Copy link
Contributor

I've implemented this loop using Choreographer.FrameCallback in #2142 PR. Can you check this on 120Hz device?

@rh101
Copy link
Contributor

rh101 commented Sep 18, 2024

@AlexandreK38 There was an issue introduced in #1504, which resulted in incorrect update calls. The attempt to fix this is in #2162. Would this be the source of the issue you're experiencing too?

@AlexandreK38
Copy link
Contributor Author

AlexandreK38 commented Sep 18, 2024

@rh101 Yeah this is the same issue. The change made in the #2162 is basically the "simple" solution I put above here :)
This works, but it works better using my thread as the simple solution block the UI thread.
There is also another possible solution in #2142 by @smilediver, currently in draft too and we're both looking into it.

@rh101
Copy link
Contributor

rh101 commented Sep 18, 2024

@rh101 Yeah this is the same issue. The change made in the #2162 is basically the "simple" solution I put above here :)

OK! That's good, since now we know this will work.

For the time being, #2162 will most likely be used as the interim solution until a decision is made on implementing one of the more drastic changes to the Android rendering code, such as in your PR and the one from @smilediver.

@smilediver
Copy link
Contributor

Agreed, #2162 should be merged, until we decide on another solution.

@halx99 halx99 linked a pull request Sep 18, 2024 that will close this issue
6 tasks
@halx99 halx99 added bug Something isn't working labels Sep 19, 2024
@AlexandreK38
Copy link
Contributor Author

Simple solution was merged into dev branch, closing the issue

@halx99 halx99 added this to the 2.2.0 milestone Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants