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: <https://gitlab.gnome.org/Incubator/loupe/-/merge_requests/210>
This commit is contained in:
Sophie Herold 2023-04-27 22:05:44 +02:00 • committed by Marge Bot
parent 6d7623b15a
commit 7c227f75a8
6 changed files with 56 additions and 48 deletions

View file

@ -33,7 +33,15 @@ use rgb::AsPixels;
use std::sync::Arc; use std::sync::Arc;
#[derive(Debug)] #[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 /// Loads plugins on first use
static LIBHEIF: Lazy<LibHeif> = Lazy::new(LibHeif::new); static LIBHEIF: Lazy<LibHeif> = Lazy::new(LibHeif::new);
@ -45,6 +53,9 @@ impl Heif {
tiles: Arc<ArcSwap<tiling::FrameBuffer>>, tiles: Arc<ArcSwap<tiling::FrameBuffer>>,
) -> Self { ) -> Self {
log::debug!("Loading HEIF"); log::debug!("Loading HEIF");
let cancellable = gio::Cancellable::new();
let cancellable_ = cancellable.clone();
updater.spawn_error_handled(move || { updater.spawn_error_handled(move || {
let file_size = file let file_size = file
.query_info( .query_info(
@ -54,7 +65,7 @@ impl Heif {
) )
.context(gettext("Failed to read image file information"))? .context(gettext("Failed to read image file information"))?
.size(); .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 stream_reader = libheif_rs::StreamReader::new(buf_reader, file_size as u64);
let ctx = HeifContext::read_from_reader(Box::new(stream_reader)) let ctx = HeifContext::read_from_reader(Box::new(stream_reader))
.context(gettext("Failed to decode image"))?; .context(gettext("Failed to decode image"))?;
@ -132,7 +143,9 @@ impl Heif {
Ok(()) Ok(())
}); });
Heif Heif {
cancellable: cancellable_,
}
} }
} }

View file

