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

Implement PackedArray::as_slice #306

Closed
MatrixDev opened this issue Jun 12, 2023 · 1 comment · Fixed by #307
Closed

Implement PackedArray::as_slice #306

MatrixDev opened this issue Jun 12, 2023 · 1 comment · Fixed by #307
Labels
c: core Core components feature Adds functionality to the library

Comments

@MatrixDev
Copy link

MatrixDev commented Jun 12, 2023

// SAFETY: Packed arrays are stored contiguously in memory, so we can use
// pointer arithmetic instead of going through `$operator_index_const` for
// every index.

If PackedArrays are stored contiguously in memory, it should be possible to return a slice instead of copying all data to the vec. It seams like a guite significant overhead especially when working with Image.

Unfortunately, currently it is impossible to implement as an extension trait because PackedArray::ptr and PackedArray::ptr_mut are private.

Proposed implementation is quite simple:

pub fn as_slice(&self) -> &[$Element] {
    let len = self.len();
    let ptr = self.ptr(0);
    unsafe { std::slice::from_raw_parts(ptr, len) }
}

pub fn as_mut_slice(&self) -> &mut [$Element] {
    let len = self.len();
    let ptr = self.ptr_mut(0);
    unsafe { std::slice::from_raw_parts_mut(ptr, len) }
}
@Bromeon Bromeon added feature Adds functionality to the library c: core Core components labels Jun 12, 2023
@Bromeon
Copy link
Member

Bromeon commented Jun 12, 2023

Fun fact: this was already implemented in #137, but the author closed the pull request and did not respond anymore 🤷

I can rewrite the code later, would anyway do a few things a bit differently.

bors bot added a commit that referenced this issue Jun 12, 2023
307: Implement `PackedArray::as_slice()`, `as_slice_mut()` r=Bromeon a=Bromeon

Resumes abandoned work of #137.
Closes #306.

Open points:
1. Test CoW across GDScript/Rust boundaries?
2. We now have `slice`, `as_slice` and `as_mut_slice`, where the first returns another array.
    I wonder if it would be clearer to name it `sub_array` or something instead of `slice`?

bors try

Co-authored-by: Jan Haller <bromeon@gmail.com>
bors bot added a commit that referenced this issue Jun 12, 2023
307: Implement `PackedArray::as_slice()`, `as_slice_mut()` r=Bromeon a=Bromeon

Resumes abandoned work of #137.
Closes #306.

Open points:
1. Test CoW across GDScript/Rust boundaries?
2. We now have `slice`, `as_slice` and `as_mut_slice`, where the first returns another array.
    I wonder if it would be clearer to name it `sub_array` or something instead of `slice`?

bors try

Co-authored-by: Jan Haller <bromeon@gmail.com>
bors bot added a commit that referenced this issue Jun 12, 2023
307: Implement `PackedArray::as_slice()`, `as_slice_mut()` r=Bromeon a=Bromeon

Resumes abandoned work of #137.
Closes #306.

Open points:
1. Test CoW across GDScript/Rust boundaries?
2. We now have `slice`, `as_slice` and `as_mut_slice`, where the first returns another array.
    I wonder if it would be clearer to name it `sub_array` or something instead of `slice`?

bors try

Co-authored-by: Jan Haller <bromeon@gmail.com>
bors bot added a commit that referenced this issue Jun 12, 2023
307: Implement `PackedArray::as_slice()`, `as_slice_mut()` r=Bromeon a=Bromeon

Resumes abandoned work of #137.
Closes #306.

Open points:
1. Test CoW across GDScript/Rust boundaries?
2. We now have `slice`, `as_slice` and `as_mut_slice`, where the first returns another array.
    I wonder if it would be clearer to name it `sub_array` or something instead of `slice`?

bors try

Co-authored-by: Jan Haller <bromeon@gmail.com>
bors bot added a commit that referenced this issue Jun 12, 2023
307: Implement `PackedArray::as_slice()`, `as_slice_mut()` r=Bromeon a=Bromeon

Resumes abandoned work of #137.
Closes #306.

Open points:
1. Test CoW across GDScript/Rust boundaries?
2. We now have `slice`, `as_slice` and `as_mut_slice`, where the first returns another array.
    I wonder if it would be clearer to name it `sub_array` or something instead of `slice`?

bors try

Co-authored-by: Jan Haller <bromeon@gmail.com>
@bors bors bot closed this as completed in 533a8d6 Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Core components feature Adds functionality to the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants