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

Change API to only publish a frame explicitly (via generate_frame). #702

Closed
glennw opened this issue Jan 10, 2017 · 3 comments
Closed

Change API to only publish a frame explicitly (via generate_frame). #702

glennw opened this issue Jan 10, 2017 · 3 comments

Comments

@glennw
Copy link
Member

@glennw glennw commented Jan 10, 2017

No description provided.

@randomPoison
Copy link
Contributor

@randomPoison randomPoison commented Jan 14, 2017

Is there more information available for this? I'd like to work on it, but it might need to be fleshed out more (I'll try to investigate it some myself but without any experience with webrender there's no telling if I'll get anywhere).

@jrmuizel
Copy link
Contributor

@jrmuizel jrmuizel commented Jan 14, 2017

Basically this just involves moving the code for build_scene(), render(), etc. out of SetRootDisplayList and SetRootPipeline into GenerateFrame() and then fixing up the callers of SetRoot* to generate the GenerateFrame messages in addition to their current messages.

@randomPoison
Copy link
Contributor

@randomPoison randomPoison commented Jan 15, 2017

Thanks for the info! I'll give it a shot.

bors-servo added a commit that referenced this issue Jan 19, 2017
…glennw

Publish a frame explicitly with RenderApi::generate_frame()

Remove the frame publishing code from the message handling for `ApiMsg::SetRootDisplayList` and `ApiMsg::SetRootPipeline`. Instead, `ApiMsg::GenerateFrame` now builds the scene and `RenderApi::set_root_display_list()` and `set_root_pipeline()` now send a `GenerateFrame` message.

This change seems pretty straightforward, but I have a few things I'm not sure about:

- I moved the call to `build_scene()` from `ApiMsg::SetRootDisplayList` to `ApiMsg::GenerateFrame`, but `ApiMsg::SetRootPipeline` wasn't calling `build_scene()` so I'm not sure that was the right thing to do.
- I added a call to `generate_frame()` to `RenderApi::set_root_display_list()` and `set_root_pipeline()`, keeping the public API the same. I'm not sure if this was the desired change, I would understand if the goal was to have code using WebRender explicitly call `generate_frame()` after calling `set_root_display_list()` and/or `set_root_pipeline()`, but I'll wait for feedback before making that change.

I tested these changes with with the wr-sample project and everything seemed kosher, but let me know if there other regression tests I should run.

As an aside, is there any reading about WebRender's design/architecture that I could look at? I don't fully understand the rational behind this change or the context in which it was requested, but I figure it'd make more sense if I knew more about how WebRender works.

closes #702

<!-- 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/722)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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