Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[embedder] Add gl present callback that takes present info #20672

Merged
merged 5 commits into from
Aug 26, 2020

Conversation

iskakaushik
Copy link
Contributor

When we present a frame we now pass back the fbo information to the embedder. While this is usually the window fbo, this could be different and now the embedder will have a chance to release the fbo or do any additional book-keeping that it wishes to do.

To this effect, we have added a present_with_info callback, which is a variant of the present callback that has FlutterPresentInfo passed to it.

See EC-2 in flutter.dev/go/double-buffered-window-resize and flutter/flutter#44136

@iskakaushik
Copy link
Contributor Author

I am looking for ideas to effectively test that the right fbo is given back to present. My current thought is to make test_gl_surface have a mode to support render-to-texture then to FBO-0. Then I could assert on the right FBO ids being passed to the callback.

Are there any better ways of doing this than to wire up the test gl surface to do the double buffering? cc: @chinmaygarde , @stuartmorgan

@iskakaushik iskakaushik force-pushed the eb-2 branch 2 times, most recently from 4b1be0e to 7389195 Compare August 20, 2020 23:12
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me, but this layer is not at all my area of expertise; @chinmaygarde should definitely sign off on it (and would be much better equipped than me to advise on testing strategies for the embedder API).

When we present a frame we now pass back the fbo information to the embedder. While this is usually the window fbo, this could be different and now the embedder will have a chance to release the fbo or do any additional book-keeping that it wishes to do.

To this effect, we have added a `present_with_info` callback, which is a
variant of the `present` callback that has `FlutterPresentInfo` passed
to it.
@@ -121,6 +136,8 @@ class EmbedderTestContext {
size_t software_surface_present_count_ = 0;
std::mutex gl_get_fbo_callback_mutex_;
GLGetFBOCallback gl_get_fbo_callback_;
std::mutex gl_present_callback_mutex_;
Copy link
Member

Choose a reason for hiding this comment

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

I suppose you could use a common mutex for these callbacks now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!


context.SetGLPresentCallback([&frame_latch](uint32_t fbo_id) {
// this must be the window FBO id.
ASSERT_EQ(fbo_id, 0u);
Copy link
Member

Choose a reason for hiding this comment

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

IMO, the test would be more resilient to changes in the context used by the fixture if it didn't assume FBO 0 was the onscreen FBO. It happens to the case in the test context. How about an accessor on the test context that returns the FBO ID upfront. You can use that in your assertion here.

This is a minor nit but worth considering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@iskakaushik iskakaushik added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 26, 2020
@fluttergithubbot fluttergithubbot merged commit 1892e03 into flutter:master Aug 26, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 27, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 27, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 27, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 27, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 27, 2020
iskakaushik added a commit to iskakaushik/engine that referenced this pull request Aug 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes platform-android platform-ios waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants