ShareImage: Place Texts on Images to generate Social Media Preview Images

Question

ShareImage is a project which lets you use an Image ("Template") and place text over it, to generate Social Media Preview Images, the ones used in the og:image tag. The project is available at the regraphic/si-rs repo. Here's the src/lib.rs file (which contains all the code):

use image::{DynamicImage, GenericImage, GenericImageView, Rgb, Rgba};
use reqwest;
use rusttype::{point, Font, Scale};
use wasm_bindgen::prelude::*;

/// Represents a font used for text rendering.
#[wasm_bindgen]
#[derive(Clone)]
pub struct SiFont {
    font: Option<Font<'static>>,
}

#[wasm_bindgen]
impl SiFont {
    /// Constructor for SiFont, asynchronously loading font data from a URL or using provided bytes.
    ///
    /// # Arguments
    ///
    /// * `src` - The URL of the font file.
    /// * `src_bytes` - Optional bytes of the font file.
    ///
    /// # Returns
    ///
    /// A `SiFont` instance containing the loaded font.
    #[wasm_bindgen(constructor)]
    #[cfg(feature = "async")]
    pub async fn new(src: &str, src_bytes: Option<Vec<u8>>) -> Result<SiFont, JsValue> {
        // Load font data from either URL or provided bytes.
        let font_data = match src_bytes {
            Some(bytes) => bytes.to_vec(),
            None => reqwest::get(src)
                .await
                .expect("Could not fetch font")
                .bytes()
                .await
                .expect("Could not extract font")
                .to_vec(),
        };
        let font = Font::try_from_vec(font_data);
        Ok(SiFont { font })
    }

    /// Constructor for SiFont, synchronously loading font data from a URL or using provided bytes.
    ///
    /// # Arguments
    ///
    /// * `src` - The URL of the font file.
    /// * `src_bytes` - Optional bytes of the font file.
    ///
    /// # Returns
    ///
    /// A `SiFont` instance containing the loaded font.
    #[cfg(feature = "blocking")]
    pub fn new(src: &str, src_bytes: Option<Vec<u8>>) -> SiFont {
        // Load font data from either URL or provided bytes.
        let font_data = match src_bytes {
            Some(bytes) => bytes.to_vec(),
            None => reqwest::blocking::get(src)
                .expect("Could not fetch font")
                .bytes()
                .expect("Could not extract font")
                .to_vec(),
        };
        let font = Font::try_from_vec(font_data.to_vec());
        SiFont { font }
    }
}

/// Represents an image with text rendering capabilities.
#[wasm_bindgen]
#[derive(Clone)]
pub struct SiImage {
    font: SiFont,
    image: DynamicImage,
    height: u32,
    width: u32,
}

#[wasm_bindgen]
impl SiImage {
    /// Constructor for SiImage, asynchronously loading an image from a URL or using provided bytes.
    ///
    /// # Arguments
    ///
    /// * `image_url` - The URL of the image file.
    /// * `font` - A `SiFont` instance for text rendering.
    /// * `image_bytes` - Optional bytes of the image file.
    ///
    /// # Returns
    ///
    /// A `SiImage` instance containing the loaded image and font.
    #[wasm_bindgen(constructor)]
    #[cfg(feature = "async")]
    pub async fn new(image_url: &str, font: SiFont, image_bytes: Option<Vec<u8>>) -> SiImage {
        // Load image data from either URL or provided bytes.
        let image_data = match image_bytes {
            Some(bytes) => bytes.to_vec(),
            None => reqwest::get(image_url)
                .await
                .expect("Could not fetch image")
                .bytes()
                .await
                .expect("Could not extract image")
                .to_vec(),
        };
        let image = image::load_from_memory(&image_data).expect("Could not decode image");
        let d = image.clone().dimensions();
        Self {
            font,
            image,
            height: d.1,
            width: d.0,
        }
    }

