Fix a bunch of defects

CMakeLists.txt

@@ -162,6 +162,7 @@ if (APPLE)
 endif()
 
 include(FetchContent)
+include(CTest)
 
 # The GPL fast float plugin improves performance a ton,
 # but distributions usually omit it.
@@ -199,6 +200,7 @@ else()
 	set(LCMS2_BUILD_TOOLS OFF CACHE BOOL "" FORCE)
 	set(LCMS2_WITH_FASTFLOAT ON CACHE BOOL "" FORCE)
 	set(LCMS2_WITH_THREADED_PLUGIN ON CACHE BOOL "" FORCE)
+	set(LCMS2_BUILD_TESTS "${BUILD_TESTING}" CACHE BOOL "Build tests (testbed)")
 
 	# Bundled static lib; do not ship headers/.a with the app package.
 	FetchContent_Declare(lcms2
@@ -225,7 +227,9 @@ else()
 		lcms2 lcms2_fast_float lcms2_threaded)
 
 	# Have CTest not fail.
-	set_target_properties(testcms PROPERTIES EXCLUDE_FROM_ALL FALSE)
+	if (TARGET testcms)
+		set_target_properties(testcms PROPERTIES EXCLUDE_FROM_ALL FALSE)
+	endif()
 endif()
 
 option(DAWN_WITH_JPEG_QS "Bundled JPEG Quant Smooth" ON)
@@ -296,7 +300,6 @@ configure_file(
 	"${PROJECT_BINARY_DIR}/dawn-config.h")
 
 include_directories("${PROJECT_SOURCE_DIR}")
-include(CTest)
 
 # Keep diagnostics after bundled dependencies so that they don't apply to them.
 if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")

dn/action.cpp

@@ -5,6 +5,8 @@
 // SPDX-License-Identifier: MPL-2.0
 //
 
+#include <dawn-config.h>
+
 #include <libdn/gettext.hpp>
 
 #include "action.hpp"
@@ -626,11 +628,11 @@ static constexpr ModeDef kModes[] = {
 	// TRANSLATORS: The application's name, in window titles.  Transliterate
 	// it if that is what your script does with foreign names; do not
 	// translate the word.
-	{"view", N_("Dawn"), kViewerMenu, kViewerKeys},
-	{"browse", N_("Dawn"), kBrowserMenu, kBrowserKeys},
-	{"cropjpeg", N_("Dawn Lossless JPEG Cropper"), kCropJpegMenu,
-		kCropJpegKeys},
-	{"commander", N_("Dawn Commander"), kCommanderMenu, {}},
+	{"view", N_("Dawn"), kViewerMenu, kViewerKeys, true},
+	{"browse", N_("Dawn"), kBrowserMenu, kBrowserKeys, true},
+	{"cropjpeg", N_("Dawn Lossless JPEG Cropper"), kCropJpegMenu, kCropJpegKeys,
+		true},
+	{"commander", N_("Dawn Commander"), kCommanderMenu, {}, DAWN_WITH_WIP},
 };
 static_assert(size(kModes) == size_t(Mode::Count));
 

dn/action.hpp

@@ -160,6 +160,8 @@ struct ModeDef {
 	const char *title;
 	std::span<const MenuNode> menu;
 	std::span<const Action> keys;
+	// Whether this build may open it; unfinished ones are parsed regardless.
+	bool available;
 };
 
 std::span<const ModeDef> modes();

dn/assoc-macos.mm

@@ -91,7 +91,7 @@ app_url_for_bundle_id(NSString *bid)
 	NSURL *url = nil;
 	if (CFArrayGetCount(urls) > 0)
 		url = (__bridge NSURL *) CFArrayGetValueAtIndex(urls, 0);
-	NSURL *copy = url ? [url copy] : nil;
+	NSURL *copy = url ? [[url copy] autorelease] : nil;
 	CFRelease(urls);
 	return copy;
 }

dn/instance.cpp

@@ -157,6 +157,10 @@ InstanceHost::Impl::on_request(
 			"unknown mode or empty open request");
 		return;
 	}
+	if (!mode_def(*mode).available) {
+		call.fail(dawn::ipc::ErrorCode::Unsupported, "unsupported mode");
+		return;
+	}
 	for (const string_view url : open_body->open.urls) {
 		const QByteArray encoded(url.data(), qsizetype(url.size()));
 		if (!this->app_.open(QUrl::fromEncoded(encoded), token, {}, *mode)) {

dn/kit-browser.cpp

@@ -2499,6 +2499,10 @@ Browser::set_files(vector<File> files)
 	for (int i = 0; i < int(this->files_.size()); i++)
 		this->file_by_path_.emplace(this->files_[size_t(i)].path, i);
 	this->file_rev_++;
+
+	// Indexes into the old listing would now name other files.
+	this->press_file_ = -1;
+	this->mid_file_ = -1;
 }
 
 int
@@ -2859,7 +2863,6 @@ Browser::press(Kit &kit, float x, float y, Qt::MouseButton button)
 
 	// A press that travels far enough drags the file out rather than
 	// opening it; until then it is still an ordinary click.
-	// XXX: We don't reset this when indexes change meaning.
 	this->press_file_ = (kit.touch_press_ || !kit.start_drag) ? -1 : i;
 	this->press_x_ = x;
 	this->press_y_ = y;

dn/kit-chrome.cpp

@@ -924,6 +924,7 @@ Hint::collect(Widget *scope)
 		Target t;
 		t.browser = browser;
 		t.file_i = i;
+		t.file_rev = browser->file_rev_;
 		t.at = clipped;
 		this->targets_.push_back(t);
 	}
