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

embedding rendering broken after webrender merge #10274

Closed
zmike opened this issue Mar 29, 2016 · 10 comments
Closed

embedding rendering broken after webrender merge #10274

zmike opened this issue Mar 29, 2016 · 10 comments

Comments

@zmike
Copy link
Contributor

@zmike zmike commented Mar 29, 2016

tracking issue for however many subissues I find on this journey

cc @pcwalton @glennw @larsbergstrom @metajack

currently getting some kind of gl errors during initialize_compositing() (which is not implemented for webrender??), in the middle of a debug build to get more info.

webrender cannot be enabled for embedding at all it looks like

@zmike
Copy link
Contributor Author

@zmike zmike commented Oct 28, 2016

Okay, after a long long time of being absent I've determined that this is most likely due to the removal of any functionality from initialize_compositing() in components/compositor.rs. This causes servo to abort very early when attempting to compile shaders, complaining that GL is not found.

As of the last functional version that I used, the code for this function was:

    self.context = Some(rendergl::RenderContext::new(self.native_display.clone(),
                                                     show_debug_borders,
                                                     opts::get().output_file.is_some()))

I spent some time trying to track down what is now happening for this, and I ended up deep inside rust-offscreen-rendering-context, which appears to use a hardcoded XOpenDisplay call, followed by more hardcoded calls to create the gl context.

There seems to be no ability to pass these params from the embedding crate like in the pre-webrender code.

@emilio
Copy link
Member

@emilio emilio commented Oct 29, 2016

@zmike: So XOpenDisplay in there should only be called to initialize WebGL contexts, so I'm really curious on how you end up there. Do you have a backtrace?

@glennw
Copy link
Member

@glennw glennw commented Nov 2, 2016

@zmike The code you referenced above creates a GL context for use by rust-layers, which is now deprecated / unused. Webrender expects a current GL context to be bound during initializing and also rendering. In the glutin port, we rely on the code in ports/glutin/window.rs to create and bind the GL context. So ideally the embedder creates a GL context and binds it before calling in to the servo methods. Will something like that work with the CEF / embedding framework?

@zmike
Copy link
Contributor Author

@zmike zmike commented Dec 8, 2016

Sorry for my late response, I didn't get email notifications about this for some reason.

@glennw CEF has always worked the way you describe; it was written specifically to work this way. The issue is that the application relies on the initialize_compositing method to create+bind the GL context, and it expects that Servo's internal GL setup occurs once this method is called--that's how it worked prior to the WR merge. If GL setup does not occur synchronously when this is called, the application cannot know when it is happening and may not have the right GL context bound.

@glennw
Copy link
Member

@glennw glennw commented Dec 21, 2016

@zmike I guess the errors you are seeing have a backtrace that contains Browser::new() in servo/lib.rs?

WR does (currently) expect a GL context to be bound when Browser::new() is called. What does the CEF initialization flow look like in terms of where Browser::new() is called and the call to initialize_compositing()?

If there's a requirement to split the WR initialization between Browser::new() and initialize_compositing(), that sounds reasonably straightforward.

The only other entry points where WR expects a GL context to be bound are wr.update() and wr.render(), which are called internally by the composite() method, so I imagine those are fine as is.

@zmike
Copy link
Contributor Author

@zmike zmike commented Dec 23, 2016

@glennw There's mainly just GL errors everywhere now :)

The initialize_compositing() method is a servo-specific extension to CEF which was added for the sole purpose of being the GL initialization entry point.

Browser::new() is called when a new pipeline is created and the page data is starting to fetch, but the application may not be ready for any GL operations at this point, so initialize_compositing() was added. I think it makes sense to once again split WR init off into this method to provide this separation for applications.

@glennw
Copy link
Member

@glennw glennw commented Jan 13, 2017

@zmike Yup, we can do that. Would you mind opening an issue in the servo/webrender repo with the details? It's a bit easier to track WR work there. Is there an easy(ish) way for me to test that this is working when I make the change?

@zmike
Copy link
Contributor Author

@zmike zmike commented Feb 27, 2017

Sorry for the delay on this, I don't understand why I'm not getting mails anymore from github :/

servo/webrender#933

@nox
Copy link
Member

@nox nox commented Oct 1, 2017

This is still true.

@jdm
Copy link
Member

@jdm jdm commented Jun 19, 2018

CEF is not in the tree any more.

@jdm jdm closed this Jun 19, 2018
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.

None yet
6 participants
You can’t perform that action at this time.