@ -23,7 +23,7 @@ use crate::util;
use crate::util::gettext::*; use crate::util::gettext::*;
use crate::util::{BufReadSeek, ToBufRead}; use crate::util::{BufReadSeek, ToBufRead};
use anyhow::{bail, Context}; use anyhow::Context;
use arc_swap::ArcSwap; use arc_swap::ArcSwap;
use gtk::prelude::*; use gtk::prelude::*;
use image_rs::{AnimationDecoder, ImageDecoder}; use image_rs::{AnimationDecoder, ImageDecoder};
@ -34,18 +34,13 @@ use std::sync::Arc;
#[derive(Debug)] #[derive(Debug)]
pub struct ImageRsOther { pub struct ImageRsOther {
thread_handle: Option<std::thread::JoinHandle<()>>, thread_handle: Option<std::thread::JoinHandle<()>>,
abort: Arc<std::sync::RwLock<bool>>, cancellable: gio::Cancellable,
} }
impl Drop for ImageRsOther { impl Drop for ImageRsOther {
fn drop(&mut self) { fn drop(&mut self) {
if let Some(handle) = &self.thread_handle { if let Some(handle) = &self.thread_handle {
if let Ok(mut abort) = self.abort.write() { self.cancellable.cancel();
*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.");
}
handle.thread().unpark(); handle.thread().unpark();
} }
} }
@ -55,8 +50,9 @@ impl ImageRsOther {
fn reader( fn reader(
file: &gio::File, file: &gio::File,
format: image_rs::ImageFormat, format: image_rs::ImageFormat,
cancellable: &gio::Cancellable,
) -> anyhow::Result<image_rs::io::Reader<Box<dyn BufReadSeek>>> { ) -> anyhow::Result<image_rs::io::Reader<Box<dyn BufReadSeek>>> {
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); let mut reader = image_rs::io::Reader::new(buf_read);
reader.set_format(format); reader.set_format(format);
Ok(reader) Ok(reader)
@ -68,7 +64,7 @@ impl ImageRsOther {
updater: UpdateSender, updater: UpdateSender,
tiles: Arc<ArcSwap<tiling::FrameBuffer>>, tiles: Arc<ArcSwap<tiling::FrameBuffer>>,
) -> Self { ) -> Self {
let abort: Arc<std::sync::RwLock<bool>> = Default::default(); let cancellable = gio::Cancellable::new();
let thread_handle = match format { let thread_handle = match format {
image_rs::ImageFormat::Gif image_rs::ImageFormat::Gif
@ -78,17 +74,17 @@ impl ImageRsOther {
format, format,
updater, updater,
tiles, tiles,
abort.clone(), cancellable.clone(),
)), )),
_ => { _ => {
Self::new_static(file, format, updater, tiles); Self::new_static(file, format, updater, tiles, cancellable.clone());
None None
} }
}; };
ImageRsOther { ImageRsOther {
thread_handle, thread_handle,
abort, cancellable,
} }
} }
@ -97,9 +93,10 @@ impl ImageRsOther {
format: image_rs::ImageFormat, format: image_rs::ImageFormat,
updater: UpdateSender, updater: UpdateSender,
tiles: Arc<ArcSwap<tiling::FrameBuffer>>, tiles: Arc<ArcSwap<tiling::FrameBuffer>>,
cancellable: gio::Cancellable,
) { ) {
updater.spawn_error_handled(move || { updater.spawn_error_handled(move || {
let reader = Self::reader(&file, format)?; let reader = Self::reader(&file, format, &cancellable)?;
let dimensions = reader let dimensions = reader
.into_dimensions() .into_dimensions()
.context("Failed to read image dimensions")?; .context("Failed to read image dimensions")?;
@ -108,7 +105,7 @@ impl ImageRsOther {
let (icc_profile, dynamic_image) = match format { let (icc_profile, dynamic_image) = match format {
image_rs::ImageFormat::Jpeg => { 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 mut decoder = image_rs::codecs::jpeg::JpegDecoder::new(reader)?;
let _ = decoder.set_limits(image_rs::io::Limits::no_limits()); let _ = decoder.set_limits(image_rs::io::Limits::no_limits());
let icc_profile = decoder.icc_profile(); let icc_profile = decoder.icc_profile();
@ -117,7 +114,7 @@ impl ImageRsOther {
(icc_profile, dynamic_image) (icc_profile, dynamic_image)
} }
image_rs::ImageFormat::Png => { 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 mut decoder = image_rs::codecs::png::PngDecoder::new(reader)?;
let _ = decoder.set_limits(image_rs::io::Limits::no_limits()); let _ = decoder.set_limits(image_rs::io::Limits::no_limits());
let icc_profile = decoder.icc_profile(); let icc_profile = decoder.icc_profile();
@ -126,7 +123,7 @@ impl ImageRsOther {
(icc_profile, dynamic_image) (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()); reader.limits(image_rs::io::Limits::no_limits());
let dynamic_image = let dynamic_image =
reader.decode().context(gettext("Failed to decode image"))?; reader.decode().context(gettext("Failed to decode image"))?;
@ -157,7 +154,7 @@ impl ImageRsOther {
format: image_rs::ImageFormat, format: image_rs::ImageFormat,
updater: UpdateSender, updater: UpdateSender,
tiles: Arc<ArcSwap<tiling::FrameBuffer>>, tiles: Arc<ArcSwap<tiling::FrameBuffer>>,
abort: Arc<std::sync::RwLock<bool>>, cancellable: gio::Cancellable,
) -> std::thread::JoinHandle<()> { ) -> std::thread::JoinHandle<()> {
updater.clone().spawn_error_handled(move || { updater.clone().spawn_error_handled(move || {
let mut nth_frame = 1; let mut nth_frame = 1;
@ -165,7 +162,7 @@ impl ImageRsOther {
// We are currently decoding for each repetition of the animation // We are currently decoding for each repetition of the animation
// TODO: Check if/how that can be solved differently // TODO: Check if/how that can be solved differently
loop { loop {
let buf_reader = file.to_buf_read()?; let buf_reader = file.to_buf_read(&cancellable)?;
let (frames, animated_format) = match format { let (frames, animated_format) = match format {
image_rs::ImageFormat::Gif => ( image_rs::ImageFormat::Gif => (
@ -178,7 +175,7 @@ impl ImageRsOther {
(decoder.into_frames(), ImageFormat::AnimatedWebP) (decoder.into_frames(), ImageFormat::AnimatedWebP)
} else { } else {
// Static WebP images need a different decoder // Static WebP images need a different decoder
Self::new_static(file, format, updater, tiles); Self::new_static(file, format, updater, tiles, cancellable);
return Ok(()); return Ok(());
} }
} }
@ -188,7 +185,7 @@ impl ImageRsOther {
(decoder.apng().into_frames(), ImageFormat::AnimatedPng) (decoder.apng().into_frames(), ImageFormat::AnimatedPng)
} else { } else {
// Static PNG images need a different decoder // Static PNG images need a different decoder
Self::new_static(file, format, updater, tiles); Self::new_static(file, format, updater, tiles, cancellable);
return Ok(()); return Ok(());
} }
} }
@ -247,13 +244,9 @@ impl ImageRsOther {
std::thread::park(); std::thread::park();
} }
if let Ok(abort_state) = abort.read().as_deref() { if cancellable.is_cancelled() {
if *abort_state { log::debug!("Terminating decoder thread.");
log::debug!("Terminating decoder thread."); return Ok(());
return Ok(());
}
} else {
bail!("Cannot read decoder thread abort instructions.");
} }
} }
} }

View file

@ -35,13 +35,13 @@ pub const RSVG_MAX_SIZE: u32 = 32_767;
pub struct Svg { pub struct Svg {
thread_handle: std::thread::JoinHandle<()>, thread_handle: std::thread::JoinHandle<()>,
current_request: Arc<std::sync::RwLock<Request>>, current_request: Arc<std::sync::RwLock<Request>>,
cancellable: gio::Cancellable,
} }
#[derive(Default, Debug, Clone)] #[derive(Default, Debug, Clone)]
enum Request { enum Request {
#[default] #[default]
None, None,
Exit,
Tile(TileRequest), Tile(TileRequest),
} }
@ -53,12 +53,7 @@ impl Request {
impl Drop for Svg { impl Drop for Svg {
fn drop(&mut self) { fn drop(&mut self) {
if let Ok(mut request) = self.current_request.write() { self.cancellable.cancel();
*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.thread_handle.thread().unpark(); self.thread_handle.thread().unpark();
} }
} }
@ -71,9 +66,11 @@ impl Svg {
) -> Self { ) -> Self {
let current_request: Arc<std::sync::RwLock<Request>> = Default::default(); let current_request: Arc<std::sync::RwLock<Request>> = Default::default();
let request_store = current_request.clone(); let request_store = current_request.clone();
let cancellable = gio::Cancellable::new();
let cancellable_ = cancellable.clone();
let thread_handle = updater.spawn_error_handled(move || { 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 renderer = rsvg::CairoRenderer::new(&handle);
let (original_width, original_height) = svg_dimensions(&renderer); let (original_width, original_height) = svg_dimensions(&renderer);
@ -92,14 +89,16 @@ impl Svg {
value value
}; };
if cancellable.is_cancelled() {
log::debug!("Terminating SVG decoder thread");
break;
}
match tile_request { match tile_request {
Request::None => { Request::None => {
std::thread::park(); std::thread::park();
} }
Request::Exit => {
log::debug!("Terminating decoder thread.");
break;
}
Request::Tile(tile_request) => { Request::Tile(tile_request) => {
let tiling = tiles let tiling = tiles
.get_layer_tiling_or_default(tile_request.zoom, tile_request.viewport); .get_layer_tiling_or_default(tile_request.zoom, tile_request.viewport);
@ -172,6 +171,7 @@ impl Svg {
Self { Self {
thread_handle, thread_handle,
current_request, current_request,
cancellable: cancellable_,
} }
} }

View file

@ -217,8 +217,9 @@ impl Decoder {
fn guess_format(file: &gio::File) -> anyhow::Result<Option<image_rs::ImageFormat>> { fn guess_format(file: &gio::File) -> anyhow::Result<Option<image_rs::ImageFormat>> {
let mut buf = Vec::new(); let mut buf = Vec::new();
// TODO: allow to cancel
let fs_file = file let fs_file = file
.to_buf_read() .to_buf_read(&gio::Cancellable::new())
.context(gettext("Could not open image"))?; .context(gettext("Could not open image"))?;
fs_file fs_file
.take(64) .take(64)

View file

@ -63,7 +63,8 @@ impl std::fmt::Debug for ImageMetadata {
impl ImageMetadata { impl ImageMetadata {
pub fn load(file: &gio::File) -> Self { pub fn load(file: &gio::File) -> Self {
log::debug!("Loading metadata for {}", file.uri()); 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(); let exifreader = exif::Reader::new();
if let Ok(exif) = exifreader.read_from_container(&mut bufreader) { if let Ok(exif) = exifreader.read_from_container(&mut bufreader) {

View file

@ -182,13 +182,13 @@ pub trait BufReadSeek: std::io::BufRead + std::io::Seek {}
impl BufReadSeek for std::io::BufReader<gio::InputStreamRead<gio::FileInputStream>> {} impl BufReadSeek for std::io::BufReader<gio::InputStreamRead<gio::FileInputStream>> {}
pub trait ToBufRead { pub trait ToBufRead {
fn to_buf_read(&self) -> anyhow::Result<Box<dyn BufReadSeek>>; fn to_buf_read(&self, cancellable: &gio::Cancellable) -> anyhow::Result<Box<dyn BufReadSeek>>;
} }
impl ToBufRead for gio::File { impl ToBufRead for gio::File {
fn to_buf_read(&self) -> anyhow::Result<Box<dyn BufReadSeek>> { fn to_buf_read(&self, cancellable: &gio::Cancellable) -> anyhow::Result<Box<dyn BufReadSeek>> {
let read = self let read = self
.read(gio::Cancellable::NONE) .read(Some(cancellable))
.context(gettext("Failed to open image"))? .context(gettext("Failed to open image"))?
.into_read(); .into_read();