@@ -952,8 +953,7 @@ Hint::refresh_rects()
 			keep.push_back(t);
 			continue;
 		}
-		if (!t.browser || t.file_i < 0 ||
-			t.file_i >= int(t.browser->files_.size()))
+		if (!t.browser || t.file_rev != t.browser->file_rev_)
 			continue;
 		const Browser::File &f = t.browser->files_[size_t(t.file_i)];
 		const Rect clipped = f.tile.intersect(t.browser->r);
@@ -989,6 +989,7 @@ Hint::fire(Kit &kit, Target t)
 	Widget *widget = t.widget;
 	Browser *browser = t.browser;
 	const int file_i = t.file_i;
+	const uint64_t file_rev = t.file_rev;
 	close(kit);
 	if (widget) {
 		// Whatever this widget calls its default action; one that has none
@@ -998,7 +999,7 @@ Hint::fire(Kit &kit, Target t)
 		kit.activate(widget);
 		return;
 	}
-	if (!browser || file_i < 0 || file_i >= int(browser->files_.size()))
+	if (!browser || file_rev != browser->file_rev_)
 		return;
 
 	browser->activate_file(browser->file_url(file_i));

dn/kit-chrome.hpp

@@ -146,6 +146,8 @@ private:
 		// TODO(p): Unwanted dependency.
 		Browser *browser = nullptr;
 		int file_i = -1;
+		// The listing file_i indexes; a rescan retires the target.
+		uint64_t file_rev = 0;
 	};
 
 	std::vector<Target> targets_;

dn/main.cpp

@@ -189,7 +189,8 @@ main(int argc, char **argv)
 	// These are stable command-line spellings, so they are never translated.
 	QStringList mode_names;
 	for (const dn::ModeDef &mode : dn::modes())
-		mode_names += QLatin1String(mode.name);
+		if (mode.available)
+			mode_names += QLatin1String(mode.name);
 
 	const QCommandLineOption mode_opt(QStringLiteral("mode"),
 		// TRANSLATORS: %1 is a comma-separated list of mode names.
@@ -248,14 +249,12 @@ main(int argc, char **argv)
 		}
 		mode = *parsed;
 	}
-#if !DAWN_WITH_WIP
-	if (mode == dn::Mode::Commander) {
+	if (!dn::mode_def(mode).available) {
 		qWarning("%s",
 			qUtf8Printable(QString::fromUtf8(_("unsupported mode: %1"))
 					.arg(QLatin1String(dn::mode_def(mode).name))));
 		return EXIT_FAILURE;
 	}
-#endif
 
 	dn::App app(argc, argv);
 	dn::accessible_init();

dn/thumbnailer.cpp

@@ -35,8 +35,8 @@ constexpr size_t kPriorityCount = 4;
 constexpr size_t kGuiBatch = 32;
 
 // A worker may queue GPU work before returning the GUI completion which
-// records that work in its client. Hold such GPU callbacks until the CPU
-// completion has at least been placed on the GUI queue.
+// records that work in its client. Hold such GPU callbacks until pump() has
+// taken that completion off the GUI queue, or the worker has found it has none.
 static thread_local shared_ptr<atomic_bool> current_cpu_gate;
 
 static bool
@@ -99,6 +99,7 @@ struct GuiTask {
 	uint64_t epoch = 0;
 	Thumbnailer::Priority priority = Thumbnailer::Priority::Maintenance;
 	Thumbnailer::Completion completion;
+	shared_ptr<atomic_bool> gate;
 };
 
 struct GpuTask {
@@ -108,6 +109,7 @@ struct GpuTask {
 	string key;
 	Thumbnailer::GpuCompletion completion;
 	shared_ptr<atomic_bool> gate;
+	shared_ptr<dawn::ThumbScaler::Control> control;
 	optional<dawn::ThumbScaler::Result> result;
 };
 
@@ -300,10 +302,11 @@ Thumbnailer::Impl::worker_loop()
 				client->second.epoch == task->epoch) {
 				client->second.gui++;
 				gui.push_back({task->client, task->epoch,
-					task->running_priority, std::move(completion)});
+					task->running_priority, std::move(completion), task->gate});
+			} else {
+				task->gate->store(true, memory_order_release);
 			}
 		}