    /// Constructor for SiImage, synchronously loading an image from a URL or using provided bytes.
    ///
    /// # Arguments
    ///
    /// * `image_url` - The URL of the image file.
    /// * `font` - A `SiFont` instance for text rendering.
    /// * `image_bytes` - Optional bytes of the image file.
    ///
    /// # Returns
    ///
    /// A `SiImage` instance containing the loaded image and font.
    #[cfg(feature = "blocking")]
    pub fn new(image_url: &str, font: SiFont, image_bytes: Option<Vec<u8>>) -> SiImage {
        // Load image data from either URL or provided bytes.
        let image_data = match image_bytes {
            Some(bytes) => bytes.to_vec(),
            None => reqwest::blocking::get(image_url)
                .expect("Could not fetch image")
                .bytes()
                .expect("Could not extract image")
                .to_vec(),
        };
        let image = image::load_from_memory(&image_data).expect("Could not decode image");
        let d = image.clone().dimensions();
        Self {
            font,
            image,
            height: d.1,
            width: d.0,
        }
    }

    /// Render text onto the image.
    ///
    /// # Arguments
    ///
    /// * `text` - The text to render.
    /// * `text_scale` - The scale factor for the text.
    /// * `pos_x` - The X-coordinate for text placement.
    /// * `pos_y` - The Y-coordinate for text placement.
    /// * `color` - Optional text color in hexadecimal format (e.g., "#RRGGBB").
    ///
    /// # Returns
    ///
    /// A new `SiImage` instance with the rendered text.
    #[wasm_bindgen]
    pub fn text(
        &mut self,
        text: &str,
        text_scale: f32,
        pos_x: f32,
        pos_y: f32,
        color: Option<String>,
    ) -> SiImage {
        let mut image = self.image.clone();
        let font = self
            .font
            .font
            .as_ref()
            .ok_or("Error loading font")
            .expect("Could not decode/load font");
        let scale = Scale::uniform(text_scale);
        let v_metrics = font.v_metrics(scale);
        let offset = point(pos_x, pos_y + v_metrics.ascent);

        for glyph in font.layout(text, scale, offset) {
            if let Some(bb) = glyph.pixel_bounding_box() {
                glyph.draw(|x, y, v| {
                    let x = x as i32 + bb.min.x;
                    let y = y as i32 + bb.min.y;
                    let pixel = image.get_pixel(x as u32, y as u32);
                    let parsed_color = match color.clone() {
                        Some(c) => hex_to_rgb(&c).unwrap_or(Rgb([0, 0, 0])),
                        None => Rgb([0, 0, 0]),
                    };
                    let new_pixel = Rgba([
                        (((parsed_color[0] as f32 * (v)) as f32) + (pixel[0] as f32 * (1.0 - v)))
                            as u8,
                        ((parsed_color[1] as f32 * (v)) as f32 + (pixel[1] as f32 * (1.0 - v)))
                            as u8,
                        ((parsed_color[2] as f32 * (v)) as f32 + (pixel[2] as f32 * (1.0 - v)))
                            as u8,
                        (pixel[3] as f32 * (v)) as u8,
                    ]);
                    image.put_pixel(x as u32, y as u32, new_pixel);
                });
            }
        }

        self.image = image.clone();

        SiImage {
            font: self.font.clone(),
            image,
            height: self.height,
            width: self.width,
        }
    }

    /// Get the image data as bytes in PNG format.
    ///
    /// # Returns
    ///
    /// A `Vec<u8>` containing the image data.
    #[wasm_bindgen(getter)]
    pub fn as_bytes(&self) -> Vec<u8> {
        let mut v = std::io::Cursor::new(Vec::new());
        self.image
            .write_to(&mut v, image::ImageFormat::Png)
            .expect("Could not write bytes");
        v.into_inner()
    }

    /// Set the font for text rendering.
    ///
    /// # Arguments
    ///
    /// * `font` - A `SiFont` instance for text rendering.
    ///
    /// # Returns
    ///
    /// A new `SiImage` instance with the updated font.
    #[wasm_bindgen(setter, js_name = "font")]
    pub fn font(&mut self, font: SiFont) -> SiImage {
        self.font = font;
        SiImage {
            font: self.font.clone(),
            image: self.image.clone(),
            height: self.height,
            width: self.width,
        }
    }

