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

backend/headless: use FBOs instead of pbuffers #2063

Merged
merged 3 commits into from
Apr 22, 2020

Conversation

emersion
Copy link
Member

@emersion emersion commented Mar 10, 2020

Allows the headless backend to use another backend's renderer.

@ammen99
Copy link
Member

ammen99 commented Mar 11, 2020

Shouldn't wlroots also offer a way to get the FB to render to? For example, Wayfire also switches between various FBs when rendering. Until now, I have always tried to reset to FB 0 to render the the output, which seems to not be the case anymore.

@ammen99
Copy link
Member

ammen99 commented Mar 11, 2020

Oh and by the way, doesn't this make the headless backend dependent on a GLES renderer? From my understanding, this wasn't the case before, but I am not very familiar with EGL.

@emersion
Copy link
Member Author

Shouldn't wlroots also offer a way to get the FB to render to? For example, Wayfire also switches between various FBs when rendering. Until now, I have always tried to reset to FB 0 to render the the output, which seems to not be the case anymore.

If you switch between FBs, you should save the previous value with glGet(GL_FRAMEBUFFER_BINDING) and restore it afterwards.

Oh and by the way, doesn't this make the headless backend dependent on a GLES renderer? From my understanding, this wasn't the case before, but I am not very familiar with EGL.

Yes, that's true. Could be fixed by using eglGetProcAddress for all GL functions.

@ammen99
Copy link
Member

ammen99 commented Mar 11, 2020

Yes, that's true. Could be fixed by using eglGetProcAddress for all GL functions.

My understanding was that we want to make it possible to write a pixman-based renderer or something :P (note I do not care about this case, just pointing it out since somebody does)

@emersion
Copy link
Member Author

My understanding was that we want to make it possible to write a pixman-based renderer or something :P (note I do not care about this case, just pointing it out since somebody does)

Yeah, I care about this, but I don't think it's related. Currently the headless backend depends on EGL, so this PR doesn't change a lot. With renderer v6 the compositor will manage swapchains directly (or indirectly via helpers) so the backend won't even care.

(My eglGetProcAddress comment was about desktop OpenGL.)

@any1
Copy link
Contributor

any1 commented Mar 11, 2020

CPU load in sway due to glReadPixels() is doubled when running weston-simple-egl in wayvnc after this change is applied. Maybe a different pixel format would give better performance?

@emersion
Copy link
Member Author

Can you try GL_RGBA4 and GL_RGBA8_OES instead of GL_RGB8_OES?

@any1
Copy link
Contributor

any1 commented Mar 13, 2020

Yes, using RGBA8_OES performs much better. GL_RGBA4 actually performs much worse.

emersion added a commit to emersion/wlroots that referenced this pull request Apr 6, 2020
Most of the pending output state is not forwarded to the backend prior
to an output commit. For instance, wlr_output_set_mode just stashes the
mode without calling any wlr_output_impl function.
wlr_output_impl.commit is responsible for applying the pending mode.

However, there are exceptions to this rule. The first one is
wlr_output_attach_render. It won't go away before renderer v6 is
complete, because it needs to set the current EGL surface.

The second one is wlr_output_attach_buffer.
wlr_output_impl.attach_buffer is removed in [1].

When wlr_output_rollback is called, all pending state is supposed to be
cleared. This works for all the state except the two exceptions
mentionned above. To fix this, introduce wlr_output_impl.rollback.

Right now, the backend resets the current EGL surface. This prevents GL
commands from affecting the output after wlr_output_rollback.

This patch is required for FBO-based outputs to work properly. The
compositor might be using FBOs for its own purposes [2], having leftover
FBO state can have bad consequences.

[1]: swaywm#2097
[2]: swaywm#2063 (comment)
@emersion
Copy link
Member Author

emersion commented Apr 6, 2020

This is now ready for review.

(Note that this PR depends on another PR, so I'll keep it as a draft to prevent it from being merged.)

emersion added a commit to emersion/wlroots that referenced this pull request Apr 8, 2020
Most of the pending output state is not forwarded to the backend prior
to an output commit. For instance, wlr_output_set_mode just stashes the
mode without calling any wlr_output_impl function.
wlr_output_impl.commit is responsible for applying the pending mode.

However, there are exceptions to this rule. The first one is
wlr_output_attach_render. It won't go away before renderer v6 is
complete, because it needs to set the current EGL surface.

The second one is wlr_output_attach_buffer.
wlr_output_impl.attach_buffer is removed in [1].

When wlr_output_rollback is called, all pending state is supposed to be
cleared. This works for all the state except the two exceptions
mentionned above. To fix this, introduce wlr_output_impl.rollback.

Right now, the backend resets the current EGL surface. This prevents GL
commands from affecting the output after wlr_output_rollback.

This patch is required for FBO-based outputs to work properly. The
compositor might be using FBOs for its own purposes [2], having leftover
FBO state can have bad consequences.

[1]: swaywm#2097
[2]: swaywm#2063 (comment)
ddevault pushed a commit that referenced this pull request Apr 8, 2020
Most of the pending output state is not forwarded to the backend prior
to an output commit. For instance, wlr_output_set_mode just stashes the
mode without calling any wlr_output_impl function.
wlr_output_impl.commit is responsible for applying the pending mode.

However, there are exceptions to this rule. The first one is
wlr_output_attach_render. It won't go away before renderer v6 is
complete, because it needs to set the current EGL surface.

The second one is wlr_output_attach_buffer.
wlr_output_impl.attach_buffer is removed in [1].

When wlr_output_rollback is called, all pending state is supposed to be
cleared. This works for all the state except the two exceptions
mentionned above. To fix this, introduce wlr_output_impl.rollback.

Right now, the backend resets the current EGL surface. This prevents GL
commands from affecting the output after wlr_output_rollback.

This patch is required for FBO-based outputs to work properly. The
compositor might be using FBOs for its own purposes [2], having leftover
FBO state can have bad consequences.

[1]: #2097
[2]: #2063 (comment)
@emersion emersion marked this pull request as ready for review April 8, 2020 15:56
@any1
Copy link
Contributor

any1 commented Apr 9, 2020

I tested this. It works well and there are no obvious regressions.

@emersion
Copy link
Member Author

Rebased and added a fallback for the internal format.

@any1
Copy link
Contributor

any1 commented Apr 14, 2020

The fallback is perhaps not terribly important, but might I suggest RGB565 instead?

@emersion emersion requested a review from ddevault April 14, 2020 13:13
@ddevault
Copy link
Contributor

Overall, this LGTM

@emersion emersion force-pushed the headless-fbo branch 2 times, most recently from f61db96 to 6ada106 Compare April 21, 2020 13:01
@emersion
Copy link
Member Author

Comments addressed.

@ddevault ddevault merged commit 6129a6f into swaywm:master Apr 22, 2020
@ddevault
Copy link
Contributor

Thanks!

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

Successfully merging this pull request may close these issues.

5 participants