Skip to content

Allow reading from buffers - #320

Merged
madsmtm merged 1 commit into
masterfrom
madsmtm/allow-reading-buffer
Mar 24, 2026
Merged

Allow reading from buffers#320
madsmtm merged 1 commit into
masterfrom
madsmtm/allow-reading-buffer

Conversation

@madsmtm

Copy link
Copy Markdown
Member

In #313, I added a note that reading from the pixel buffer is discouraged, because I wanted to set kIOSurfaceMapWriteCombineCache on macOS once I got around to implementing proper IOSurface support. See e.g. the following links for a few places where it's used in the wild:

I thought this was fine, because I couldn't see a use-case where you'd need to read from the pixel buffer, but that was perhaps a bit hasty, I found at least one use-case: converting the alpha mode (premultiplying components or making the alpha channel opaque) by modifying the buffer in-place in a separate step before presenting it buffer.

This means that the argument for only having the mutable version instead of distinct pixels/pixels_mut, pixel_rows/pixel_rows_mut and pixels_iter/pixels_iter_mut methods is a bit weaker.

WDYT? Should we provide both immutable and mutable methods on Buffer<'_>?

@madsmtmmadsmtm added documentation Improvements or additions to documentation enhancement New feature or request DS - CoreGraphics macOS/iOS/tvOS/watchOS/visionOS backend labels Jan 23, 2026
@madsmtmmadsmtm changed the title Add API for making buffers write-only and allow reading from them by defaultAdd API for making buffers write-only, and allow reading from them by defaultJan 23, 2026
Comment threadsrc/lib.rs Outdated
/// Set the buffer as optimized for only being written to.
///
/// Setting this allows the underlying storage to bypass certain caches and reduce cache
/// pollution. In turn, this may make reading from the buffer data perform very poorly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's not really "write only", but more optimized for not reading? Though I guess it should generally be treated the same...

Not sure if any other name would be more appropriate for this method. Maybe not.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe write_optimized? Or read_penalized?

@madsmtm

Copy link
Copy Markdown
MemberAuthor

I just found gfx-rs/wgpu#8897, we might have to make this method unsafe because the write-combined memory seems to work weirdly with atomics? I'm unfamiliar with how write-combined memory actually works.

@madsmtm
madsmtmforce-pushed the madsmtm/allow-reading-buffer branch from 2ac7aa8 to 51f29e0CompareMarch 18, 2026 14:32
@madsmtmmadsmtm changed the title Add API for making buffers write-only, and allow reading from them by defaultAllow reading from buffersMar 18, 2026
@madsmtm

Copy link
Copy Markdown
MemberAuthor

I've changed this PR to only be a docs update to allow reading from the buffer, we can reconsider the set_write_only API later.

@madsmtm
madsmtmforce-pushed the madsmtm/allow-reading-buffer branch from 51f29e0 to 53be8ddCompareMarch 24, 2026 17:01
Disallowing reading is overly restrictive, and we don't use
write-combined memory anywhere yet anyhow.
@madsmtm
madsmtmforce-pushed the madsmtm/allow-reading-buffer branch from 53be8dd to fdef29aCompareMarch 24, 2026 17:01
@madsmtm
madsmtm merged commit 7140e44 into masterMar 24, 2026
40 checks passed
@madsmtm
madsmtm deleted the madsmtm/allow-reading-buffer branch March 24, 2026 17:09
@nicoburns

nicoburns commented Mar 24, 2026

Copy link
Copy Markdown

My understanding is that on certain devices (usually mobile devices, but not sure about Apple mobile devices), there is a hardware fast path for rendering directly into the display controller (skipping the GPU), but that can't then be read back. So the performance gain here is likely from setting the flag on the IoSurface or whatever. Probably it makes sense to let users control this depending on their needs?

@madsmtmmadsmtm mentioned this pull request Mar 25, 2026
3 tasks
@madsmtm

Copy link
Copy Markdown
MemberAuthor

I'm not aware of any option for controlling this on Android? The closest I could find was AHARDWAREBUFFER_USAGE_CPU_READ_NEVER , but that seems to be used for a rwlock instead, it doesn't actually control the memory caching mode?

I think your understanding is about right on Apple mobile devices though, but it only matters if we can use IOSurface, which I don't currently see a good way to do.

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

Labels

documentationImprovements or additions to documentationDS - CoreGraphicsmacOS/iOS/tvOS/watchOS/visionOS backendenhancementNew feature or request

Development

Successfully merging this pull request may close these issues.

3 participants

@madsmtm@nicoburns@ids1024