Port Cpp code to Rust

[triniwiz]

So I've been trying to convert some cpp to rust (still learning ) I tried a couple times but it doesn't seem to work and I'm just scratching my head

void flipPixels(GLubyte *pixels, size_t bytesPerRow, size_t rows) {
  if (!pixels) {
    return;
  }

  GLuint middle = (GLuint)rows / 2;
  GLuint intsPerRow = (GLuint)bytesPerRow / sizeof(GLuint);
  GLuint remainingBytes = (GLuint)bytesPerRow - intsPerRow * sizeof(GLuint);

  for (GLuint rowTop = 0, rowBottom = (GLuint)rows - 1; rowTop < middle; ++rowTop, --rowBottom) {
    // Swap in packs of sizeof(GLuint) bytes
    GLuint *iTop = (GLuint *)(pixels + rowTop * bytesPerRow);
    GLuint *iBottom = (GLuint *)(pixels + rowBottom * bytesPerRow);
    GLuint iTmp;
    GLuint n = intsPerRow;
    do {
      iTmp = *iTop;
      *iTop++ = *iBottom;
      *iBottom++ = iTmp;
    } while (--n > 0);

    // Swap remainder bytes
    GLubyte *bTop = (GLubyte *)iTop;
    GLubyte *bBottom = (GLubyte *)iBottom;
    GLubyte bTmp;
    switch (remainingBytes) {
      case 3:
        bTmp = *bTop;
        *bTop++ = *bBottom;
        *bBottom++ = bTmp;
      case 2:
        bTmp = *bTop;
        *bTop++ = *bBottom;
        *bBottom++ = bTmp;
      case 1:
        bTmp = *bTop;
        *bTop = *bBottom;
        *bBottom = bTmp;
    }
  }
}

My first attempt

 fn flip_in_place(pixels: *mut u8, bytes_per_row: i32, height: i32) {
    let middle: u32 = (height / 2) as u32;
    let ints_per_row = bytes_per_row / std::mem::size_of::<u32>() as i32;
    let remaining_bytes = bytes_per_row - ints_per_row * std::mem::size_of::<u32>() as i32;
    let mut row_top = 0;
    let mut row_bottom = height - 1;
    unsafe {
        for _ in 0..middle {
            let mut i_top = pixels.offset((row_top * bytes_per_row) as isize);
            let mut i_bottom = pixels.offset((row_bottom * bytes_per_row) as isize);
            let mut i_tmp;
            let mut n = ints_per_row;
            loop {
                i_tmp = i_top;

                i_top = i_top.offset(1);
                *i_top = *i_bottom;

                i_bottom = i_bottom.offset(1);
                *i_bottom = *i_tmp;
                n -= 1;
                if n == 0 {
                    break;
                }
            }

            let mut b_top = i_top;
            let mut b_bottom = i_bottom;
            let mut b_tmp;
            match remaining_bytes {
                3 => {
                    b_tmp = b_top;
                    b_top = b_top.offset(1);
                    *b_top = *b_bottom;
                    b_bottom = b_bottom.offset(1);
                    *b_bottom = *b_tmp;
                }
                2 => {
                    b_tmp = b_top;
                    b_top = b_top.offset(1);
                    *b_top = *b_bottom;
                    b_bottom = b_bottom.offset(1);
                    *b_bottom = *b_tmp;
                }
                1 => {
                    b_tmp = b_top;
                    *b_top = *b_bottom;
                    *b_bottom = *b_tmp;
                }
                _ => {}
            }
            
             row_top += 1;
            row_bottom -= 1;
        }
    }
}

There were others but feeling lost at this point

Thanks in advance ... oh the method is for UNPACK_FLIP_Y_WEBGL

[bjorn3]

@triniwiz

*bTop++ = *bBottom;

This should be translated to:

*i_bottom = *i_tmp;
i_bottom = i_bottom.offset(1);

Postfix increment returns the old value, not the new one.

[2ndTaleStudio]

@triniwiz

void flipPixels(GLubyte *pixels, size_t bytesPerRow, size_t rows)

Hey - without digging to much into the details at first I know that C/C++ is often using pointers to represent the existence of arrays. I would not try to use raw pointers to reflect the same in Rust.

