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

Frame tree contains iframes that are hidden / not visible. #13149

Open
glennw opened this issue Sep 1, 2016 · 4 comments
Open

Frame tree contains iframes that are hidden / not visible. #13149

glennw opened this issue Sep 1, 2016 · 4 comments
Labels
A-constellation Involves the constellation A-webrender

Comments

@glennw
Copy link
Member

glennw commented Sep 1, 2016

This is not necessarily a bug - I'm opening this to discuss a solution to the problem below:

The symptom:
Several iframe reftests are intermittently failing. When testing against webrender, these always fail.

The problem:
When running reftests, the compositor asks WR "what is the last epoch you rendered on screen for this pipeline id"? If that matches the epoch of the pipeline in the corresponding script/layout thread, we know that the image is stable and ready to be written (there's other conditions too, not relevant here).

WR will only report a valid epoch number for a pipeline if it was drawn during the last render. For instance, if an iframe is painted on frame 1, and then hidden by the script thread, on frame 2 WR will not report a valid epoch for the iframe (it returns None).

However, the iframe in this situation still exists in the frame tree, and therefore the compositor doesn't think it can output the image - the returned epoch for the (invisible) iframe is None.

A possible solution:
I can easily change WR behaviour to maintain the "last valid epoch" for each pipeline, and return that value. In this case it would fix the problem - the compositor would see that the last painted epoch for the iframe matches the epoch in the script thread, and it will save the image.

An alternative is to change the compositor / constellation so that the frame tree doesn't include iframes in the tree that are not visible (although maybe this is very complex, since you don't know until layout).

What is the expected behaviour in this situation? (a) Should the frame tree contain non-visible iframes? (b) Should WR return the last painted epoch for an iframe, even if it's not currently part of the current render?

@glennw
Copy link
Member Author

glennw commented Sep 1, 2016

@glennw
Copy link
Member Author

glennw commented Sep 1, 2016

Hmmm, this is tricky. I can imagine a situation where an iframe is created, and hidden immediately by script. In this case, even if I do maintain a "last painted epoch" it's possible that the iframe will never actually get drawn, but it will exist in the frame tree.

I wonder if there is a way to safely detect that the output of an iframe would have been irrelevant to the final image (without mis-detecting this while the iframe is loading).

@metajack
Copy link
Contributor

metajack commented Sep 1, 2016

It seems like the only reason to want invisible frames in the compositor frame tree is so minimize mutation when visibility is being toggled often. Or is there some other use case I'm missing?

It seems slightly weird to me that the compositor would need any knowledge of invisible content. So I think I'd prefer your alternative solution, but without understanding the issues better it's hard to have a strong opinion.

@pcwalton
Copy link
Contributor

pcwalton commented Sep 2, 2016

Could layout just notify the compositor of iframe visibility state?

@cbrewster cbrewster added A-constellation Involves the constellation A-webrender labels Sep 4, 2016
glennw pushed a commit to glennw/webrender that referenced this issue Sep 6, 2016
Instead of storing the last rendered epoch for each pipeline from
the previous frame, accumulate this information for each frame.
This is a workaround for the way servo's compositor keeps iframes
in the frame tree when they are invisible.

This is a workaround for servo/servo#13149.

Long term, we should find a better solution for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-constellation Involves the constellation A-webrender
Projects
None yet
Development

No branches or pull requests

4 participants