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

Late night refactors #195

Merged
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
13 changes: 5 additions & 8 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,8 @@ tempfile = "3.8.1"
wiremock = "0.5.21"

# Selectively bump up opt level for some dependencies to improve dev build perf
[profile.dev.package.ttf-parser]
opt-level = 2
[profile.dev.package.rustybuzz]
opt-level = 2
[profile.dev.package.cosmic-text]
opt-level = 2
[profile.dev.package.png]
opt-level = 2
[profile.dev.package]
ttf-parser.opt-level = 2
rustybuzz.opt-level = 2
cosmic-text.opt-level = 2
png.opt-level = 2
15 changes: 13 additions & 2 deletions src/color.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fs::File;
use std::io::BufReader;
use std::path::PathBuf;
use std::sync::OnceLock;

use anyhow::Context;
use serde::Deserialize;
Expand Down Expand Up @@ -45,6 +46,11 @@ pub struct Theme {

impl Theme {
pub fn dark_default() -> Self {
static CACHED_CODE_HIGHLIGHTER: OnceLock<SyntectTheme> = OnceLock::new();
// Initializing this is non-trivial. Cache so it only runs once
let code_highlighter = CACHED_CODE_HIGHLIGHTER
.get_or_init(|| ThemeDefaults::Base16OceanDark.into())
.to_owned();
Self {
text_color: 0x9DACBB,
background_color: 0x1A1D22,
Expand All @@ -53,11 +59,16 @@ impl Theme {
link_color: 0x4182EB,
select_color: 0x3675CB,
checkbox_color: 0x0A5301,
code_highlighter: SyntectTheme::from(ThemeDefaults::Base16OceanDark),
code_highlighter,
}
}

pub fn light_default() -> Self {
static CACHED_CODE_HIGHLIGHTER: OnceLock<SyntectTheme> = OnceLock::new();
// Initializing this is non-trivial. Cache so it only runs once
let code_highlighter = CACHED_CODE_HIGHLIGHTER
.get_or_init(|| ThemeDefaults::InspiredGithub.into())
.to_owned();
Self {
text_color: 0x000000,
background_color: 0xFFFFFF,
Expand All @@ -66,7 +77,7 @@ impl Theme {
link_color: 0x5466FF,
select_color: 0xCDE8F0,
checkbox_color: 0x96ECAE,
code_highlighter: SyntectTheme::from(ThemeDefaults::InspiredGithub),
code_highlighter,
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/file_watcher/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fn endlessly_handle_messages<C: Callback>(
.unwrap();

let poll_registering_watcher = |watcher: &mut RecommendedWatcher, file_path: &Path| loop {
std::thread::sleep(Duration::from_millis(20));
std::thread::sleep(Duration::from_millis(15));

let _ = watcher.unwatch(file_path);
if watcher
Expand Down
144 changes: 104 additions & 40 deletions src/file_watcher/tests.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::fs;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::mpsc;
use std::time::Duration;

use super::{Callback, Watcher};
use crate::test_utils::init_test_log;

use tempfile::TempDir;

impl Callback for mpsc::Sender<()> {
fn file_reload(&self) {
Expand Down Expand Up @@ -59,70 +60,133 @@ impl Delays {
}
}

#[test]
fn the_gauntlet() {
init_test_log();

// This test can be flaky, so give it a few chances to succeed
let mut last_panic = None;
let mut delays = Delays::new();
for _ in 0..3 {
let result = std::panic::catch_unwind(|| the_gauntlet_flaky(delays.clone()));
let Err(panic) = result else {
return;
};
last_panic = Some(panic);
delays.increase_delays();
}

std::panic::resume_unwind(last_panic.unwrap());
struct TestEnv {
temp_dir: TempDir,
main_file: PathBuf,
rel_file: PathBuf,
watcher: Watcher,
callback_rx: mpsc::Receiver<()>,
}

// Unfortunately this needs to be littered with sleeps/timeouts to work right :/
fn the_gauntlet_flaky(delays: Delays) {
// Create our dummy test env
let temp_dir = tempfile::Builder::new()
.prefix("inlyne-tests-")
.tempdir()
.unwrap();
let base = temp_dir.path();
let main_file = base.join("main.md");
let rel_file = base.join("rel.md");
let swapped_in_file = base.join("swap_me_in.md");
let swapped_out_file = base.join("swap_out_to_me.md");
fs::write(&main_file, "# Main\n\n[rel](./rel.md)").unwrap();
fs::write(&rel_file, "# Rel").unwrap();
fs::write(&swapped_in_file, "# Swapped").unwrap();
impl TestEnv {
fn init() -> Self {
// Create our dummy test env
let temp_dir = tempfile::Builder::new()
.prefix("inlyne-tests-")
.tempdir()
.unwrap();
let base = temp_dir.path();
let main_file = base.join("main.md");
let rel_file = base.join("rel.md");
fs::write(&main_file, "# Main\n\n[rel](./rel.md)").unwrap();
fs::write(&rel_file, "# Rel").unwrap();

// Setup our watcher
let (callback_tx, callback_rx) = mpsc::channel();
let watcher = Watcher::spawn_inner(callback_tx, main_file.clone());

// Setup our watcher
let (callback_tx, callback_rx) = mpsc::channel::<()>();
let watcher = Watcher::spawn_inner(callback_tx, main_file.clone());
Self {
temp_dir,
main_file,
rel_file,
watcher,
callback_rx,
}
}
}

macro_rules! gen_watcher_test {
( $( ($test_name:ident, $test_fn:ident) ),* $(,)? ) => {
$(
#[test]
fn $test_name() {
$crate::test_utils::init_test_log();

// Give the test a few chances
let mut last_panic = None;
let mut delays = Delays::new();
for _ in 0..3 {
let result = std::panic::catch_unwind(|| {
let test_dir = TestEnv::init();
// Give the watcher time to get comfy :)
delays.delay();

$test_fn(test_dir, delays.clone())
});
let Err(panic) = result else {
return;
};
last_panic = Some(panic);
delays.increase_delays();
}

std::panic::resume_unwind(last_panic.unwrap());
}
)*
}
}

gen_watcher_test!(
(sanity, sanity_fn),
(update_moves_watcher, update_moves_watcher_fn),
(slowly_swap_file, slowly_swap_file_fn),
);

fn sanity_fn(
TestEnv {
main_file,
callback_rx,
..
}: TestEnv,
delays: Delays,
) {
// Sanity check watching
touch(&main_file);
delays.assert_at_least_one_message(&callback_rx);
}

fn update_moves_watcher_fn(
TestEnv {
main_file,
rel_file,
watcher,
callback_rx,
..
}: TestEnv,
delays: Delays,
) {
// Updating a file follows the new file and not the old one
watcher.update_file(&rel_file, fs::read_to_string(&rel_file).unwrap());
delays.assert_at_least_one_message(&callback_rx);
touch(&main_file);
delays.assert_no_message(&callback_rx);
touch(&rel_file);
delays.assert_at_least_one_message(&callback_rx);
}

fn slowly_swap_file_fn(
TestEnv {
temp_dir,
callback_rx,
main_file,
..
}: TestEnv,
delays: Delays,
) {
let swapped_in_file = temp_dir.path().join("swap_me_in.md");
let swapped_out_file = temp_dir.path().join("swap_out_to_me.md");
fs::write(&swapped_in_file, "# Swapped").unwrap();

// We can slowly swap out the file and it will only follow the file it's supposed to
fs::rename(&rel_file, &swapped_out_file).unwrap();
fs::rename(&main_file, &swapped_out_file).unwrap();
touch(&swapped_out_file);
delays.assert_no_message(&callback_rx);
// The "slowly" part of this (give the watcher time to fail and start polling)
delays.delay();
fs::rename(&swapped_in_file, &rel_file).unwrap();
fs::rename(&swapped_in_file, &main_file).unwrap();
delays.assert_at_least_one_message(&callback_rx);
fs::remove_file(&swapped_out_file).unwrap();
delays.assert_no_message(&callback_rx);
touch(&rel_file);
touch(&main_file);
delays.assert_at_least_one_message(&callback_rx);
}
48 changes: 22 additions & 26 deletions src/image/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::utils::usize_in_mib;
use image::codecs::{
gif::GifDecoder, jpeg::JpegDecoder, png::PngDecoder, tiff::TiffDecoder, webp::WebPDecoder,
};
use image::{ColorType, GenericImageView, ImageDecoder, ImageFormat};
use image::{ColorType, GenericImageView, ImageDecoder, ImageFormat, ImageResult};
use lz4_flex::frame::{BlockSize, FrameDecoder, FrameEncoder, FrameInfo};

pub fn lz4_compress<R: io::Read>(reader: &mut R) -> anyhow::Result<Vec<u8>> {
Expand All @@ -30,30 +30,17 @@ pub fn lz4_decompress(blob: &[u8], size: usize) -> anyhow::Result<Vec<u8>> {
Ok(decompressed)
}

pub fn decode_and_compress(contents: &[u8]) -> anyhow::Result<(Vec<u8>, (u32, u32))> {
pub type ImageParts = (Vec<u8>, (u32, u32));

pub fn decode_and_compress(contents: &[u8]) -> anyhow::Result<ImageParts> {
// We can stream decoding some formats although decoding may still load everything into memory
// at once depending on how the decoder behaves
let maybe_streamed = match image::guess_format(contents)? {
ImageFormat::Png => {
let dec = PngDecoder::new(io::Cursor::new(&contents))?;
stream_decode_and_compress(dec)
}
ImageFormat::Jpeg => {
let dec = JpegDecoder::new(io::Cursor::new(&contents))?;
stream_decode_and_compress(dec)
}
ImageFormat::Gif => {
let dec = GifDecoder::new(io::Cursor::new(&contents))?;
stream_decode_and_compress(dec)
}
ImageFormat::Tiff => {
let dec = TiffDecoder::new(io::Cursor::new(&contents))?;
stream_decode_and_compress(dec)
}
ImageFormat::WebP => {
let dec = WebPDecoder::new(io::Cursor::new(&contents))?;
stream_decode_and_compress(dec)
}
ImageFormat::Png => stream_decode_and_compress(contents, PngDecoder::new)?,
ImageFormat::Jpeg => stream_decode_and_compress(contents, JpegDecoder::new)?,
ImageFormat::Gif => stream_decode_and_compress(contents, GifDecoder::new)?,
ImageFormat::Tiff => stream_decode_and_compress(contents, TiffDecoder::new)?,
ImageFormat::WebP => stream_decode_and_compress(contents, WebPDecoder::new)?,
_ => None,
};

Expand All @@ -63,16 +50,24 @@ pub fn decode_and_compress(contents: &[u8]) -> anyhow::Result<(Vec<u8>, (u32, u3
}
}

fn stream_decode_and_compress<'img, Dec>(dec: Dec) -> Option<(Vec<u8>, (u32, u32))>
fn stream_decode_and_compress<'img, Dec>(
contents: &'img [u8],
decoder_constructor: fn(io::Cursor<&'img [u8]>) -> ImageResult<Dec>,
) -> anyhow::Result<Option<ImageParts>>
where
Dec: ImageDecoder<'img>,
{
let dec = decoder_constructor(io::Cursor::new(contents))?;

let total_size = dec.total_bytes();
let dimensions = dec.dimensions();
let start = Instant::now();

let mut adapter = Rgba8Adapter::new(dec)?;
lz4_compress(&mut adapter).ok().map(|lz4_blob| {
let Some(mut adapter) = Rgba8Adapter::new(dec) else {
return Ok(None);
};

let maybe_image_parts = lz4_compress(&mut adapter).ok().map(|lz4_blob| {
log::debug!(
"Streaming image decode & compression:\n\
- Full {:.2} MiB\n\
Expand All @@ -84,7 +79,8 @@ where
);

(lz4_blob, dimensions)
})
});
Ok(maybe_image_parts)
}

/// An adapter that can do a streaming transformation from some pixel formats to RGBA8
Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// I don't really care enough about the names here to fix things atm
clippy::enum_variant_names,
)]
#![deny(
#![warn(
// Generally we don't want this sneaking into `main`
clippy::todo,
// This should be used very sparingly compared between logging and clap
Expand Down
9 changes: 7 additions & 2 deletions src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::collections::HashMap;
use std::io;
use std::sync::{Arc, Mutex};
use std::sync::{Arc, Mutex, OnceLock};

use crate::image::ImageData;

Expand All @@ -10,6 +10,7 @@ use comrak::{markdown_to_html_with_plugins, ComrakOptions};
use indexmap::IndexMap;
use serde::Deserialize;
use syntect::highlighting::{Theme as SyntectTheme, ThemeSet as SyntectThemeSet};
use syntect::parsing::SyntaxSet;
use winit::window::CursorIcon;

pub(crate) fn default<T: Default>() -> T {
Expand Down Expand Up @@ -140,7 +141,11 @@ pub fn markdown_to_html(md: &str, syntax_theme: SyntectTheme) -> String {
theme_set
.themes
.insert(String::from(dummy_name), syntax_theme);
let syn_set = two_face::syntax::extra_no_newlines();
static CACHED_SYN_SET: OnceLock<SyntaxSet> = OnceLock::new();
// Initializing this is non-trivial. Cache so it only runs once
let syn_set = CACHED_SYN_SET
.get_or_init(two_face::syntax::extra_no_newlines)
.to_owned();
let adapter = SyntectAdapterBuilder::new()
.syntax_set(syn_set)
.theme_set(theme_set)
Expand Down