    /// Get the height of the image.
    #[wasm_bindgen(getter)]
    pub fn height(&self) -> u32 {
        self.height
    }

    /// Get the width of the image.
    #[wasm_bindgen(getter)]
    pub fn width(&self) -> u32 {
        self.width
    }
}

/// Converts a hexadecimal color string (e.g., "#RRGGBB") to an `Rgb<u8>` color.
///
/// # Arguments
///
/// * `hex` - The hexadecimal color string.
///
/// # Returns
///
/// An `Option<Rgb<u8>>` representing the RGB color.
pub fn hex_to_rgb(hex: &str) -> Option<Rgb<u8>> {
    let hex = hex.trim_start_matches('#'); // Remove "#" if present
    if hex.len() == 6 {
        let r = u8::from_str_radix(&hex[0..2], 16).ok()?;
        let g = u8::from_str_radix(&hex[2..4], 16).ok()?;
        let b = u8::from_str_radix(&hex[4..6], 16).ok()?;
        Some(Rgb([r, g, b]))
    } else if hex.len() == 3 {
        let r = u8::from_str_radix(&hex[0..1].repeat(2), 16).ok()?;
        let g = u8::from_str_radix(&hex[1..2].repeat(2), 16).ok()?;
        let b = u8::from_str_radix(&hex[2..3].repeat(2), 16).ok()?;
        Some(Rgb([r, g, b]))
    } else {
        Some(Rgb([0, 0, 0]))
    }
}

The code is primarily the base for the shareimage (Node.js) and sideno (Deno) projects and is ~6x faster than the existing ones. However, I'm open to any feedback on it.

Answer 1

You clone the Vec<u8> unnecessarily

In each of the new() methods, you have an Option<Vec<u8>>. Then you pattern-match on it and get Vec<u8> (or fetch the bytes from the network). Then you call .to_vec() on that, which effectively clones the Vec, unnecessarily since you already have a Vec and you don't need it twice.

You do it again unnecessarily

In the blocking version of SiFont::new() you call .to_vec() on before Font::try_from_vec() again (either on the bytes from the network or on the Vec from the parameter), again cloning it for no need.

There's a more efficient way to convert Bytes to Vec<u8>

reqwests bytes() gives you a Bytes. You need a Vec<u8>. Since Bytes derefs to [u8], you can call slice's methods on it, including to_vec(). However, <[T]>::to_vec() will always allocate a new Vec; there is a more efficient way to do this conversion.

You can use the impl From<Bytes> for Vec<u8>. This avoids allocating a new Vec if the Bytes instances uniquely owns the data (it is not shared), and I think (not sure) it also avoids a copy if the data is already in the right place. Since both are likely (perhaps will always) to hold when the Bytes came from reqwest, there can be a significant saving here.

You clone the image unnecessarily

When calling dimensions() on it. This is not needed, dimensions() takes &self.

Don't force callers into allocating

SiFont's constructor actually need an owned Vec<u8> for the bytes. But SiImage's constructor does not. All it needs is &[u8], for image::load_from_memory(). Therefore, it should take &[u8], to avoid forcing a caller that only has a slice (for example, because the image is embedded into the binary) to allocate and copy. This also allows you to avoid the conversion of Bytes to Vec (although, if done correctly as per the previous note, should be cheap).

Using the data from reqwest::get() as a slice without getting a lifetime error might be tricky, though, but you can use a little trick: declare the Bytes response before the match, so its lifetime is long enough, but only initialize it in the match (conditionally-initialize it). Then, return a reference to it from the match, and store it in a variable. The compiler knows that in all code paths, it is initialized before accessed (if accessed).

Let the caller decide whether to clone

