Skip to content

Add reborrowing PixmapMut::as_mut #150

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

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

Conversation

robbie01
Copy link

@robbie01 robbie01 commented May 4, 2025

This allows you to get PixmapMuts all the way down, which can simplify structs and function signatures.

This also adds PixmapRef::as_ref for parity, which is just a Copy alias.

Also add PixmapRef::as_ref for parity
@RazrFalcon
Copy link
Collaborator

What is the use case for this?

@robbie01
Copy link
Author

robbie01 commented May 5, 2025

What is the use case for this?

I was working on an API boundary, and ended up with a function signature that looked like:

fn render(&self, canvas: &mut PixmapMut<'_>)

which felt a bit redundant to me, as a PixmapMut already represents a mutable borrow on a (real or logical) Pixmap. Reborrowing would allow just passing a PixmapMut, which avoids indirection by not computing any new pointers; it just reuses the existing one with borrow checking.

This also allows for better ergonomics, as you can pass pixmap.as_mut(), pixmap_mut, or pixmap_mut.as_mut() (note that the last one doesn't consume pixmap_mut) to the function instead of needing to pass &mut pixmap.as_mut() or &mut pixmap_mut.

The most compelling piece of prior art (in my opinion) is std::pin::Pin::as_mut (docs). More prior art exists in the form of std::io::BorrowedCursor::reborrow (docs) and esp_hal::peripherals::PeripheralRef::reborrow (docs), which both reborrow a smart reference without indirection. The BorrowMut trait achieves something similar for vanilla references.

@RazrFalcon
Copy link
Collaborator

which avoids indirection by not computing any new pointers

There are no pointers in this code. Just references and they are zero cost in Rust, unless I'm missing something.

The current expected approach is to use pixmap: &mut tiny_skia::PixmapMut. Otherwise you would not be able to call mutating methods on PixmapMut. For example, this code would not compile:

fn fill_path(pixmap: PixmapMut) {
    pixmap.fill(Color::TRANSPARENT);
}

@CryZe
Copy link
Contributor

CryZe commented May 6, 2025

But

fn fill_path(mut pixmap: PixmapMut) {
    pixmap.fill(Color::TRANSPARENT);
}

would.

Just references and they are zero cost in Rust, unless I'm missing something.

Unless the function is inlined, Rust references basically always compile down to pointers. So @robbie01 is indeed correct that it reduces the amount of indirection. It just likely won't last long as fill_path itself takes &mut self (&mut PixmapMut).

@RazrFalcon
Copy link
Collaborator

Good point. I don't remember why I have ended up using &mut PixmapMut everywhere.
Still, I cannot imagine it effecting performance in any way.

So @robbie01 is indeed correct that it reduces the amount of indirection.

That's strange. I thought that the only source of implicit indirection in Rust is trait objects.

@RazrFalcon
Copy link
Collaborator

Ideally, we should follow some Rust API guidelines here, if there are any. I'm not sure what is "the correct" way of handling "mutable pointer wrappers".

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

Successfully merging this pull request may close these issues.

3 participants