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

render/pixman: avoid copy when creating textures #2892

Merged
merged 5 commits into from
Jun 7, 2021

Conversation

emersion
Copy link
Member

@emersion emersion commented Apr 26, 2021

This allows the Pixman renderer to stop memcpy'ing textures.

  • Address the shm client buffer SIGBUS issue

Depends on: #2851

cc @bl4ckb0ne

emersion added a commit to emersion/wlroots that referenced this pull request Apr 26, 2021
Allow wlr_buffer_impl.get_data_ptr to return a format.

This allows the Pixman renderer to not care about get_dmabuf/get_shm,
and only care about get_data_ptr. This will also help with [1], because
client wl_shm buffers can't implement get_shm.

[1]: swaywm#2892

References: swaywm#2864
emersion added a commit that referenced this pull request Apr 27, 2021
Allow wlr_buffer_impl.get_data_ptr to return a format.

This allows the Pixman renderer to not care about get_dmabuf/get_shm,
and only care about get_data_ptr. This will also help with [1], because
client wl_shm buffers can't implement get_shm.

[1]: #2892

References: #2864
@emersion emersion force-pushed the shm-reuse-buffer branch 2 times, most recently from bee9a4a to df0436c Compare April 27, 2021 18:56
@emersion
Copy link
Member Author

With this PR, the memcpy for client buffers is removed with the Pixman renderer. On my machine, CPU usage goes from 6% down to 3% when playing a fullscreen video.

@emersion emersion changed the title WIP: cache and re-use shm textures WIP: render/pixman: avoid copy when creating textures May 19, 2021
@emersion
Copy link
Member Author

emersion commented May 19, 2021

The last commit contains an attempt at fixing the SIGBUS issue (see #2864). It adds guards to data ptr accesses to allow wl_shm_buffer to prevent compositor crashes. To test, start wlroots with the Pixman renderer and then run wleird's sigbus client.

Some questions:

  • I'm not 100% happy with the additional state-tracking that wlr_buffer needs to do. OTOH, a more elaborate acquire/release mechanism would likely be over-engineering, and wl_shm_buffer offers the same {begin,end}_access API.
  • Should the guard be separate from the data ptr attribute retrieval? e.g. have both functions in the API: wlr_buffer_get_data_ptr(buffer, &data, &format, &stride) + wlr_buffer_begin_data_ptr_access(buffer)
  • Should we transition to a more mmap-like interface? e.g. wlr_buffer_map(buffer, &data, &format, &stride) + wlr_buffer_unmap(buffer, data) (implementations would be free to retain the same mapping under the hood). See also: gbm_bo_{map,unmap}.
  • Name bikeshedding?

cc @bl4ckb0ne

@emersion emersion force-pushed the shm-reuse-buffer branch 11 times, most recently from 6a147ca to 9b5d1c4 Compare June 2, 2021 15:50
This new API allows buffer implementations to know when a user is
actively accessing the buffer's underlying storage. This is
important for the upcoming client-backed wlr_buffer implementation.
Introduce wlr_shm_client_buffer, which provides a wlr_buffer wrapper
around wl_shm_buffer.

Because the client can destroy the wl_buffer while we still are using
it, we need to do some libwayland tricks to still be able to continue
accessing its underlying storage. We need to reference the wl_shm_pool
and save the data pointer.
@emersion emersion changed the title WIP: render/pixman: avoid copy when creating textures render/pixman: avoid copy when creating textures Jun 2, 2021
@emersion emersion added the breaking Breaking change in public API label Jun 2, 2021
@emersion emersion added this to the 0.14.0 milestone Jun 2, 2021
@emersion emersion marked this pull request as ready for review June 2, 2021 15:57
@emersion
Copy link
Member Author

emersion commented Jun 2, 2021

Should the guard be separate from the data ptr attribute retrieval?

No: in the case of wl_shm_buffer, the data pointer is not static and may change if the client resizes the shm pool.

Should we transition to a more mmap-like interface?

We can't easily provide such an interface with wl_shm_buffer. We may be able to do it if we re-implement wl_shm.

@emersion
Copy link
Member Author

emersion commented Jun 2, 2021

This is now ready for review.

@bl4ckb0ne bl4ckb0ne merged commit 534615c into swaywm:master Jun 7, 2021
@emersion emersion deleted the shm-reuse-buffer branch June 7, 2021 13:39
@emersion emersion mentioned this pull request Jun 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking Breaking change in public API
Development

Successfully merging this pull request may close these issues.

None yet

2 participants