Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Assertion failed when creating cursor FBO on multi-GPU setup #2479

Closed
J0nnyMak0 opened this issue Nov 16, 2020 · 5 comments · Fixed by #2520
Closed

Assertion failed when creating cursor FBO on multi-GPU setup #2479

J0nnyMak0 opened this issue Nov 16, 2020 · 5 comments · Fixed by #2520
Labels
backend/drm bug

Comments

@J0nnyMak0
Copy link
Contributor

With latest sources, sway fails to start. Log attached.
sway.log
@emersion emersion changed the title Sway fails to start Assertion failed when creating cursor FBO on multi-GPU setup Nov 16, 2020
@emersion emersion transferred this issue from swaywm/sway Nov 16, 2020
@emersion emersion added the bug label Nov 16, 2020
@neon64
Copy link
Contributor

neon64 commented Nov 19, 2020

I don't know nearly enough about wlroots to understand this bug, but in the meantime adding this hack inside of gles2_get_renderer_in_context at least prevents the crash. I'm sure the actual fix is elsewhere though as gles2_get_renderer_in_context is supposed to assume the context is already current, right?

if(!wlr_egl_is_current(renderer->egl)) {
    wlr_egl_make_current(renderer->egl, EGL_NO_SURFACE, NULL);
}

function calls leading up to the crash:
wlr_output_cursor_set_surface -> drm_connector_set_cursor -> drm_fb_lock_surface -> drm_surface_unset_current -> wlr_renderer_bind_buffer -> gles2_bind_buffer -> gles2_get_renderer_in_context

@J0nnyMak0
Copy link
Contributor Author

FWIW I see the assert only when external monitors are connected.

@J0nnyMak0
Copy link
Contributor Author

I can create a pull request if this is a satisfactory solution. Even with this change, I still get freezes from time to time when hotplugging external monitors. Not sure if that is related to this or some other change. (I haven't kept up with more recent changes to wlroots/sawy as I've not been using sway on a daily basis because of this issue.)

@emersion
Copy link
Member

emersion commented Dec 4, 2020

drm_surface_unset_current must only be called after making the surface current. Apparently that's not the case here.

@emersion emersion mentioned this issue Dec 4, 2020
Closed
emersion added a commit to emersion/wlroots that referenced this issue Dec 7, 2020
@emersion
backend/drm: unset current surface before importing
479e2be 
drm_fb_import_wlr may need to change the current EGL context. For
instance by calling drm_fb_clear, which calls wlr_buffer_unlock, which
may destroy a buffer if the cursor swapchain size has changed, which
calls gles2's destroy_buffer, which calls glDeleteFramebuffers.

Closes: swaywm#2479
@emersion emersion mentioned this issue Dec 7, 2020
Merged
@emersion
Copy link
Member

emersion commented Dec 7, 2020

Can you try #2520?

emersion added a commit that referenced this issue Dec 7, 2020
@emersion
backend/drm: unset current surface before importing
e69bbfd 
drm_fb_import_wlr may need to change the current EGL context. For
instance by calling drm_fb_clear, which calls wlr_buffer_unlock, which
may destroy a buffer if the cursor swapchain size has changed, which
calls gles2's destroy_buffer, which calls glDeleteFramebuffers.

Closes: #2479
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
backend/drm bug
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

backend/drm: unset current surface before importing emersion/wlroots
3 participants
@emersion @J0nnyMak0 @neon64