-
Notifications
You must be signed in to change notification settings - Fork 6k
[android] set presentation time via eglPresentationTimeANDROID #33881
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,17 @@ void LogLastEGLError() { | |
FML_LOG(ERROR) << "Unknown EGL Error"; | ||
} | ||
|
||
namespace { | ||
|
||
static bool HasExtension(const char* extensions, const char* name) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have this method in like 3 or 4 places now. Or maybe we will with the swiftshader patch. We should consider moving it somewhere common There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dnfield i think this is out of scope for this PR. Will refactor this later if that is ok. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM |
||
const char* r = strstr(extensions, name); | ||
auto len = strlen(name); | ||
// check that the extension name is terminated by space or null terminator | ||
return r != nullptr && (r[len] == ' ' || r[len] == 0); | ||
} | ||
|
||
} // namespace | ||
|
||
class AndroidEGLSurfaceDamage { | ||
public: | ||
void init(EGLDisplay display, EGLContext context) { | ||
|
@@ -170,13 +181,6 @@ class AndroidEGLSurfaceDamage { | |
|
||
bool partial_redraw_supported_; | ||
|
||
bool HasExtension(const char* extensions, const char* name) { | ||
const char* r = strstr(extensions, name); | ||
auto len = strlen(name); | ||
// check that the extension name is terminated by space or null terminator | ||
return r != nullptr && (r[len] == ' ' || r[len] == 0); | ||
} | ||
|
||
std::list<SkIRect> damage_history_; | ||
}; | ||
|
||
|
@@ -188,6 +192,14 @@ AndroidEGLSurface::AndroidEGLSurface(EGLSurface surface, | |
context_(context), | ||
damage_(std::make_unique<AndroidEGLSurfaceDamage>()) { | ||
damage_->init(display_, context); | ||
|
||
const char* extensions = eglQueryString(display, EGL_EXTENSIONS); | ||
|
||
if (HasExtension(extensions, "EGL_ANDROID_presentation_time")) { | ||
presentation_time_proc_ = | ||
reinterpret_cast<PFNEGLPRESENTATIONTIMEANDROIDPROC>( | ||
eglGetProcAddress("eglPresentationTimeANDROID")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For future us: this was the big part of what was missing last time. When I tested this locally, I hacked in a defintiion of this method and saw improvements. I then refactored things to actually do the proc lookup and put the extension name instead of the method name here, which resulted in no improvements (and slight regressions!). |
||
} | ||
} | ||
|
||
AndroidEGLSurface::~AndroidEGLSurface() { | ||
|
@@ -240,6 +252,16 @@ void AndroidEGLSurface::SetDamageRegion( | |
damage_->SetDamageRegion(display_, surface_, buffer_damage); | ||
} | ||
|
||
bool AndroidEGLSurface::SetPresentationTime( | ||
const fml::TimePoint& presentation_time) { | ||
if (presentation_time_proc_) { | ||
const auto time_ns = presentation_time.ToEpochDelta().ToNanoseconds(); | ||
return presentation_time_proc_(display_, surface_, time_ns); | ||
} else { | ||
return false; | ||
} | ||
} | ||
|
||
bool AndroidEGLSurface::SwapBuffers( | ||
const std::optional<SkIRect>& surface_damage) { | ||
TRACE_EVENT0("flutter", "AndroidContextGL::SwapBuffers"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: use
NiceMock<MockDelegate>
(using ::testing::NiceMock
) so this doesn't spam output about unused calls, here and elsewhere.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! flutter/flutter#105631, i think we want to use it everywhere in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm updating the rest of the file in #33890 which is currently stuck on Fuchsia weirdness.