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

fix(wayland) : buffer->busy is always true in cpu render mode #242

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HR1025
Copy link
Contributor

@HR1025 HR1025 commented Oct 31, 2022

weston is an implementation of wayland. And weston provide three render ways:

  • OpenGL : GPU
  • Pixman : CPU
  • Noop : CPU

use weston --use-pixman to explicitly indicate render in pixman render mode.

In fact, handle_wl_buffer_release is only invoked in GPU render mode. Just like the comment state:

	/**
	 * compositor releases buffer
	 *
	 * Sent when this wl_buffer is no longer used by the compositor.
	 * The client is now free to reuse or destroy this buffer and its
	 * backing storage.
	 *
	 * If a client receives a release event before the frame callback
	 * requested in the same wl_surface.commit that attaches this
	 * wl_buffer to a surface, then the client is immediately free to
	 * reuse the buffer and its backing storage, and does not need a
	 * second buffer for the next surface content update. Typically
	 * this is possible, when the compositor maintains a copy of the
	 * wl_surface contents, e.g. as a GL texture. This is an important
	 * optimization for GL(ES) compositors with wl_shm clients.
	 */

@HR1025
Copy link
Contributor Author

HR1025 commented Oct 31, 2022

Base on 10.0.2 branch of weston, just change the code as follow:
image

When use cpu to render, the log is shown below:
image

When use gpu to render, the log is shown below:
image

And wl_buffer_send_release is the driver of handle_wl_buffer_release.

Just like you see, when use gpu to render, weston will send a nullptr to weston_buffer_reference, making the code can
step into wl_buffer_send_release. weston use reference count to manger share memory!!!

@HR1025
Copy link
Contributor Author

HR1025 commented Oct 31, 2022

It may be a bad solution but it's useful. (May be we can find a better solution to solove this problem.)

Now, if weston use cpu to render, we will always at the follow code and can't commit render:

    else if (buffer->busy)
    {
        LV_LOG_WARN("skip flush since wayland backing buffer is busy");
        lv_disp_flush_ready(disp_drv);
        return;
    }

@kisvegabor
Copy link
Member

Thanks for this PR. Unfortunately, I don't know the Wayland driver in such detail to comment so I tag @WallaceIT @simplejack-src, the former contributors of this driver. Could you take a look a guys?

@WallaceIT
Copy link
Contributor

WallaceIT commented Nov 3, 2022

Hi @HR1025 ,

thank you for the analysis. If I understand well, in the CPU renderer case the release events are not sent by weston because we keep updating the same wl_buffer - and this is not what the compositor expect.

I'm not sure yours is the most correct solution, as it might lead to concurrent usage of a buffer. We should probably destroy and re-create the wl_buffer once it has been used. I need some time to think about this solution and try to implement it (but contributions are welcome, obviously).

@simplejack-src do you have some other ideas?

@HR1025
Copy link
Contributor Author

HR1025 commented Nov 3, 2022

Yes, that's what I want to say.Thanks.

@HR1025
Copy link
Contributor Author

HR1025 commented Nov 10, 2022

@kisvegabor
So far, I haven't found a more appropriate solution. I think https://github.com/lvgl/lv_drivers/pull/250 may solve this problem.
Should we convert these merge to issue for now?

I will still explore in weston for a period of time. If possible, I will try to solve these problems. But it may take some time, maybe a month or two.

@WallaceIT
Copy link
Contributor

Hi,

can you try the code in PR #253? While @simplejack-src solution in PR #250 is more flexible, I think a simple double-buffering approach could work, at least for now.

@HR1025
Copy link
Contributor Author

HR1025 commented Nov 12, 2022

@WallaceIT I will try it latter. Thanks.

@stale
Copy link

stale bot commented Apr 20, 2023

This issue or pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants