From 7c227f75a801292730b813405f8a2781d75c0e2a Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Thu, 27 Apr 2023 22:05:44 +0200 Subject: [PATCH] decoder: Cancel file loading on decoder drop That will probably at least work for scrolling quickly through images. It will however usually not cancel an ongoing decoding process. Issue #155 Part-of: --- src/decoder/formats/heif.rs | 19 +++++++++-- src/decoder/formats/image_rs_other.rs | 49 ++++++++++++--------------- src/decoder/formats/svg.rs | 24 ++++++------- src/decoder/mod.rs | 3 +- src/image_metadata/mod.rs | 3 +- src/util/mod.rs | 6 ++-- 6 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/decoder/formats/heif.rs b/src/decoder/formats/heif.rs index 7e31b80..4c07567 100644 --- a/src/decoder/formats/heif.rs +++ b/src/decoder/formats/heif.rs @@ -33,7 +33,15 @@ use rgb::AsPixels; use std::sync::Arc; #[derive(Debug)] -pub struct Heif; +pub struct Heif { + cancellable: gio::Cancellable, +} + +impl Drop for Heif { + fn drop(&mut self) { + self.cancellable.cancel(); + } +} /// Loads plugins on first use static LIBHEIF: Lazy = Lazy::new(LibHeif::new); @@ -45,6 +53,9 @@ impl Heif { tiles: Arc>, ) -> Self { log::debug!("Loading HEIF"); + let cancellable = gio::Cancellable::new(); + let cancellable_ = cancellable.clone(); + updater.spawn_error_handled(move || { let file_size = file .query_info( @@ -54,7 +65,7 @@ impl Heif { ) .context(gettext("Failed to read image file information"))? .size(); - let buf_reader = file.to_buf_read()?; + let buf_reader = file.to_buf_read(&cancellable)?; let stream_reader = libheif_rs::StreamReader::new(buf_reader, file_size as u64); let ctx = HeifContext::read_from_reader(Box::new(stream_reader)) .context(gettext("Failed to decode image"))?; @@ -132,7 +143,9 @@ impl Heif { Ok(()) }); - Heif + Heif { + cancellable: cancellable_, + } } } diff --git a/src/decoder/formats/image_rs_other.rs b/src/decoder/formats/image_rs_other.rs index 15a46e4..53ac93c 100644 --- a/src/decoder/formats/image_rs_other.rs +++ b/src/decoder/formats/image_rs_other.rs @@ -23,7 +23,7 @@ use crate::util; use crate::util::gettext::*; use crate::util::{BufReadSeek, ToBufRead}; -use anyhow::{bail, Context}; +use anyhow::Context; use arc_swap::ArcSwap; use gtk::prelude::*; use image_rs::{AnimationDecoder, ImageDecoder}; @@ -34,18 +34,13 @@ use std::sync::Arc; #[derive(Debug)] pub struct ImageRsOther { thread_handle: Option>, - abort: Arc>, + cancellable: gio::Cancellable, } impl Drop for ImageRsOther { fn drop(&mut self) { if let Some(handle) = &self.thread_handle { - if let Ok(mut abort) = self.abort.write() { - *abort = true; - } else { - // Don't use log in drop because not sure if always still initialized - eprintln!("Unable to write abort signal to decoding thread."); - } + self.cancellable.cancel(); handle.thread().unpark(); } } @@ -55,8 +50,9 @@ impl ImageRsOther { fn reader( file: &gio::File, format: image_rs::ImageFormat, + cancellable: &gio::Cancellable, ) -> anyhow::Result>> { - let buf_read = file.to_buf_read()?; + let buf_read = file.to_buf_read(cancellable)?; let mut reader = image_rs::io::Reader::new(buf_read); reader.set_format(format); Ok(reader) @@ -68,7 +64,7 @@ impl ImageRsOther { updater: UpdateSender, tiles: Arc>, ) -> Self { - let abort: Arc> = Default::default(); + let cancellable = gio::Cancellable::new(); let thread_handle = match format { image_rs::ImageFormat::Gif @@ -78,17 +74,17 @@ impl ImageRsOther { format, updater, tiles, - abort.clone(), + cancellable.clone(), )), _ => { - Self::new_static(file, format, updater, tiles); + Self::new_static(file, format, updater, tiles, cancellable.clone()); None } }; ImageRsOther { thread_handle, - abort, + cancellable, } } @@ -97,9 +93,10 @@ impl ImageRsOther { format: image_rs::ImageFormat, updater: UpdateSender, tiles: Arc>, + cancellable: gio::Cancellable, ) { updater.spawn_error_handled(move || { - let reader = Self::reader(&file, format)?; + let reader = Self::reader(&file, format, &cancellable)?; let dimensions = reader .into_dimensions() .context("Failed to read image dimensions")?; @@ -108,7 +105,7 @@ impl ImageRsOther { let (icc_profile, dynamic_image) = match format { image_rs::ImageFormat::Jpeg => { - let reader = file.to_buf_read()?; + let reader = file.to_buf_read(&cancellable)?; let mut decoder = image_rs::codecs::jpeg::JpegDecoder::new(reader)?; let _ = decoder.set_limits(image_rs::io::Limits::no_limits()); let icc_profile = decoder.icc_profile(); @@ -117,7 +114,7 @@ impl ImageRsOther { (icc_profile, dynamic_image) } image_rs::ImageFormat::Png => { - let reader = file.to_buf_read()?; + let reader = file.to_buf_read(&cancellable)?; let mut decoder = image_rs::codecs::png::PngDecoder::new(reader)?; let _ = decoder.set_limits(image_rs::io::Limits::no_limits()); let icc_profile = decoder.icc_profile(); @@ -126,7 +123,7 @@ impl ImageRsOther { (icc_profile, dynamic_image) } _ => { - let mut reader = Self::reader(&file, format)?; + let mut reader = Self::reader(&file, format, &cancellable)?; reader.limits(image_rs::io::Limits::no_limits()); let dynamic_image = reader.decode().context(gettext("Failed to decode image"))?; @@ -157,7 +154,7 @@ impl ImageRsOther { format: image_rs::ImageFormat, updater: UpdateSender, tiles: Arc>, - abort: Arc>, + cancellable: gio::Cancellable, ) -> std::thread::JoinHandle<()> { updater.clone().spawn_error_handled(move || { let mut nth_frame = 1; @@ -165,7 +162,7 @@ impl ImageRsOther { // We are currently decoding for each repetition of the animation // TODO: Check if/how that can be solved differently loop { - let buf_reader = file.to_buf_read()?; + let buf_reader = file.to_buf_read(&cancellable)?; let (frames, animated_format) = match format { image_rs::ImageFormat::Gif => ( @@ -178,7 +175,7 @@ impl ImageRsOther { (decoder.into_frames(), ImageFormat::AnimatedWebP) } else { // Static WebP images need a different decoder - Self::new_static(file, format, updater, tiles); + Self::new_static(file, format, updater, tiles, cancellable); return Ok(()); } } @@ -188,7 +185,7 @@ impl ImageRsOther { (decoder.apng().into_frames(), ImageFormat::AnimatedPng) } else { // Static PNG images need a different decoder - Self::new_static(file, format, updater, tiles); + Self::new_static(file, format, updater, tiles, cancellable); return Ok(()); } } @@ -247,13 +244,9 @@ impl ImageRsOther { std::thread::park(); } - if let Ok(abort_state) = abort.read().as_deref() { - if *abort_state { - log::debug!("Terminating decoder thread."); - return Ok(()); - } - } else { - bail!("Cannot read decoder thread abort instructions."); + if cancellable.is_cancelled() { + log::debug!("Terminating decoder thread."); + return Ok(()); } } } diff --git a/src/decoder/formats/svg.rs b/src/decoder/formats/svg.rs index 3d67a95..0841016 100644 --- a/src/decoder/formats/svg.rs +++ b/src/decoder/formats/svg.rs @@ -35,13 +35,13 @@ pub const RSVG_MAX_SIZE: u32 = 32_767; pub struct Svg { thread_handle: std::thread::JoinHandle<()>, current_request: Arc>, + cancellable: gio::Cancellable, } #[derive(Default, Debug, Clone)] enum Request { #[default] None, - Exit, Tile(TileRequest), } @@ -53,12 +53,7 @@ impl Request { impl Drop for Svg { fn drop(&mut self) { - if let Ok(mut request) = self.current_request.write() { - *request = Request::Exit; - } else { - // Don't use log in drop because not sure if always still initialized - eprintln!("Unable to write exit request: RwLock is poisoned."); - } + self.cancellable.cancel(); self.thread_handle.thread().unpark(); } } @@ -71,9 +66,11 @@ impl Svg { ) -> Self { let current_request: Arc> = Default::default(); let request_store = current_request.clone(); + let cancellable = gio::Cancellable::new(); + let cancellable_ = cancellable.clone(); let thread_handle = updater.spawn_error_handled(move || { - let handle = rsvg::Loader::new().read_file(&file, gio::Cancellable::NONE)?; + let handle = rsvg::Loader::new().read_file(&file, Some(&cancellable))?; let renderer = rsvg::CairoRenderer::new(&handle); let (original_width, original_height) = svg_dimensions(&renderer); @@ -92,14 +89,16 @@ impl Svg { value }; + if cancellable.is_cancelled() { + log::debug!("Terminating SVG decoder thread"); + break; + } + match tile_request { Request::None => { std::thread::park(); } - Request::Exit => { - log::debug!("Terminating decoder thread."); - break; - } + Request::Tile(tile_request) => { let tiling = tiles .get_layer_tiling_or_default(tile_request.zoom, tile_request.viewport); @@ -172,6 +171,7 @@ impl Svg { Self { thread_handle, current_request, + cancellable: cancellable_, } } diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 8f4be13..e1088f3 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -217,8 +217,9 @@ impl Decoder { fn guess_format(file: &gio::File) -> anyhow::Result> { let mut buf = Vec::new(); + // TODO: allow to cancel let fs_file = file - .to_buf_read() + .to_buf_read(&gio::Cancellable::new()) .context(gettext("Could not open image"))?; fs_file .take(64) diff --git a/src/image_metadata/mod.rs b/src/image_metadata/mod.rs index 1141935..51e2d00 100644 --- a/src/image_metadata/mod.rs +++ b/src/image_metadata/mod.rs @@ -63,7 +63,8 @@ impl std::fmt::Debug for ImageMetadata { impl ImageMetadata { pub fn load(file: &gio::File) -> Self { log::debug!("Loading metadata for {}", file.uri()); - if let Ok(mut bufreader) = file.to_buf_read() { + // TODO: make possible to cancel + if let Ok(mut bufreader) = file.to_buf_read(&gio::Cancellable::new()) { let exifreader = exif::Reader::new(); if let Ok(exif) = exifreader.read_from_container(&mut bufreader) { diff --git a/src/util/mod.rs b/src/util/mod.rs index ca6db2a..c41bd1c 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -182,13 +182,13 @@ pub trait BufReadSeek: std::io::BufRead + std::io::Seek {} impl BufReadSeek for std::io::BufReader> {} pub trait ToBufRead { - fn to_buf_read(&self) -> anyhow::Result>; + fn to_buf_read(&self, cancellable: &gio::Cancellable) -> anyhow::Result>; } impl ToBufRead for gio::File { - fn to_buf_read(&self) -> anyhow::Result> { + fn to_buf_read(&self, cancellable: &gio::Cancellable) -> anyhow::Result> { let read = self - .read(gio::Cancellable::NONE) + .read(Some(cancellable)) .context(gettext("Failed to open image"))? .into_read();