Improve variable naming for RgbaSurface. Remove unnecessary comments. #6
Conversation
| /// The pixel data for the image. | ||
| /// The data does not need to be tightly packed, but if it isn't, stride must be different from `width * 4`. | ||
| /// | ||
| /// Expected to be at least `stride * height` bytes long. |
There was a problem hiding this comment.
change stride to stride_in_bytes :)
| pub data: &'a [u8], | ||
| pub width: u32, | ||
| pub height: u32, | ||
| pub stride: u32, | ||
| /// The stride between the rows of pixels, in bytes. |
There was a problem hiding this comment.
I think there's no need to prefix with The here, same for the other doc-comments?
| let mut surface = kernel::rgba_surface { | ||
| width: surface.width as i32, | ||
| height: surface.height as i32, | ||
| stride: surface.stride as i32, | ||
| stride: surface.stride_in_bytes as i32, | ||
| ptr: surface.data.as_ptr() as *mut u8, | ||
| }; |
There was a problem hiding this comment.
Any idea why every file is manually converting from RgbaSurface to kernel::RgbaSurface?
There was a problem hiding this comment.
That's taken from Graham's code. kernel::rgba_surface appears to be part of the generated code, so adding an impl for it seems like it would be hacky, since you'll be relying on it matching the generated Rust code for the struct.
I assume this might be the reason it also isn't straight up using kernel::rgba_surface as part of the interface. I think it would be more user friendly to have a struct that is unlikely to change between versions, and if you have changes to the kernel struct, you only change the conversion rather than the user having to change their code.
There was a problem hiding this comment.
Generated code is imported into this crate, so you should be able to safely implement an impl From<RgbaSurface> for kernel::rgba_surface and do away with a bunch of duplicated code.
No description provided.