I'd first start with your function signature and start then from there. How this signature finally looks like depends a bit on how it will be called. To reduce the need for converting number types back and fourth I'd use usize as much as possible as this is the type to be used to index into an array/slice.

fn flip_in_place(pixels: &mut [u8], bytes_per_row: usize, height: usize) {
}

If the pixels slice might be optional to this function you could wrap it into an Option<&mut [u8]> and immediately leave the function if this is_none. However, with the pixels being provided as a slice you do not need unsafe code to work and manipulate the same. You could start using iterators and related functions to achieve your goal.

[tafia]

Once the signature has been set, in idiomatic rust, you don't work with pointers at all, you will only work with slices (borrowed) or owned buffers.

let mut input = vec![0u8; 1000];

// ...
let pixels = &mut input;

// you can swap rows this way
let tmp = pixels[i_top * bytesPerRow ..(i_top + 1) * bytesPerRow].to_vec(); // move to an owned tmp buffer
pixels[i_top * bytesPerRow ..(i_top + 1) * bytesPerRow].copy_from_slice(&pixels[i_bottom * bytesPerRow ..(i_bottom + 1) * bytesPerRow]);
pixels[i_bottom * bytesPerRow ..(i_bottom + 1) * bytesPerRow].copy_from_slice(&tmp);

[triniwiz]

Thanks for the info since I started wondering what the postfix increment would look like

[triniwiz]

Thanks using usize now also the pixels incoming would be from ffi (android/ios and maybe web if the framework reaches there ) sometimes it might be from a rust vec or not

[triniwiz]

Thanks for this I updated like

 fn flip_in_place(pixels: *mut u8, length: usize, bytes_per_row: usize, height: usize) {
    let mut slice = unsafe { std::slice::from_raw_parts_mut(pixels, length) };
    let mut i_top = 0;
    let mut i_bottom = height - 1;
    let middle = height / 2;
    for _ in 0..middle {
        let tmp = slice[i_top * bytes_per_row..(i_top + 1) * bytes_per_row].to_vec();
        let bottom_row = slice[i_bottom * bytes_per_row..(i_bottom + 1) * bytes_per_row].to_vec();
        slice[i_top * bytes_per_row..(i_top + 1) * bytes_per_row].copy_from_slice(&bottom_row);
        slice[i_bottom * bytes_per_row..(i_bottom + 1) * bytes_per_row].copy_from_slice(&tmp);
        i_top += 1;
        i_bottom -= 1;
    }
}

Think it might be wasteful with the extra .to_vec() ?

[Yandros]

assuming:

mod GL {
    #![allow(nonstandard_style)]
    pub type ubyte = ...; // say `u8`
    pub type uint = ...; // say `u32`
}

---

