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

Introduce wlr_buffer #1050

Merged
merged 4 commits into from Jun 13, 2018

Conversation

Projects
None yet
5 participants
@emersion
Copy link
Member

emersion commented Jun 8, 2018

This PR introduces wlr_buffer, as discussed in #1046 (comment).

wlr_buffers are ref-counted and hold a texture. When they are destroyed, the wl_buffer is released and the texture is destroyed.

A compositor can reference the buffer when it needs to:

struct wlr_buffer *buffer = wlr_buffer_ref(surface->buffer);

// Use buffer->texture (can be NULL if client destroys the buffer)

wlr_buffer_unref(buffer);

@ammen99 Can you provide some feedback? Would this approach work for you?

  • We probably want to get rid of wlr_surface.texture
  • Listen for renderer destroy

Fixes #1046

emersion added some commits Jun 8, 2018

@ammen99

This comment has been minimized.

Copy link
Member

ammen99 commented Jun 9, 2018

can be NULL if client destroys the buffer
I'm not sure about how buffers work in wayland, but is this going to happen for buffers that are uploaded as textures (like wl_shm)?

Otherwise this approach seems good to me.

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jun 9, 2018

I'm not sure about how buffers work in wayland, but is this going to happen for buffers that are uploaded as textures (like wl_shm)?

For wl_shm, wlr_buffer.resource will be NULL if the client destroys the buffer, but wlr_buffer.texture won't be NULL because we copied the data when uploading to the GPU (so even if the client destroys its copy, we're still able to read our own copy).

For wl_drm and dma-buf, the texture is shared with the client, so if the client destroys it early (before we send the release event) we loose access to the data. In general, this should not happen. This may happen when a client exits for instance (if you still have a reference to the wlr_buffer when the client terminates).

Additional note: if you reference a wlr_buffer backed by wl_shm and don't release it before the next commit, we'll have no choice but to re-upload the whole texture (wlr_buffers that are not released are immutable). I'm trying to think of a way to do it that would allow you to keep two buffers around and upload damage from the last 2 frames, but this will probably require more work in wlr_surface (maybe by allowing users to override the apply damage function, or require users to manage their wlr_buffers themselves).

@ddevault ddevault requested a review from ascent12 Jun 9, 2018

@ammen99

This comment has been minimized.

Copy link
Member

ammen99 commented Jun 10, 2018

@emersion, fair enough, I'd also be interested in a way to be able to check whether the texture will survive (e.g wl_shm) or it can be destroyed at any time (dma-buf), because for some reason I might decide I really need the texture so I'd want to copy it manually if it is necessary.

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jun 10, 2018

I'd also be interested in a way to be able to check whether the texture will survive (e.g wl_shm) or it can be destroyed at any time (dma-buf)

That's easy, just check wlr_buffer.released. If the buffer is not released, then then client can destroy the texture.

/**
* Reference the buffer.
*/
void wlr_buffer_ref(struct wlr_buffer *buffer);

This comment has been minimized.

@atomnuker

atomnuker Jun 11, 2018

Contributor

wlr_buffer_ref should return a wlr_buffer *, since that's what API users will do - ref, create a new wlr_buffer struct and copy the contents of the original wlr_buffer.

This comment has been minimized.

@emersion

emersion Jun 11, 2018

Member

Good idea. I don't get the last part though:

create a new wlr_buffer struct and copy the contents of the original wlr_buffer.

This would duplicate the ref counter?

@Timidger Timidger referenced this pull request Jun 11, 2018

Open

Wrap wlr_buffer #199

@emersion

This comment has been minimized.

Copy link
Member

emersion commented Jun 13, 2018

Ping @ascent12

@ddevault ddevault merged commit 5e4af48 into swaywm:master Jun 13, 2018

1 check passed

builds.sr.ht builds.sr.ht job completed successfully
Details
@ddevault

This comment has been minimized.

Copy link
Member

ddevault commented Jun 13, 2018

Thanks!

@emersion emersion deleted the emersion:wlr-buffer branch Jun 13, 2018

ddevault added a commit that referenced this pull request Jun 13, 2018

Revert "Merge pull request #1050 from emersion/wlr-buffer"
This reverts commit 5e4af48, reversing
changes made to 9a1f0e2.

@emersion emersion restored the emersion:wlr-buffer branch Jun 14, 2018

@emersion emersion referenced this pull request Jun 14, 2018

Merged

Add back wlr_buffer #1062

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment