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

Feature: Extend CLI #14

Merged
merged 3 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 126 additions & 0 deletions src/cli/binary.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
use crate::cli::defines::BinaryFormat;
use crate::write_buf_as_raw_array;
use anyhow::Result;
use clap::{Args, ValueEnum};
use std::{
fs::File,
io::{self, BufReader, BufWriter, Write},
mem,
path::PathBuf,
};

// MARK: - Args

#[derive(Args, Debug)]
pub struct BinaryArgs {
/// Path to the PNG input file
input: String,

/// Output file. Defaults to input file name with ".bin" appended
#[arg(short)]
output: Option<String>,

/// Output format
#[arg(value_enum, short, long)]
format: BinaryFormat,

/// Flip the image on the x axis
#[arg(long)]
flip_x: bool,

/// Flip the image on the y axis
#[arg(long)]
flip_y: bool,

/// Output a raw C array which can be `#include`d in a file. The default output type width matches the FORMAT provided, but it can be overridden with --c_array_width
#[arg(long)]
c_array: bool,

/// Overrides the natural fit of each format when outputting a C array
#[arg(long, value_enum)]
c_array_width: Option<CArrayWidth>,
}

// MARK: - Handlers

pub fn handle_binary(args: &BinaryArgs) -> Result<()> {
let input_file = File::open(&args.input).expect("could not open input file");
let mut input_reader = BufReader::new(input_file);

// Convert the image
let mut bin: Vec<u8> = Vec::new();
if let BinaryFormat::Palette = args.format {
pigment64::create_palette_from_png(&mut input_reader, &mut bin)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some places say crate:: and others say pigment64::. Is it needed to be different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so that has to do with when something comes from the library and what comes from within the cli package. Things prefixed with pigment64 are being imported from the lib - the cli is a user of the lib. Things with crate are within the cli package and are not part of the lib.

} else {
let mut image = pigment64::PNGImage::read(&mut input_reader)?;

if args.flip_x || args.flip_y {
image = image.flip(args.flip_x, args.flip_y);
}

image.as_native(&mut bin, args.format.as_native())?;
};

let mut output_file: Box<dyn Write>;

if args.c_array && args.output.is_none() {
output_file = Box::from(io::stdout());
} else {
let output_path = PathBuf::from(args.output.clone().unwrap_or_else(|| {
let mut path = args.input.clone();
if args.c_array {
path.push_str(".inc.c");
} else {
path.push_str(".bin");
}
path
}));

let file = File::create(output_path)?;
output_file = Box::from(file);
}

if args.c_array {
// Override array width if the user passed the appropriate flag
let c_array_width = args.c_array_width.unwrap_or(args.format.get_width());

match c_array_width {
CArrayWidth::U8 => write_buf_as_u8(&mut output_file, &bin),
CArrayWidth::U16 => write_buf_as_u16(&mut output_file, &bin),
CArrayWidth::U32 => write_buf_as_u32(&mut output_file, &bin),
CArrayWidth::U64 => write_buf_as_u64(&mut output_file, &bin),
}
} else {
BufWriter::new(output_file).write_all(&bin)?;
}

Ok(())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for returning an empty Ok(()) instead of just a void return? I don't see any Error anywhere

Copy link
Contributor Author

@dcvz dcvz Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be hard see unless you’re familiar with it, but some things that /could/ error are being handled with ? at the end, which would bubble the error out. If you make it to the last Ok(()) then nothing before failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's what ? mean. Gotcha, thanks.

}

// MARK: - Structs

#[derive(Copy, Clone, PartialEq, Eq, ValueEnum, Debug)]
pub enum CArrayWidth {
U8,
U16,
U32,
U64,
}

// MARK: - Helpers

fn write_buf_as_u8(output_file: &mut Box<dyn Write>, bin: &[u8]) {
write_buf_as_raw_array!(output_file, bin, u8);
}

fn write_buf_as_u16(output_file: &mut Box<dyn Write>, bin: &[u8]) {
write_buf_as_raw_array!(output_file, bin, u16);
}

fn write_buf_as_u32(output_file: &mut Box<dyn Write>, bin: &[u8]) {
write_buf_as_raw_array!(output_file, bin, u32);
}

fn write_buf_as_u64(output_file: &mut Box<dyn Write>, bin: &[u8]) {
write_buf_as_raw_array!(output_file, bin, u64);
}
65 changes: 65 additions & 0 deletions src/cli/defines.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use crate::cli::binary::CArrayWidth;
use clap::ValueEnum;
use pigment64::ImageSize::Bits4;
use pigment64::{ImageSize, ImageType};

#[derive(Copy, Clone, PartialEq, Eq, ValueEnum, Debug)]
pub enum BinaryFormat {
Ci4,
Ci8,
I4,
I8,
Ia4,
Ia8,
Ia16,
Rgba16,
Rgba32,
Palette,
}

impl BinaryFormat {
pub fn get_width(&self) -> CArrayWidth {
match self {
BinaryFormat::Ci4 => CArrayWidth::U8,
BinaryFormat::Ci8 => CArrayWidth::U8,
BinaryFormat::I4 => CArrayWidth::U8,
BinaryFormat::I8 => CArrayWidth::U8,
BinaryFormat::Ia4 => CArrayWidth::U8,
BinaryFormat::Ia8 => CArrayWidth::U8,
BinaryFormat::Ia16 => CArrayWidth::U16,
BinaryFormat::Rgba16 => CArrayWidth::U16,
BinaryFormat::Rgba32 => CArrayWidth::U32,
BinaryFormat::Palette => CArrayWidth::U16,
}
}

pub fn as_native(&self) -> ImageType {
match self {
BinaryFormat::Ci4 => ImageType::Ci4,
BinaryFormat::Ci8 => ImageType::Ci8,
BinaryFormat::I4 => ImageType::I4,
BinaryFormat::I8 => ImageType::I8,
BinaryFormat::Ia4 => ImageType::Ia4,
BinaryFormat::Ia8 => ImageType::Ia8,
BinaryFormat::Ia16 => ImageType::Ia16,
BinaryFormat::Rgba16 => ImageType::Rgba16,
BinaryFormat::Rgba32 => ImageType::Rgba32,
BinaryFormat::Palette => panic!("cannot convert palette to native format"),
}
}

pub fn get_size(&self) -> ImageSize {
match self {
BinaryFormat::Ci4 => Bits4,
BinaryFormat::Ci8 => ImageSize::Bits8,
BinaryFormat::I4 => ImageSize::Bits4,
BinaryFormat::I8 => ImageSize::Bits8,
BinaryFormat::Ia4 => ImageSize::Bits4,
BinaryFormat::Ia8 => ImageSize::Bits8,
BinaryFormat::Ia16 => ImageSize::Bits16,
BinaryFormat::Rgba16 => ImageSize::Bits16,
BinaryFormat::Rgba32 => ImageSize::Bits32,
BinaryFormat::Palette => panic!("cannot convert palette to native format"),
}
}
}
17 changes: 17 additions & 0 deletions src/cli/macros.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#[macro_export]
macro_rules! write_buf_as_raw_array {
($dst:expr, $bin:expr, $type_width:ident) => {
let width = mem::size_of::<$type_width>();

for row in $bin.chunks(16) {
let mut line_list = Vec::new();
for bytes in row.chunks(width) {
let value = $type_width::from_be_bytes(bytes.try_into().unwrap());

line_list.push(format!("0x{value:00$X}", 2 * width));
}
let line = line_list.join(", ");
write!($dst, " {line},\n").expect("could not write to output file");
}
};
}
5 changes: 5 additions & 0 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pub mod defines;
pub mod macros;

pub mod binary;
pub mod png;
105 changes: 105 additions & 0 deletions src/cli/png.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
use crate::cli::defines::BinaryFormat;
use anyhow::Result;
use clap::Args;
use pigment64::image::native_image::parse_tlut;
use pigment64::TextureLUT;
use std::fs::File;
use std::io::{BufReader, BufWriter, Read, Write};
use std::path::PathBuf;

// MARK: - Args

#[derive(Args, Debug)]
pub struct PngArgs {
/// Path to the binary input file
input: String,

/// Width of the binary image
#[arg(long)]
width: u32,

/// Height of the binary image
#[arg(long)]
height: u32,

/// Input format
#[arg(value_enum, short, long)]
format: BinaryFormat,

/// Output file. Defaults to input file name with ".png" appended
#[arg(short, long)]
output: Option<String>,

/// Path to the palette binary file (only required for CI formats)
#[arg(short, long)]
palette: Option<String>,

/// Flip the image on the x axis
#[arg(long)]
flip_x: bool,

/// Flip the image on the y axis
#[arg(long)]
flip_y: bool,
}

// MARK: - Handlers

pub fn handle_png(args: &PngArgs) -> Result<()> {
assert_ne!(args.format, BinaryFormat::Palette, "palette is not a supported standalone output format. Use format ci4/ci8 with --palette instead.");

// Open the input file
let input_file = File::open(&args.input).expect("could not open input file");
let mut input_reader = BufReader::new(input_file);

// Convert the image
let image = pigment64::NativeImage::read(
&mut input_reader,
args.format.as_native(),
args.width,
args.height,
)?;

let mut output: Vec<u8> = Vec::new();

// if format is ci4/ci8, read the palette
if let BinaryFormat::Ci4 | BinaryFormat::Ci8 = args.format {
// Read the palette
let palette_file = File::open(
args.palette
.as_ref()
.expect("palette is required for ci4/ci8 formats"),
)
.expect("could not open palette file");
let mut palette_reader = BufReader::new(palette_file);
let mut palette_bytes = Vec::new();
palette_reader.read_to_end(&mut palette_bytes)?;

let palette = parse_tlut(&palette_bytes, args.format.get_size(), TextureLUT::Rgba16)?;
image.as_png(&mut output, Some(&palette))?;
} else {
image.as_png(&mut output, None)?;
}

// Handle flips, we do this on the already produced PNG because it's easier
if args.flip_x || args.flip_y {
let mut image = pigment64::PNGImage::read(&mut output.as_slice())?;
image = image.flip(args.flip_x, args.flip_y);
output.clear();
image.as_png(&mut output)?;
}

// Write the file
let output_file: Box<dyn Write>;
let output_path = PathBuf::from(args.output.clone().unwrap_or_else(|| {
let mut path = args.input.clone();
path.push_str(".png");
path
}));

let file = File::create(output_path)?;
output_file = Box::from(file);
BufWriter::new(output_file).write_all(&output)?;

Ok(())
}
11 changes: 11 additions & 0 deletions src/image/png_image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,17 @@ impl PNGImage {
}
}

/// Writes the image as a PNG to the given writer.
/// This is useful for when you need to flip an existing image.
pub fn as_png<W: Write>(&self, writer: &mut W) -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be more clear to have this function named as write_as_png?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We’re using this naming semantic everywhere else so I kept it. We’re writing data to some medium for sure. I don’t feel strongly about it. Do you find this name confusing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, if this naming scheme is already being used then it is fine.
I think it may be a bit confusing to read something like image.as_png(), but that discussion is out of scope for this PR

let mut encoder = png::Encoder::new(writer, self.width, self.height);
encoder.set_color(self.color_type);
encoder.set_depth(self.bit_depth);
let mut writer = encoder.write_header()?;
writer.write_image_data(&self.data)?;
Ok(())
}

pub fn as_native<W: Write>(&self, writer: &mut W, image_type: ImageType) -> Result<()> {
match image_type {
ImageType::I4 => self.as_i4(writer),
Expand Down
Loading
Loading