void flipPixels (GLubyte *pixels, size_t bytesPerRow, size_t rows)
{

becomes

fn flip_pixels (
    pixels: &'_ mut [GL::ubyte], // contains `total_bytes_count`
    rows: usize,
)
{
    let bytes_per_row = pixels.len() / rows; 

---

  if (!pixels) {
    return;
  }

becomes:

• unnecessary

---

  GLuint middle = (GLuint)rows / 2;
  GLuint intsPerRow = (GLuint)bytesPerRow / sizeof(GLuint);
  GLuint remainingBytes = (GLuint)bytesPerRow - intsPerRow * sizeof(GLuint);

becomes:

    let middle = rows / 2;
    let ints_per_row =
        bytes_per_row / mem::size_of::<GL::uint>()
    ;
    let remaining_bytes = bytes_per_row % mem::size_of::<GL::uint>();

• I have replaced your unchecked casts and wrong usage of uint with usize (C's size_t).

---

for (GLuint rowTop = 0, rowBottom = (GLuint)rows - 1; rowTop < middle; ++rowTop, --rowBottom) {

becomes:

for (row_top, row_bottom) in (0 .. middle).zip((middle .. rows).rev()) {

---

    // Swap in packs of sizeof(GLuint) bytes
    GLuint *iTop = (GLuint *)(pixels + rowTop * bytesPerRow);
    GLuint *iBottom = (GLuint *)(pixels + rowBottom * bytesPerRow);
    GLuint iTmp;
    GLuint n = intsPerRow;
    do {
      iTmp = *iTop;
      *iTop++ = *iBottom;
      *iBottom++ = iTmp;
    } while (--n > 0);

becomes:

debug_assert_ne!(row_top, row_bottom);
let bytes = bytes.as_mut_ptr();
let i_row_top: &mut [GL::uint] = unsafe {
    slice::from_raw_parts_mut(
        bytes.add(row_top * bytes_per_row).cast::<GL::uint>(),
        ints_per_row,
    )
};
let i_row_bot: &mut [GL::uint] = unsafe {
    slice::from_raw_parts_mut(
        bytes.add(row_bottom * bytes_per_row).cast::<GL::uint>(),
        ints_per_row,
    )
};
for (i_top, i_bot) in i_row_top.iter_mut().zip(i_row_bot) {
    mem::swap(i_top, i_bot);
}

• with a use ::core::{mem, slice};

---

    // Swap remainder bytes
    GLubyte *bTop = (GLubyte *)iTop;
    GLubyte *bBottom = (GLubyte *)iBottom;
    GLubyte bTmp;
    switch (remainingBytes) {
      case 3:
        bTmp = *bTop;
        *bTop++ = *bBottom;
        *bBottom++ = bTmp;
      case 2:
        bTmp = *bTop;
        *bTop++ = *bBottom;
        *bBottom++ = bTmp;
      case 1:
        bTmp = *bTop;
        *bTop = *bBottom;
        *bBottom = bTmp;
    }

becomes:

if remaining_bytes == 0 { continue; }
let mut b_top = row_top * bytes_per_row + ints_per_row * mem::size_of::<GL::uint>();
let mut b_bottom = row_bottom * bytes_per_row + ints_per_row * mem::size_of::<GL::uint>();
debug_assert!(remaining_bytes < 4);
for _ in 0 .. remaining_bytes { // optimizer should be able to unroll this loop
    let tmp = pixels[b_top];
    pixels[b_top] = pixels[b_bottom];
    b_top += 1;
    pixels[b_bottom] = tmp;
    b_bottom += 1;
}

---

Whole code:

use ::core::{mem, slice};

mod GL {
    #![allow(nonstandard_style)]
    pub type ubyte = ...; // say `u8`
    pub type uint = ...; // say `u32`
}

pub
fn flip_pixels (
    pixels: &'_ mut [GL::ubyte], // contains `total_bytes_count`
    rows: usize,
)
{
    let bytes_per_row = pixels.len() / rows;
    let middle = rows / 2;
    let ints_per_row =
        bytes_per_row / mem::size_of::<GL::uint>()
    ;
    let remaining_bytes =
        bytes_per_row % mem::size_of::<GL::uint>()
    ;
    for (row_top, row_bottom)
        in (0 .. middle).zip((middle .. rows).rev())
    {
        debug_assert_ne!(row_top, row_bottom);
        let bytes = pixels.as_mut_ptr();
        let i_row_top: &mut [GL::uint] = unsafe {
            slice::from_raw_parts_mut(
                bytes.add(row_top * bytes_per_row)
                    .cast::<GL::uint>()
                ,
                ints_per_row,
            )
        };
        let i_row_bot: &mut [GL::uint] = unsafe {
            slice::from_raw_parts_mut(
                bytes.add(row_bottom * bytes_per_row)
                    .cast::<GL::uint>()
                ,
                ints_per_row,
            )
        };
        for (i_top, i_bot)
            in i_row_top.iter_mut().zip(i_row_bot)
        {
            mem::swap(i_top, i_bot);
        }
        
        if remaining_bytes == 0 { continue; }
        let mut b_top =
            row_top * bytes_per_row
            +
            ints_per_row * mem::size_of::<GL::uint>()
        ;
        let mut b_bottom =
            row_bottom * bytes_per_row
            +
            ints_per_row * mem::size_of::<GL::uint>()
        ;
        debug_assert!(remaining_bytes < 4);
        for _ in 0 .. remaining_bytes { // optimizer should be able to unroll this loop
            let tmp = pixels[b_top];
            pixels[b_top] = pixels[b_bottom];
            b_top += 1;
            pixels[b_bottom] = tmp;
            b_bottom += 1;
        }
    }
}

• Playground

---

Final version

The above is the "naively" translated code. Now, that code is using unsafe, and bit too much manual indexing, imho. Time to use Rust slices and iterators for the win:

use ::core::{convert::TryInto, mem};

pub
fn flip_pixels (
    pixels: &'_ mut [u8],
    rows: usize,
)
{
    let bytes_per_row = pixels.len() / rows;
    let (first_half, second_half) =
        pixels.split_at_mut(pixels.len() / 2)
    ;
    let top_rows = first_half.chunks_mut(bytes_per_row);
    let bot_rows = second_half.chunks_mut(bytes_per_row).rev();
    for (top, bot) in top_rows.zip(bot_rows) {
        // mem::swap(top, bot);
        const SWAP_SIZE: usize = 4; // tweak to your liking
        let mut i_tops = top.chunks_exact_mut(SWAP_SIZE);
        let mut i_bots = bot.chunks_exact_mut(SWAP_SIZE);
        for (i_top, i_bot) in Iterator::zip(
            i_tops.by_ref(),
            i_bots.by_ref(),
        )
        {
            type SwapBlock = [u8; SWAP_SIZE];
            let i_top: &mut SwapBlock = i_top.try_into().unwrap();
            let i_bot: &mut SwapBlock = i_bot.try_into().unwrap();
            mem::swap(i_top, i_bot);
        }
        for (b_top, b_bot) in Iterator::zip(
            i_tops.into_remainder().iter_mut(),
            i_bots.into_remainder().iter_mut(),
        )
        {
            mem::swap(b_top, b_bot);
        }
    }
}

• Playground

[AndreKR]

@triniwiz

*bTop++ = *bBottom;

And this, ladies and gentlemen, is why Rust does not have an increment operator.

[triniwiz]

I didn't know it was that bad the code is from react-native-expo-gl is I kinda assumed it worked fine (since it worked when I test it on ios ) .. I wanted to do something similar in rust since I love it

[triniwiz]

looking at this makes me feel like I'm not even close to understanding rust #win

[AndreKR]

@triniwiz

I didn't know it was that bad

I didn't say that. Personally I find it quite difficult to "run"

*bTop++ = *bBottom;

in my head, but then again I'm not really a C++ guy.

[ZiCog]

That is a particularly horrible way to write anything in C. Even worse in C++.

[ZiCog]

Actually, if I recall correctly that kind of C thing is some kind of "idiomatic" C way to do things dating back to the original Kernigan and Ritchie "White Book": https://en.wikipedia.org/wiki/The_C_Programming_Language

Where you might see a copy of the elements of one array to another as a loop around:

    *dst++ = *src++;

But really, it's horrible. Is that dereferencing the src pointer and then incrementing the thing it points to? Or is it dereferencing src, and assigning the thing it points to to the left had side, then incrementing the pointer. Or WTF?

Which is why I get nervous when I hear people talk of "idiomatic Rust".

You mean you want to rewrite your code such that nobody intimately familiar with Rust can understand it?

[alanhkarp]

Or, @ZiCog, is it line noise? That's what it looks like to an old Fortran programmer like me.

[ZiCog]

It's line noise.

It's line noise that becomes so common in the language that the language experts think it's immediately obvious as they have seen it so much.

It could be written as:

*dst = *src;
dst = dst +1;
src = src +1;

Which would at least give the uninitiated a chance to be sure what is intended in the code.

But oh no, the advice is to do it the "idiomatic" way. Even if it has no performance benefit or anything.

I recall my English teacher in school explaining that an "idiom" was a common phrase in some language which made no sense at all when literally translated to other languages. Very hard to understand by non-native speakers. For example "Teaching your grandmother to suck eggs".

Well, guess what? We live in a world of dozens, hundreds of programming languages in common use. Many people have to deal with more than one of them.

The last thing we need in source code is particular language idioms that are incomprehensible to those from other programming languages and not entirely necessary.