Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion shell/platform/android/android_environment_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,24 @@ AndroidEnvironmentGL::AndroidEnvironmentGL()
return;
}

// See if we can use the EGL_ANDROID_presentation_time extension. This
// extension may be available from API18 onward.
// If this extension is available, it will be invoked with the target vsync
// for the current frame before calling eglSwapBuffers. This helps when the
// engine over-stuffs the buffer queue. This can happen when a frame goes over
// budget into the next vsync period, and a subsequent frame is quickly
// submitted in the same vsync period. Dequeuing buffers will then be slow
// until the engine stops submitting frames and lets the GPU catch up.
// However, using this extension means the SurfaceFlinger will only use the
// newest buffer for the current vsync, and the GPU and CPU will stay more
// in sync.
// See https://developer.android.com/games/sdk/frame-pacing for more
// details and diagrams.
auto* extensions = eglQueryString(display_, EGL_EXTENSIONS);
if (strstr(extensions, "EGL_ANDROID_presentation_time")) {
presentation_time_proc_ =
reinterpret_cast<PFNEGLPRESENTATIONTIMEANDROIDPROC>(
eglGetProcAddress("sEGL_ANDROID_presentation_time"));
Copy link

Choose a reason for hiding this comment

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

should this have been EGL_ANDROID_presentation_time ? What is the relationship between this value and L24? @dnfield

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what happened here is that I meant to copy and paste eglPresentationTimeANDROID, and instead I copy and pasted EGL_ANDROID_presentation_time .. and hit the s key by accident .. and then didn't notice it.

When I initially did some tests with this extension, I was just directly calling it and hacking it in as if it wouldn't have to be looked up. I then refactored to do it the right way (get the proc address this way), but never re-tested things to make sure it was still doing the good thing.. and then this got missed in review :(

Copy link
Contributor

Choose a reason for hiding this comment

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

@iskakaushik FYI since I think you reviewed the original PR

Copy link
Contributor

Choose a reason for hiding this comment

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

The bottom line is: this lookup is correct and we should accept it.

Copy link
Contributor Author

@MxSoul MxSoul Dec 9, 2021

Choose a reason for hiding this comment

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

Maybe can see this L92 and L193
https://cs.android.com/android/platform/superproject/+/android-10.0.0_r1:frameworks/native/opengl/libs/EGL/egl_platform_entries.cpp;l=92

In my opinion:
EGL_ANDROID_presentation_time is gBuiltinExtensionString, and It is used to determine whether an extension point exists.
eglPresentationTimeANDROID is key when get address.

Copy link

@blasten blasten Dec 9, 2021

Choose a reason for hiding this comment

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

could we add a comment about what EGL_ANDROID_presentation_time and eglPresentationTimeANDROID are used for?

It will definitely help next time there's a refactor, and the blame doesn't point to this PR. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I am not native speaker. I'm not sure my description is accurate. Maybe @dnfield could help to add this comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

@blasten I added some comments.

eglGetProcAddress("eglPresentationTimeANDROID"));
}

valid_ = true;
Expand Down