Tileset export feature #5
Conversation
|
I'm not sure how we want to go about resolving the indexed pixels in this case. This may change depending on how we want to factor the pixel format data, per #4. We could store a reference to the required index-resolution data. EDIT: We could do something similar to |
|
Ok, this PR makes a few new changes:
|
src/pixel.rs
Outdated
| @@ -43,13 +48,13 @@ impl Grayscale { | |||
| let alpha = reader.byte()?; | |||
| Ok(Self { value, alpha }) | |||
| } | |||
| pub(crate) fn as_rgba(&self) -> Rgba { | |||
| pub(crate) fn to_rgba(self) -> Rgba { | |||
There was a problem hiding this comment.
Why did you rename this function? Is it because it consumes the Self type? Maybe into_rgba would be clearer, as that parallels into_iter? Though, I'm not aware of a clear convention.
There was a problem hiding this comment.
It looks like "into" is the correct one in this case: https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv
src/file.rs
Outdated
| ) -> pixel::Rgba { | ||
| let palette = self.palette.as_ref().expect("Expected a palette present when resolving indexed image. Should have been caught in validation"); | ||
| let transparent_color_index = self.pixel_format.transparent_color_index().expect("Indexed tilemap pixels in non-indexed pixel format. Should have been caught in validation"); | ||
| pixel |
There was a problem hiding this comment.
Since this function is called per-pixel it probably shouldn't have so many failure points. These first two should always be valid after the first invocation of the function. So, maybe we can make this a standalone function (i.e., no self argument) and pass in the palette reference and transparent color index as arguments. Then do the checks where you construct index resolver closure below.
There was a problem hiding this comment.
The closure would still need to be constructed conditionally to avoid unwrapping stuff when the closure's not needed. I briefly tried a "closure factory" signature (e.g || -> |px| -> rgba) and that caused some lifetime issues. I decided to just have Pixels accept the optional arguments, and in the Indexed case, do the unwrapping, and then iterate.
src/tileset.rs
Outdated
| @@ -134,6 +137,37 @@ impl Tileset { | |||
| self.external_file.as_ref() | |||
| } | |||
|
|
|||
| pub(crate) fn image(&self, image_pixels: Vec<image::Rgba<u8>>) -> RgbaImage { | |||
There was a problem hiding this comment.
Small nit. Since this is an internal function with the same name as a global API function, it would be nice if this would have a more internal-sounding name. E.g., more along the lines of tileset_image_from_pixels (not great). Or even tilemap_image. Maybe you can think of something better.
There was a problem hiding this comment.
I decided on write_to_image to match similar internal functions that return RgbaImage.
|
Do you think this is ready for review/merge? |
Yes, I think so. We might want to merge both this and #6 together as the 0.3 release, or we can increment the version twice. |
|
@alpine-alpaca Are there any other changes you'd like here? |
|
This is included in the merged PR, right? |
|
Handled by #9 |
When working with tile data, it can be useful to export an image of the Tileset in order to render each tile on the texture exactly once.
The Aseprite beta UI doesn't currently support exporting a Tileset image directly. For now, @dacap shared a lua script that will do the same thing. The function in this PR is tested against an image generated by this lua script.