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

[RFC] Add support for pushing already built display lists. #934

Merged
merged 1 commit into from Mar 6, 2017

Conversation

@jrmuizel
Copy link
Contributor

jrmuizel commented Feb 27, 2017

Gecko has the concept of 'empty transactions' where we need to get a new
frame composited but aren't rebuilding the Gecko display lists. To
support this we'd like to keep around a retained representation of the
partial WebRender display list.

One easy way to implement this is to just use a BuiltDisplayList
as the retained representation and support appending them
to display lists. Unfortunately, the implementation is a bit fragile
because of needing to deal with ItemRanges.


This change is Reviewable

@glennw
Copy link
Member

glennw commented Feb 27, 2017

@jrmuizel If you call api.generate_frame(None) it will redraw the current display list. Does this achieve what you want or is there a difference between that and what gecko requires above?

@jrmuizel
Copy link
Contributor Author

jrmuizel commented Feb 27, 2017

My understanding is that the display list that's sent to webrender is not identical, but changed in a way that doesn't require us to rebuild the display list.

It may be that this empty transaction functionality is not needed in the long term, but I'll need to investigate that further.

@jrmuizel
Copy link
Contributor Author

jrmuizel commented Mar 1, 2017

I have some more details on the scenarios in which this happens. Imagine you have a page that's using js to continuously set the transform or opacity on some piece of content. In Gecko we'll avoid building the Gecko display list and just send an updated layer tree to the compositor. In this case we need some kind of retained version of the display list that we keep around on the gecko side so that we can send it over as needed.

I think we'll be able to come a better solution to this in the long term but for now this will allow Gecko to avoid having to have it's own duplicate way of retaining parts of a display list.

@glennw
Copy link
Member

glennw commented Mar 1, 2017

For that use case, the preferred API is here - #832.

The way it works, roughly, is:

  • During DL construction, you set properties you want to animate to be a "binding" (opacity and transform are supported now, but others are easy to add).
  • Then, when you call api.generate_frame() you can pass an optional list of property values (these are the current value of the animated properties).

In this way, there is no need to maintain a client-side copy of the display list. You can just re-draw the same display list by calling generate_frame(), but specifying different values for the animated properties.

Does this sound like it would cover all the use cases you know of?

@glennw
Copy link
Member

glennw commented Mar 1, 2017

I guess the problem is that you don't know which properties JS might change in this case, so you end up setting every stacking context as a binding? Which isn't necessarily a problem - I don't think that will have any particularly bad side effects.

@jrmuizel
Copy link
Contributor Author

jrmuizel commented Mar 1, 2017

Yes, #832 is the longer term api that I had in mind. And yes, we'd need to set every stacking context as a binding.

How would you feel about adding api like this temporarily until we can transition to something saner?

@glennw
Copy link
Member

glennw commented Mar 1, 2017

I guess that would be OK - so long as we clearly mark that it's a temporary API and will be removed.

Gecko has the concept of 'empty transactions' where we need to get a new
frame composited but aren't rebuilding the Gecko display lists. To
support this we'd like to keep around a retained representation of the
partial WebRender display list.

One easy way to implement this is to just use a BuiltDisplayList
as the retained representation and support appending them
to display lists. Unfortunately, the implementation is a bit fragile
because of needing to deal with ItemRanges.

Hopefully, we can get rid of this eventually and avoid rebuilding the
display list at all in those scenarios.
@jrmuizel jrmuizel force-pushed the jrmuizel:push_dl branch from face0e6 to 6ce5378 Mar 2, 2017
@jrmuizel
Copy link
Contributor Author

jrmuizel commented Mar 4, 2017

I have this being used in https://bugzilla.mozilla.org/show_bug.cgi?id=1344396 so it would be good to land in webrender.

r? @glennw

@glennw
Copy link
Member

glennw commented Mar 6, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Mar 6, 2017

📌 Commit 6ce5378 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Mar 6, 2017

Testing commit 6ce5378 with merge 178a65d...

bors-servo added a commit that referenced this pull request Mar 6, 2017
[RFC] Add support for pushing already built display lists.

Gecko has the concept of 'empty transactions' where we need to get a new
frame composited but aren't rebuilding the Gecko display lists. To
support this we'd like to keep around a retained representation of the
partial WebRender display list.

One easy way to implement this is to just use a BuiltDisplayList
as the retained representation and support appending them
to display lists. Unfortunately, the implementation is a bit fragile
because of needing to deal with ItemRanges.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/934)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 6, 2017

☀️ Test successful - status-travis
Approved by: glennw
Pushing 178a65d to master...

@bors-servo bors-servo merged commit 6ce5378 into servo:master Mar 6, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
moz-v2v-gh pushed a commit to mozilla/gecko-projects that referenced this pull request Mar 7, 2017
This is a largely uninteresting patch that just uses the DisplayListBuilder
directly. A wonderful cleanup patch will come after this. One of the more
interesting pieces is the use of PushBuiltDisplayList. This is needed for
handling empty transactions. See servo/webrender#934
for more info.
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request Mar 13, 2017
This is a largely uninteresting patch that just uses the DisplayListBuilder
directly. A wonderful cleanup patch will come after this. One of the more
interesting pieces is the use of PushBuiltDisplayList. This is needed for
handling empty transactions. See servo/webrender#934
for more info.
Manishearth pushed a commit to Manishearth/gecko-dev that referenced this pull request Mar 15, 2017
This is a largely uninteresting patch that just uses the DisplayListBuilder
directly. A wonderful cleanup patch will come after this. One of the more
interesting pieces is the use of PushBuiltDisplayList. This is needed for
handling empty transactions. See servo/webrender#934
for more info.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
This is a largely uninteresting patch that just uses the DisplayListBuilder
directly. A wonderful cleanup patch will come after this. One of the more
interesting pieces is the use of PushBuiltDisplayList. This is needed for
handling empty transactions. See servo/webrender#934
for more info.

UltraBlame original commit: 0d411df025f5315626bd6a8bfec32d264e24cd84
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
This is a largely uninteresting patch that just uses the DisplayListBuilder
directly. A wonderful cleanup patch will come after this. One of the more
interesting pieces is the use of PushBuiltDisplayList. This is needed for
handling empty transactions. See servo/webrender#934
for more info.

UltraBlame original commit: 0d411df025f5315626bd6a8bfec32d264e24cd84
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
This is a largely uninteresting patch that just uses the DisplayListBuilder
directly. A wonderful cleanup patch will come after this. One of the more
interesting pieces is the use of PushBuiltDisplayList. This is needed for
handling empty transactions. See servo/webrender#934
for more info.

UltraBlame original commit: 0d411df025f5315626bd6a8bfec32d264e24cd84
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.