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

Use X11 display to create EGL display #21796

Closed
wants to merge 1 commit into from
Closed

Conversation

wmww
Copy link
Contributor

@wmww wmww commented Oct 13, 2020

Description

Use the X11 display to create the EGL display. This brings the implementation in line with the Wayland renderer, and maybe fixes flutter/flutter#60429 (I can't reproduce that, so I do not know for sure). I don't think this raises any thread safety problems, as the display is used with different contexts on each thread.

Related Issues

flutter/flutter#60429

Tests

None

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change. If not, delete the remainder of this section.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@robert-ancell
Copy link
Contributor

This used to cause Flutter to crash do to multiple threads accessing xlib. I don't know if there have been changes in Flutter that ensure that can't happen anymore.

@wmww
Copy link
Contributor Author

wmww commented Oct 13, 2020

I thought we'd be fine because we make our own context and you can use a single EGL display from multiple threads as long as you use multiple contexts. What does the EGL display have to do with xlib? Was it the call to gdk_x11_get_default_xdisplay() itself that was being made on the wrong thread?

@robert-ancell
Copy link
Contributor

I thought we'd be fine because we make our own context and you can use a single EGL display from multiple threads as long as you use multiple contexts. What does the EGL display have to do with xlib? Was it the call to gdk_x11_get_default_xdisplay() itself that was being made on the wrong thread?

EGL is using the Display object from XLib (DRI3 calls?) and GTK is also using the same object for its operations. That's where the collision was occurring.

@robert-ancell
Copy link
Contributor

Based on the EGL X11 extension, perhaps we should be using:

eglGetPlatformDisplayEXT(EGL_PLATFORM_X11_EXT, NULL, NULL);

@wmww
Copy link
Contributor Author

wmww commented Oct 14, 2020

That looks like it would work, but I can't find what I need to include to get eglGetPlatformDisplayEXT() and EGL_PLATFORM_X11_EXT.

@robert-ancell
Copy link
Contributor

robert-ancell commented Oct 14, 2020

Confirmed the currently proposed code crashes. I ran the demo app created by flutter create, then wildly resized the window to get:

[xcb] Unknown sequence number while processing queue
[xcb] Most likely this is a multi-threaded client and XInitThreads has not been called
[xcb] Aborting, sorry about that.
test2: ../../src/xcb_io.c:260: poll_for_event: Assertion `!xcb_xlib_threads_sequence_lost' failed.
Lost connection to device.

@robert-ancell
Copy link
Contributor

Found a solution in #21831

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[linux]: Crash in gdk_window_has_impl on startup
3 participants