-		task->gate->store(true, memory_order_release);
 		cv.notify_all();
 		owner->schedule_pump();
 	}
@@ -390,6 +393,7 @@ Thumbnailer::Impl::erase_gpu(Client id, ClientState &state)
 {
 	for (auto it = gpu_tasks.begin(); it != gpu_tasks.end();) {
 		if (it->second.client == id) {
+			it->second.control->canceled = true;
 			if (scaler)
 				scaler->cancel(it->first);
 			state.gpu--;
@@ -580,6 +584,7 @@ Thumbnailer::reprioritize(
 		found_any = true;
 		if (priority != task.priority) {
 			task.priority = priority;
+			task.control->priority = scaler_priority(priority);
 			if (!task.result)
 				gpu.push_back(gpu_id);
 		}
@@ -621,6 +626,7 @@ Thumbnailer::cancel(Client id, const string &key)
 				it++;
 				continue;
 			}
+			it->second.control->canceled = true;
 			if (impl_->scaler)
 				impl_->scaler->cancel(it->first);
 			client->second.gpu--;
@@ -656,6 +662,9 @@ Thumbnailer::submit_gpu(Client id, uint64_t epoch, Priority priority,
 		gpu_id = impl_->next_gpu++;
 		if (!gpu_id)
 			gpu_id = impl_->next_gpu++;
+		auto control = make_shared<dawn::ThumbScaler::Control>();
+		control->priority = scaler_priority(priority);
+		job.control = control;
 		impl_->gpu_tasks.emplace(gpu_id,
 			GpuTask{
 				.client = id,
@@ -664,12 +673,12 @@ Thumbnailer::submit_gpu(Client id, uint64_t epoch, Priority priority,
 				.key = key,
 				.completion = std::move(completion),
 				.gate = current_cpu_gate,
+				.control = std::move(control),
 			});
 		client->second.gpu++;
 		client->second.activity_pending = true;
 	}
 	job.user = gpu_id;
-	job.priority = scaler_priority(priority);
 	// queue() waits for pump() to free ring space.  Workers may block;
 	// this thread may not.  Hand GUI callers a CPU task that does the copy.
 	if (QThread::currentThread() != thread()) {
@@ -886,6 +895,8 @@ Thumbnailer::pump()
 				break;
 			GuiTask task = std::move(impl_->gui.front());
 			impl_->gui.pop_front();
+			// Its GPU callbacks are only collected below, after it has run.
+			task.gate->store(true, memory_order_release);
 			auto client = impl_->clients.find(task.client);
 			if (client == impl_->clients.end())
 				continue;

libdn/cmm.cpp

@@ -21,6 +21,7 @@
 #include <algorithm>
 #include <cmath>
 #include <cstring>
+#include <exception>
 #include <optional>
 #include <thread>
 #include <vector>
@@ -98,8 +99,12 @@ transform_tiled(cmsContext ctx, cmsHPROFILE src_h, cmsUInt32Number src_fmt,
 		return true;
 	}
 
+	// Allocate everything that can throw before there is anything to release.
 	const unsigned n = cms_workers(height);
 	vector<cmsHTRANSFORM> xforms(n);
+	vector<thread> pool;
+	if (n > 1)
+		pool.reserve(n - 1);
 	for (unsigned i = 0; i < n; i++) {
 		xforms[i] = create();
 		if (!xforms[i]) {
@@ -109,15 +114,17 @@ transform_tiled(cmsContext ctx, cmsHPROFILE src_h, cmsUInt32Number src_fmt,
 		}
 	}
 
-	vector<thread> pool;
-	if (n > 1)
-		pool.reserve(n - 1);
 	for (unsigned i = 1; i < n; i++) {
 		const uint32_t y0 = uint32_t(uint64_t(height) * i / n);
 		const uint32_t y1 = uint32_t(uint64_t(height) * (i + 1) / n);
 		if (y0 >= y1)
 			continue;
-		pool.emplace_back([&, i, y0, y1] { run_band(xforms[i], y0, y1); });
+
+		try {
+			pool.emplace_back([&, i, y0, y1] { run_band(xforms[i], y0, y1); });
+		} catch (const exception &) {
+			run_band(xforms[i], y0, y1);
+		}
 	}
 	run_band(xforms[0], 0, uint32_t(uint64_t(height) / n));
 	for (thread &t : pool)

libdn/libdn.cpp

@@ -129,8 +129,9 @@ image_new(uint32_t width, uint32_t height)
 	if (height > SIZE_MAX / stride)
 		return nullptr;
 
-	auto image = make_shared<Image>();
+	ImagePtr image;
 	try {
+		image = make_shared<Image>();
 		StageClock clk(&OpenTiming::alloc_ms);
 		image->data.assign(size_t(stride) * height, 0);
 	} catch (const bad_alloc &) {

libdn/libdn.hpp

@@ -505,8 +505,10 @@ std::shared_ptr<Cmm> cmm_or_default(const OpenContext &ctx);
 void finish_image(
 	Image &image, const OpenContext &ctx, Profile *source, bool input_premul);
 
-/// finish_image() for a page and all its animation frames, which inherit
-/// the page's source profile when they carry none of their own.
+/// finish_image() for a page and all its animation frames, all converted
+/// from the page's source profile, whatever ICC the frames carry themselves.
+/// Frames that have no `effective_profile` take the page's.  A format whose
+/// frames may embed profiles of their own finishes them one by one instead.
 void finish_frames(
 	Image &page, const OpenContext &ctx, Profile *source, bool input_premul);
 

libdn/load-dnrs.cpp

@@ -308,11 +308,11 @@ load_dnrs(span<const uint8_t> data, const OpenContext &ctx, Error *error)
 			? Orientation(page_info.orientation)
 			: Orientation::Unknown;
 		frames->loops = page_info.loop_count;
-		copy_blob(frames->icc, document.icc);
-		copy_blob(frames->exif, document.exif);
-		copy_blob(frames->xmp, document.xmp);
-		for (size_t i = 0; i < document.text_length; i++) {
-			const dnrs_text &entry = document.text[i];
+		copy_blob(frames->icc, page_info.icc);
+		copy_blob(frames->exif, page_info.exif);
+		copy_blob(frames->xmp, page_info.xmp);
+		for (size_t i = 0; i < page_info.text_length; i++) {
+			const dnrs_text &entry = page_info.text[i];
 			if (entry.key && entry.value)
 				frames->text.emplace(entry.key, entry.value);
 		}

libdn/thumb-scaler.cpp

@@ -24,7 +24,6 @@
 #include <optional>
 #include <string>
 #include <unordered_map>
-#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -249,8 +248,6 @@ struct ThumbScaler::Impl {
 	vector<Range> free_ranges;
 	unordered_map<uint32_t, Range> live;
 	vector<Waiter *> waiters;
-	unordered_map<uint64_t, Priority> priority_overrides;
-	unordered_set<uint64_t> canceled;
 	vector<Pending> pending;
 	vector<Result> failed_results;
 	array<Batch, kBatchSlots> batches;
@@ -385,7 +382,6 @@ destroy_all(ThumbScaler::Impl &e)
 	e.free_ranges.clear();
 	e.live.clear();
 	e.waiters.clear();
-	e.priority_overrides.clear();
 	e.pending.clear();
 	e.failed_results.clear();
 	e.ready = false;
@@ -477,8 +473,22 @@ release_range(ThumbScaler::Impl &e, uint32_t id)
 }
 
 static bool
-claim(ThumbScaler::Impl &e, size_t bytes, uint64_t user,
-	ThumbScaler::Priority priority, Slot *slot)
+canceled(const ThumbScaler::Job &job)
+{
+	return job.control && job.control->canceled;
+}
+
+// Read under the lock that reprioritize() takes, so that whatever it misses
+// in the queues is seen here.
+static ThumbScaler::Priority
+current_priority(const ThumbScaler::Job &job)
+{
+	return job.control ? job.control->priority.load() : job.priority;
+}
+
+static bool
+claim(
+	ThumbScaler::Impl &e, size_t bytes, const ThumbScaler::Job &job, Slot *slot)
 {
 	if (!slot)
 		return false;
@@ -488,25 +498,19 @@ claim(ThumbScaler::Impl &e, size_t bytes, uint64_t user,
 		return false;
 
 	unique_lock lock(e.mu);
-	if (e.canceled.contains(user))
+	if (canceled(job))
 		return false;
 
-	if (auto found = e.priority_overrides.find(user);
-		found != e.priority_overrides.end())
-		priority = found->second;
-	Waiter waiter{user, priority, e.next_waiter++, bytes};
+	Waiter waiter{job.user, current_priority(job), e.next_waiter++, bytes};
 	if (!e.next_waiter)
 		e.next_waiter = 1;
 	e.waiters.push_back(&waiter);
 	while (!e.stop) {
-		if (e.canceled.contains(user)) {
+		if (canceled(job)) {
 			erase(e.waiters, &waiter);
 			e.cv.notify_all();
 			return false;
 		}
-		if (auto found = e.priority_overrides.find(user);
-			found != e.priority_overrides.end())
-			waiter.priority = found->second;
 		Waiter *first = nullptr;
 		for (Waiter *candidate : e.waiters)
 			if (!first || higher(candidate->priority, first->priority) ||
@@ -648,7 +652,8 @@ ensure_reduced(ThumbScaler::Impl &e, Session &s, string *error)
 }
 
 static bool
-begin_session(ThumbScaler::Impl &e, const SessionInfo &info, uint32_t *id)
+begin_session(ThumbScaler::Impl &e, const SessionInfo &info,
+	const ThumbScaler::Job &job, uint32_t *id)
 {
 	if (!e.ready || !id || !info.src_w || !info.src_h || info.outputs.empty() ||
 		!info.tile_count)
@@ -663,9 +668,7 @@ begin_session(ThumbScaler::Impl &e, const SessionInfo &info, uint32_t *id)
 		return false;
 
 	lock_guard lock(e.mu);
-	if (auto priority = e.priority_overrides.find(info.user);
-		priority != e.priority_overrides.end())
-		s->info.priority = priority->second;
+	s->info.priority = current_priority(job);
 	s->id = e.next_session++;
 	if (!e.next_session)
 		e.next_session = 1;
@@ -1070,13 +1073,12 @@ build_batch(ThumbScaler::Impl &e, Batch &b, vector<Pending> jobs,
 // --- Queueing ----------------------------------------------------------------
 
 static bool
-enqueue(ThumbScaler::Impl &e, const Request &req)
+enqueue(ThumbScaler::Impl &e, const Request &req, const ThumbScaler::Job &job)
 {
 	lock_guard lock(e.mu);
 	auto found = e.live.find(req.slot);
 	if (!e.ready || e.stop || found == e.live.end() || !req.src_w ||
-		!req.src_h || (!req.session && req.outputs.empty()) ||
-		e.canceled.contains(req.user)) {
+		!req.src_h || (!req.session && req.outputs.empty()) || canceled(job)) {
 		release_range(e, req.slot);
 		return false;
 	}
@@ -1089,9 +1091,7 @@ enqueue(ThumbScaler::Impl &e, const Request &req)
 		s->enqueued++;
 	}
 	Request queued = req;
-	if (auto priority = e.priority_overrides.find(req.user);
-		priority != e.priority_overrides.end())
-		queued.priority = priority->second;
+	queued.priority = current_priority(job);
 	e.pending.push_back({std::move(queued), found->second});
 	return true;
 }
@@ -1104,7 +1104,7 @@ queue_full(ThumbScaler::Impl &e, const ThumbScaler::Job &job)
 		return false;
 
 	Slot slot;
-	if (!claim(e, size_t(bytes), job.user, job.priority, &slot))
+	if (!claim(e, size_t(bytes), job, &slot))
 		return false;
 
 	const Image &image = *job.image;
@@ -1135,7 +1135,7 @@ queue_full(ThumbScaler::Impl &e, const ThumbScaler::Job &job)
 	req.user = job.user;
 	req.priority = job.priority;
 	req.path = job.path;
-	return enqueue(e, req);
+	return enqueue(e, req, job);
 }
 
 // --- Interface ---------------------------------------------------------------
@@ -1267,27 +1267,19 @@ ThumbScaler::queue(const Job &job)
 	if (!impl_ || !impl_->ready)
 		return false;
 	Impl &e = *impl_;
-	auto fail = [&] {
-		lock_guard lock(e.mu);
-		e.priority_overrides.erase(job.user);
-		return false;
-	};
 	uint64_t bytes = 0;
 	if (!job_size(job, nullptr, &bytes))
-		return fail();
+		return false;
 	const Image &image = *job.image;
 	if (bytes <= e.ring_bytes && image.width <= e.max_image_dim &&
-		image.height <= e.max_image_dim) {
-		if (queue_full(e, job))
-			return true;
-		return fail();
-	}
+		image.height <= e.max_image_dim)
+		return queue_full(e, job);
 
 	uint32_t k = 0;
 	vector<Tile> tiles;
 	if (!choose_k(e, image.width, image.height, &k) ||
 		!plan_tiles(e, image.width, image.height, k, &tiles) || tiles.empty())
-		return fail();
+		return false;
 
 	SessionInfo info;
 	info.path = job.path;
@@ -1303,13 +1295,13 @@ ThumbScaler::queue(const Job &job)
 	info.opaque = opaque_bgra16(
 		image.data.data(), image.width, image.height, image.stride);
 	uint32_t session = 0;
-	if (!begin_session(e, info, &session))
-		return fail();
+	if (!begin_session(e, info, job, &session))
+		return false;
 
 	for (const Tile &tile : tiles) {
 		Slot slot;
 		const size_t tile_row = size_t(tile.w) * kBytesPerPixel;
-		if (!claim(e, tile_row * tile.h, job.user, job.priority, &slot))
+		if (!claim(e, tile_row * tile.h, job, &slot))
 			break;
 
 		auto *dst = (uint8_t *) slot.mapped;
@@ -1335,7 +1327,7 @@ ThumbScaler::queue(const Job &job)
 		req.tile_oy = tile.oy;
 		req.tile_w = tile.w;
 		req.tile_h = tile.h;
-		if (!enqueue(e, req))
+		if (!enqueue(e, req, job))
 			break;
 	}
 	end_session(e, session);
@@ -1351,7 +1343,6 @@ ThumbScaler::reprioritize(uint64_t user, Priority priority)
 	Impl &e = *impl_;
 	lock_guard lock(e.mu);
 	bool found = false;
-	e.priority_overrides[user] = priority;
 	for (Waiter *waiter : e.waiters) {
 		if (waiter->user == user) {
 			found = true;
@@ -1384,7 +1375,6 @@ ThumbScaler::cancel(uint64_t user)
 	Impl &e = *impl_;
 	lock_guard lock(e.mu);
 	bool found = false;
-	e.canceled.insert(user);
 	for (auto it = e.pending.begin(); it != e.pending.end();) {
 		if (it->req.user != user) {
 			it++;
@@ -1404,7 +1394,6 @@ ThumbScaler::cancel(uint64_t user)
 	}
 	for (Waiter *waiter : e.waiters)
 		found |= waiter->user == user;
-	e.priority_overrides.erase(user);
 	e.cv.notify_all();
 	return found;
 }
@@ -1566,12 +1555,6 @@ ThumbScaler::poll(vector<Result> *done)
 						finished.end())
 						finished.push_back(s->id);
 				}
-				e.priority_overrides.erase(item.req.user);
-				e.canceled.erase(item.req.user);
-			} else {
-				lock_guard lock(e.mu);
-				e.priority_overrides.erase(item.req.user);
-				e.canceled.erase(item.req.user);
 			}
 		}
 		for (Result &result : batch_results)
@@ -1597,11 +1580,8 @@ ThumbScaler::poll(vector<Result> *done)
 	}
 	{
 		lock_guard lock(e.mu);
-		for (Result &result : e.failed_results) {
-			e.priority_overrides.erase(result.user);
-			e.canceled.erase(result.user);
+		for (Result &result : e.failed_results)
 			done->push_back(std::move(result));
-		}
 		e.failed_results.clear();
 		vector<uint32_t> erase;
 		for (auto &entry : e.sessions) {

libdn/thumb-scaler.hpp

@@ -11,6 +11,7 @@
 
 #include <vulkan/vulkan.h>
 
+#include <atomic>
 #include <cstdint>
 #include <memory>
 #include <string>
@@ -34,6 +35,14 @@ struct ThumbScaler
 		Maintenance,
 	};
 
+	/// Reaches a job whose queue() may not have started or returned yet,
+	/// which cancel() and reprioritize() by user cannot.  Change it first,
+	/// then call those for work already queued.
+	struct Control {
+		std::atomic_bool canceled = false;
+		std::atomic<Priority> priority = Priority::Maintenance;
+	};
+
 	struct Job {
 		struct Output {
 			uint32_t width = 0;
@@ -49,6 +58,8 @@ struct ThumbScaler
 		Priority priority = Priority::Maintenance;
 		uint64_t user = 0;
 		std::string path;
+		// Optional; when present, its priority replaces the one above.
+		std::shared_ptr<const Control> control;
 	};
 	struct Result {
 		struct Output {
@@ -78,6 +89,7 @@ struct ThumbScaler
 	bool queue(const Job &job);
 	/// Cancel work that has not been submitted to Vulkan. Submitted work may
 	/// finish, but its result is discarded by the caller's generation gate.
+	/// A producer still inside queue() only stops for its Job::control.
 	bool cancel(uint64_t user);
 	/// Change the priority of a job waiting for staging or submission.
 	bool reprioritize(uint64_t user, Priority priority);

libdnrs/lib.rs

@@ -55,11 +55,6 @@ pub enum dnrs_pixel_format {
 pub struct dnrs_document_info {
 	codec: *const c_char,
 	page_count: u32,
-	icc: dnrs_blob,
-	exif: dnrs_blob,
-	xmp: dnrs_blob,
-	text: *const dnrs_text,
-	text_length: usize,
 }
 
 #[repr(C)]
@@ -69,6 +64,11 @@ pub struct dnrs_page_info {
 	frame_count: u32,
 	orientation: u32,
 	loop_count: u64,
+	icc: dnrs_blob,
+	exif: dnrs_blob,
+	xmp: dnrs_blob,
+	text: *const dnrs_text,
+	text_length: usize,
 }
 
 #[repr(C)]
@@ -101,6 +101,11 @@ struct Page {
 	orientation: u32,
 	loop_count: u64,
 	frames: Vec<Frame>,
+	icc: Vec<u8>,
+	exif: Vec<u8>,
+	xmp: Vec<u8>,
+	_text_storage: Vec<(CString, CString)>,
+	text: Vec<dnrs_text>,
 }
 
 #[derive(Default)]
@@ -114,11 +119,6 @@ struct Metadata {
 
 pub struct dnrs_decoder {
 	codec: CString,
-	icc: Vec<u8>,
-	exif: Vec<u8>,
-	xmp: Vec<u8>,
-	_text_storage: Vec<(CString, CString)>,
-	text: Vec<dnrs_text>,
 	pages: Vec<Page>,
 	page_index: Option<usize>,
 	frame_index: usize,
@@ -791,20 +791,14 @@ fn decoder_from_pages(
 	codec: String,
 	decoded: Vec<(Frame, Metadata)>,
 ) -> Result<dnrs_decoder, String> {
-	let mut first = None;
-	let mut pages = Vec::new();
-	for (frame, metadata) in decoded {
-		pages.push(Page {
-			width: frame.width,
-			height: frame.height,
-			orientation: metadata.orientation,
-			loop_count: 0,
-			frames: vec![frame],
-		});
-		first.get_or_insert(metadata);
+	let pages = decoded
+		.into_iter()
+		.map(|(frame, metadata)| make_page(vec![frame], 0, metadata))
+		.collect::<Result<Vec<_>, String>>()?;
+	if pages.is_empty() {
+		return Err("the image has no pages".into());
 	}
-	let first = first.ok_or("the image has no pages")?;
-	make_decoder(codec, pages, first)
+	make_decoder(codec, pages)
 }
 
 fn decoder_from_frames(
@@ -813,24 +807,30 @@ fn decoder_from_frames(
 	loop_count: u64,
 	metadata: Metadata,
 ) -> Result<dnrs_decoder, String> {
-	let first = frames
-		.first()
-		.ok_or_else(|| "the image has no frames".to_string())?;
-	let page = Page {
-		width: first.width,
-		height: first.height,
-		orientation: metadata.orientation,
-		loop_count,
-		frames,
-	};
-	make_decoder(codec, vec![page], metadata)
+	make_decoder(codec, vec![make_page(frames, loop_count, metadata)?])
 }
 
 fn make_decoder(
 	codec: String,
 	pages: Vec<Page>,
-	metadata: Metadata,
 ) -> Result<dnrs_decoder, String> {
+	Ok(dnrs_decoder {
+		codec: CString::new(codec).map_err(|_| "invalid codec name")?,
+		pages,
+		page_index: None,
+		frame_index: 0,
+	})
+}
+
+fn make_page(
+	frames: Vec<Frame>,
+	loop_count: u64,
+	metadata: Metadata,
+) -> Result<Page, String> {
+	let (width, height) = frames
+		.first()
+		.map(|first| (first.width, first.height))
+		.ok_or_else(|| "the image has no frames".to_string())?;
 	let text_storage = metadata
 		.text
 		.into_iter()
@@ -849,16 +849,17 @@ fn make_decoder(
 			value: value.as_ptr(),
 		})
 		.collect();
-	Ok(dnrs_decoder {
-		codec: CString::new(codec).map_err(|_| "invalid codec name")?,
+	Ok(Page {
+		width,
+		height,
+		orientation: metadata.orientation,
+		loop_count,
+		frames,
 		icc: metadata.icc,
 		exif: metadata.exif,
 		xmp: metadata.xmp,
 		_text_storage: text_storage,
 		text,
-		pages,
-		page_index: None,
-		frame_index: 0,
 	})
 }
 
@@ -922,20 +923,6 @@ pub unsafe extern "C" fn dnrs_decoder_get_info(
 		*info = dnrs_document_info {
 			codec: decoder.codec.as_ptr(),
 			page_count: u32::try_from(decoder.pages.len()).unwrap_or(u32::MAX),
-			icc: dnrs_blob {
-				data: decoder.icc.as_ptr(),
-				length: decoder.icc.len(),
-			},
-			exif: dnrs_blob {
-				data: decoder.exif.as_ptr(),
-				length: decoder.exif.len(),
-			},
-			xmp: dnrs_blob {
-				data: decoder.xmp.as_ptr(),
-				length: decoder.xmp.len(),
-			},
-			text: decoder.text.as_ptr(),
-			text_length: decoder.text.len(),
 		};
 		Ok(true)
 	})
@@ -963,6 +950,20 @@ pub unsafe extern "C" fn dnrs_decoder_next_page(
 			frame_count: u32::try_from(page.frames.len()).unwrap_or(u32::MAX),
 			orientation: page.orientation,
 			loop_count: page.loop_count,
+			icc: dnrs_blob {
+				data: page.icc.as_ptr(),
+				length: page.icc.len(),
+			},
+			exif: dnrs_blob {
+				data: page.exif.as_ptr(),
+				length: page.exif.len(),
+			},
+			xmp: dnrs_blob {
+				data: page.xmp.as_ptr(),
+				length: page.xmp.len(),
+			},
+			text: page.text.as_ptr(),
+			text_length: page.text.len(),
 		};
 		decoder.page_index = Some(next);
 		decoder.frame_index = 0;

libdnrs/libdnrs.h

@@ -45,19 +45,21 @@ enum dnrs_pixel_format {
 struct dnrs_document_info {
 	const char *codec;
 	uint32_t page_count;
-	struct dnrs_blob icc;
-	struct dnrs_blob exif;
-	struct dnrs_blob xmp;
-	const struct dnrs_text *text;
-	size_t text_length;
 };
 
+// Metadata is per page, as multipage TIFF has it, and borrowed from the
+// decoder until it is freed.
 struct dnrs_page_info {
 	uint32_t width;
 	uint32_t height;
 	uint32_t frame_count;
 	uint32_t orientation;
 	uint64_t loop_count;
+	struct dnrs_blob icc;
+	struct dnrs_blob exif;
+	struct dnrs_blob xmp;
+	const struct dnrs_text *text;
+	size_t text_length;
 };
 
 struct dnrs_frame {

libdnrs/tests.rs

@@ -151,16 +151,15 @@ fn png_rgba8_metadata_and_iteration() {
 		true,
 	);
 	let decoder = open(&data, false);
-	let document = document_info(&decoder);
-	assert_eq!(document.page_count, 1);
-	assert!(!document.exif.data.is_null());
-	assert!(document.exif.length >= 26);
-	assert_eq!(document.text_length, 1);
-	let text = unsafe { &*document.text };
-	assert_eq!(unsafe { CStr::from_ptr(text.key) }.to_bytes(), b"prompt");
-	assert_eq!(unsafe { CStr::from_ptr(text.value) }.to_bytes(), b"hello");
+	assert_eq!(document_info(&decoder).page_count, 1);
 
 	let page = next_page(&decoder);
+	assert!(!page.exif.data.is_null());
+	assert!(page.exif.length >= 26);
+	assert_eq!(page.text_length, 1);
+	let text = unsafe { &*page.text };
+	assert_eq!(unsafe { CStr::from_ptr(text.key) }.to_bytes(), b"prompt");
+	assert_eq!(unsafe { CStr::from_ptr(text.value) }.to_bytes(), b"hello");
 	assert_eq!((page.width, page.height, page.frame_count), (2, 1, 1));
 	assert_eq!(page.orientation, 6);
 	let mut frame = next_frame(&decoder);
@@ -274,6 +273,37 @@ fn tiff_pages_are_separate() {
 	}
 }
 
+#[test]
+fn tiff_pages_keep_their_own_metadata() {
+	use tiff::encoder::{colortype, TiffEncoder};
+	use tiff::tags::Tag;
+
+	// Nothing here parses the profiles, so any distinct bytes will do.
+	// The tiff crate budgets tag values against the decoded image size, as
+	// several times their byte length, so the pages cannot be tiny.
+	let mut cursor = Cursor::new(Vec::new());
+	{
+		let mut encoder = TiffEncoder::new(&mut cursor).unwrap();
+		for icc in [&b"first"[..], &b"second"[..]] {
+			let mut image =
+				encoder.new_image::<colortype::RGB8>(16, 16).unwrap();
+			image.encoder().write_tag(Tag::IccProfile, icc).unwrap();
+			image.write_data(&[0; 16 * 16 * 3]).unwrap();
+		}
+	}
+	let decoder = open(&cursor.into_inner(), false);
+	assert_eq!(document_info(&decoder).page_count, 2);
+	for expected in [&b"first"[..], &b"second"[..]] {
+		let page = next_page(&decoder);
+		assert_eq!(
+			unsafe {
+				std::slice::from_raw_parts(page.icc.data, page.icc.length)
+			},
+			expected
+		);
+	}
+}
+
 #[test]
 fn float_tiff_keeps_linear_floats() {
 	use tiff::encoder::{colortype, TiffEncoder};

msys2-configure.sh

@@ -54,7 +54,7 @@ fetch() {
 	url=https://aka.ms/vs/17/release/vc_redist.x64.exe
 	sha256=cc0ff0eb1dc3f5188ae6300faef32bf5beeba4bdd6e8e445a9184072096b713b
 	name=$(basename "$url")
-	echo "$sha256 *packages/$name" >> db.sums
+	echo "$sha256 *$name" >> db.sums
 	status Fetching "$name"
 	[ -f "$name" ] || curl -#Lo "$name" "$url"
 
@@ -70,7 +70,7 @@ fetch() {
 	url=https://github.com/linebender/resvg/releases/download/v0.48.1/resvg-0.48.1.tar.xz
 	sha256=13ed5a2bae7a01156288ecae5bf944cf7d1c572742c19fc68027947a4d87294c
 	name=$(basename "$url")
-	echo "$sha256 *packages/$name" >> db.sums
+	echo "$sha256 *$name" >> db.sums
 	status Fetching "$name"
 	[ -f "$name" ] || curl -#Lo "$name" "$url"
 	ln -sf "$name" resvg.tar.xz