In SiImage::text(), either take &mut self, update the image in place and return nothing (better), or take self and return Self (builder-like pattern). You can also take &mut self and return &mut Self, for chaining. But either way, let the caller decide if they want to clone. They know better than you if they need two copies of the image.

Currently, your code does the worst of both worlds: it clones the image, updates the clone then clones it again to also update the original. This way, you always clone twice for no reason: if the caller only needs the updated image, it needs to update in-place (no clone at all), but you clone twice; and if the caller wants both the original and the updated image, it still needs to clone before to keep the original image (so, three clones instead of just one).

You clone color unnecessarily too

In match color.clone().

This time, this actually has a reason (the code won't compile without it), but it is easily fixable: match on &color instead of on color, to not move it.

...and you should take Option<&str> for it, anyway

Since you don't really need it as a String, so again, let the caller decide.

Extract the color parsing out of the loop

You need to do it once and not more.

Also, there are some more comments on color parsing if it would be performant sensitive. They would make it less idiomatic but faster. Currently it is performance sensitive. If you'll extract it out of the loop it won't be anymore.

---

The rest are not performance notes.

Handle errors gracefully

Instead of panicking in the case of errors, return proper Results.

Features should be additive

Imagine what happens if someone enables both the async and the blocking features. It doesn't even have to be the same person: one dependency of you uses the async version and thus enables the async feature, and another one uses the blocking interface and thus enables the blocking feature. They work great in isolation, until you try to add both to your project and suddenly it fails to compile with a mysterious error, since Cargo is trying to unify the features.

This is why features should be additive; that is, enabling a feature should not change API, but add API. With your code this is not the case: enabling the features changes the asyncness of a function.

You should make it two functions, one async behind the async feature gate and the other sync behind the blocking feature gate. You can name them new() and new_blocking(), or new_async() and new(), or new_async() and new_blocking() (or whatever you want).

One constructor is just not enough

It's nice to have only one constructor, but in this case it just complicates things. There are two ways to construct a SiFont and a SiImage, one from bytes and the other from the network. Trying to unify them just doesn't work. Everyone wanting to construct from bytes will need to provide a dummy url, and everyone wanting a network request will need to provide None for the bytes (of course, this could also be the opposite. or you could make both Options, but then you have to handle the error of both absent or both present at runtime). And it's also easy to have both a URL and bytes by mistake, and the code perhaps won't do what you want it to do.

Instead, make illegal states irrepresentable. Have two constructor functions, from_bytes() and from_network(). For WebAssembly, you can overload the constructor to determine if it was given bytes or string (although I don't think wasm_bindgen can do that automatically), but IMHO it will be clearer to leave the constructor for one of the options and use a static factory function for the other (or use static functions for both!).

Destructure the dimensions immediately

After let d = image.clone().dimensions(), it is not immediately clear what is the width and what is the height (also, I'd use a more descriptive name than d, but this is arguable). It is much better to destructure it on the same line: let (width, height) = image.clone().dimensions().

Handle errors when they occur, not later

Instead of storing Option<Font>, store Font and handle the case of an invalid font when the font is loaded. This way, you also don't litter every access with expect().

You may want to differentiate between "no color given" and "invalid color was given"

Currently, you use black as a fallback for both. Consider instead returning an error in the case of an invalid color.

text() is not a clear name

And also, I prefer method names to be verbs. paint_text() would be more appropriate IMHO.

Neither as_ nor getter

Your as_bytes() method violates two expectations of people: first, it is named as_. By convention, as_ prefix should be reserved to operations that are near free. It is not. It is expensive. It should be named to_bytes(). Second, it is a getter (in JavaScript). People expect getters to be cheap. It is not. And as the tip of the iceberg, it is a getter named as_bytes, which is probably not what you intended (you probably wanted to call it bytes).

font() should not clone too

Just as with text(), let the caller decide. Either take &mut self and update in place, or take self and return Self, but don't do both.

font() sounds like a getter

By convention, you should name it set_font(). Unless you go for a builder-style API (which might as well be what you're trying to do, I'm not sure).