Improve gdk-pixbuf colour management

README.adoc

@@ -5,7 +5,8 @@ fastiv
 raw photos, HEIC, AVIF, WebP, SVG, X11 cursors and TIFF, or whatever gdk-pixbuf
 loads.
 
-Its development status can be summarized as '`beta`'.
+Its development status can be summarized as '`beta`'.  E.g., colour management
+is partial, and certain GIFs animate wrong.
 
 Non-goals
 ---------
@@ -25,8 +26,8 @@ Building and Running
 Build dependencies: Meson, pkg-config +
 Runtime dependencies: gtk+-3.0, glib>=2.64, pixman-1, shared-mime-info,
 spng>=0.7.0, libturbojpeg +
-Optional dependencies: LibRaw, librsvg-2.0, xcursor, libwebp, libheif, libtiff,
-gdk-pixbuf-2.0, ExifTool
+Optional dependencies: lcms2, LibRaw, librsvg-2.0, xcursor, libwebp, libheif,
+libtiff, gdk-pixbuf-2.0, ExifTool
 
  $ git clone --recursive https://git.janouch.name/p/fastiv.git
  $ meson builddir

fastiv.c

@@ -130,9 +130,15 @@ static struct key_group help_keys_view[] = {
 		{}
 	}},
 	{"Configuration", (struct key[]) {
+#ifdef HAVE_LCMS2
+		{"c", "Toggle color management"},
+#endif
 		{"x", "Toggle scale to fit if larger"},
 		{"i", "Toggle smooth scaling"},
 		{"t", "Toggle transparency highlighting"},
+#ifdef HAVE_JPEG_QS
+		{"e", "Toggle low-quality JPEG enhancement"},
+#endif
 		{}
 	}},
 	{"Control", (struct key[]) {

fiv-io.c

@@ -241,19 +241,19 @@ trivial_cmyk_to_host_byte_order_argb(unsigned char *p, int len)
 
 static void
 fiv_io_profile_cmyk(
-	cairo_surface_t *surface, FivIoProfile src, FivIoProfile dst)
+	cairo_surface_t *surface, FivIoProfile source, FivIoProfile target)
 {
 	unsigned char *data = cairo_image_surface_get_data(surface);
 	int w = cairo_image_surface_get_width(surface);
 	int h = cairo_image_surface_get_height(surface);
 
 #ifndef HAVE_LCMS2
-	(void) src;
-	(void) dst;
+	(void) source;
+	(void) target;
 #else
 	cmsHTRANSFORM transform = NULL;
-	if (src && dst) {
-		transform = cmsCreateTransform(src, TYPE_CMYK_8_REV, dst,
+	if (source && target) {
+		transform = cmsCreateTransform(source, TYPE_CMYK_8_REV, target,
 			FIV_IO_LCMS2_ARGB, INTENT_PERCEPTUAL, 0);
 	}
 	if (transform) {
@@ -266,13 +266,13 @@ fiv_io_profile_cmyk(
 }
 
 static void
-fiv_io_profile_xrgb(
-	cairo_surface_t *surface, FivIoProfile src, FivIoProfile dst)
+fiv_io_profile_xrgb32(
+	cairo_surface_t *surface, FivIoProfile source, FivIoProfile target)
 {
 #ifndef HAVE_LCMS2
 	(void) surface;
-	(void) src;
-	(void) dst;
+	(void) source;
+	(void) target;
 #else
 	unsigned char *data = cairo_image_surface_get_data(surface);
 	int w = cairo_image_surface_get_width(surface);
@@ -280,12 +280,12 @@ fiv_io_profile_xrgb(
 
 	// TODO(p): We should make this optional.
 	cmsHPROFILE src_fallback = NULL;
-	if (dst && !src)
-		src = src_fallback = cmsCreate_sRGBProfile();
+	if (target && !source)
+		source = src_fallback = cmsCreate_sRGBProfile();
 
 	cmsHTRANSFORM transform = NULL;
-	if (src && dst) {
-		transform = cmsCreateTransform(src, FIV_IO_LCMS2_ARGB, dst,
+	if (source && target) {
+		transform = cmsCreateTransform(source, FIV_IO_LCMS2_ARGB, target,
 			FIV_IO_LCMS2_ARGB, INTENT_PERCEPTUAL, 0);
 	}
 	if (transform) {
@@ -297,11 +297,72 @@ fiv_io_profile_xrgb(
 #endif
 }
 
-// --- Wuffs -------------------------------------------------------------------
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
+static void
+fiv_io_profile_xrgb32_page(cairo_surface_t *page, FivIoProfile target)
+{
+	GBytes *bytes = NULL;
+	gsize len = 0;
+	gconstpointer p = NULL;
+	FivIoProfile source = NULL;
+	if ((bytes = cairo_surface_get_user_data(page, &fiv_io_key_icc)) &&
+		(p = g_bytes_get_data(bytes, &len)))
+		source = fiv_io_profile_new(p, len);
+
+	// TODO(p): Animations need to be composited in a linear colour space.
+	for (cairo_surface_t *frame = page; frame != NULL;
+		frame = cairo_surface_get_user_data(frame, &fiv_io_key_frame_next))
+		fiv_io_profile_xrgb32(frame, source, target);
+
+	if (source)
+		fiv_io_profile_free(source);
+}
+
+// TODO(p): Offer better integration, upgrade the bit depth if appropriate.
+static cairo_surface_t *
+fiv_io_profile_finalize(cairo_surface_t *image, FivIoProfile target)
+{
+	if (!image || !target)
+		return image;
+
+	for (cairo_surface_t *page = image; page != NULL;
+		page = cairo_surface_get_user_data(page, &fiv_io_key_page_next)) {
+		// TODO(p): 1. un/premultiply ARGB, 2. do colour management
+		// early enough, so that no avoidable increase of quantization error
+		// occurs beforehands, and also for correct alpha compositing.
+		// FIXME: This assumes that if the first frame is opaque, they all are.
+		if (cairo_image_surface_get_format(page) == CAIRO_FORMAT_RGB24)
+			fiv_io_profile_xrgb32_page(page, target);
+	}
+	return image;
+}
 
 // From libwebp, verified to exactly match [x * a / 255].
 #define PREMULTIPLY8(a, x) (((uint32_t) (x) * (uint32_t) (a) * 32897U) >> 23)
 
+static void
+fiv_io_premultiply_argb32(cairo_surface_t *surface)
+{
+	int w = cairo_image_surface_get_width(surface);
+	int h = cairo_image_surface_get_height(surface);
+	unsigned char *data = cairo_image_surface_get_data(surface);
+	int stride = cairo_image_surface_get_stride(surface);
+
+	for (int y = 0; y < h; y++) {
+		uint32_t *dstp = (uint32_t *) (data + stride * y);
+		for (int x = 0; x < w; x++) {
+			uint32_t argb = dstp[x], a = argb >> 24;
+			dstp[x] = a << 24 |
+				PREMULTIPLY8(a, 0xFF & (argb >> 16)) << 16 |
+				PREMULTIPLY8(a, 0xFF & (argb >>  8)) <<  8 |
+				PREMULTIPLY8(a, 0xFF &  argb);
+		}
+	}
+}
+
+// --- Wuffs -------------------------------------------------------------------
+
 static bool
 pull_passthrough(const wuffs_base__more_information *minfo,
 	wuffs_base__io_buffer *src, wuffs_base__io_buffer *dst, GError **error)
@@ -738,7 +799,7 @@ fail:
 
 static cairo_surface_t *
 open_wuffs_using(wuffs_base__image_decoder *(*allocate)(),
-	const gchar *data, gsize len, GError **error)
+	const gchar *data, gsize len, FivIoProfile profile, GError **error)
 {
 	wuffs_base__image_decoder *dec = allocate();
 	if (!dec) {
@@ -749,7 +810,7 @@ open_wuffs_using(wuffs_base__image_decoder *(*allocate)(),
 	cairo_surface_t *surface = open_wuffs(
 		dec, wuffs_base__ptr_u8__reader((uint8_t *) data, len, TRUE), error);
 	free(dec);
-	return surface;
+	return fiv_io_profile_finalize(surface, profile);
 }
 
 // --- JPEG --------------------------------------------------------------------
@@ -859,7 +920,7 @@ load_jpeg_finalize(cairo_surface_t *surface, bool cmyk,
 	if (cmyk)
 		fiv_io_profile_cmyk(surface, source, destination);
 	else
-		fiv_io_profile_xrgb(surface, source, destination);
+		fiv_io_profile_xrgb32(surface, source, destination);
 
 	if (source)
 		fiv_io_profile_free(source);
@@ -886,7 +947,8 @@ open_libjpeg_turbo(
 		return NULL;
 	}
 
-	int pixel_format = (colorspace == TJCS_CMYK || colorspace == TJCS_YCCK)
+	bool use_cmyk = colorspace == TJCS_CMYK || colorspace == TJCS_YCCK;
+	int pixel_format = use_cmyk
 		? TJPF_CMYK
 		: (G_BYTE_ORDER == G_LITTLE_ENDIAN ? TJPF_BGRX : TJPF_XRGB);
 
@@ -917,8 +979,7 @@ open_libjpeg_turbo(
 		}
 	}
 
-	load_jpeg_finalize(
-		surface, (pixel_format == TJPF_CMYK), profile, data, len);
+	load_jpeg_finalize(surface, use_cmyk, profile, data, len);
 	tjDestroy(dec);
 	return surface;
 }
@@ -961,8 +1022,10 @@ open_libjpeg_enhanced(
 	jpeg_create_decompress(&cinfo);
 	jpeg_mem_src(&cinfo, (const unsigned char *) data, len);
 	(void) jpeg_read_header(&cinfo, true);
-	if (cinfo.jpeg_color_space == JCS_CMYK ||
-		cinfo.jpeg_color_space == JCS_YCCK)
+
+	bool use_cmyk = cinfo.jpeg_color_space == JCS_CMYK ||
+		cinfo.jpeg_color_space == JCS_YCCK;
+	if (use_cmyk)
 		cinfo.out_color_space = JCS_CMYK;
 	else if (G_BYTE_ORDER == G_BIG_ENDIAN)
 		cinfo.out_color_space = JCS_EXT_XRGB;
@@ -1003,8 +1066,7 @@ open_libjpeg_enhanced(
 			surface_data, cinfo.output_width * cinfo.output_height);
 	(void) jpegqs_finish_decompress(&cinfo);
 
-	load_jpeg_finalize(
-		surface, (cinfo.out_color_space == JCS_CMYK), profile, data, len);
+	load_jpeg_finalize(surface, use_cmyk, profile, data, len);
 	jpeg_destroy_decompress(&cinfo);
 	return surface;
 }
@@ -1353,6 +1415,8 @@ open_xcursor(const gchar *data, gsize len, GError **error)
 	static cairo_user_data_key_t key = {};
 	cairo_surface_set_user_data(
 		pages, &key, images, (cairo_destroy_func_t) XcursorImagesDestroy);
+
+	// Do not bother doing colour correction, there is no correct rendering.
 	return pages;
 }
 
@@ -1492,7 +1556,8 @@ fail:
 }
 
 static cairo_surface_t *
-open_libwebp(const gchar *data, gsize len, const gchar *path, GError **error)
+open_libwebp(const gchar *data, gsize len, const gchar *path,
+	FivIoProfile profile, GError **error)
 {
 	// It is wholly zero-initialized by libwebp.
 	WebPDecoderConfig config = {};
@@ -1556,7 +1621,7 @@ open_libwebp(const gchar *data, gsize len, const gchar *path, GError **error)
 
 fail:
 	WebPFreeDecBuffer(&config.output);
-	return result;
+	return fiv_io_profile_finalize(result, profile);
 }
 
 #endif  // HAVE_LIBWEBP --------------------------------------------------------
@@ -1615,18 +1680,8 @@ load_libheif_image(struct heif_image_handle *handle, GError **error)
 	}
 
 	// TODO(p): Test real behaviour on real transparent images.
-	if (has_alpha && !heif_image_handle_is_premultiplied_alpha(handle)) {
-		for (int y = 0; y < h; y++) {
-			uint32_t *dstp = (uint32_t *) (dst + dst_stride * y);
-			for (int x = 0; x < w; x++) {
-				uint32_t argb = dstp[x], a = argb >> 24;
-				dstp[x] = a << 24 |
-					PREMULTIPLY8(a, 0xFF & (argb >> 16)) << 16 |
-					PREMULTIPLY8(a, 0xFF & (argb >>  8)) <<  8 |
-					PREMULTIPLY8(a, 0xFF &  argb);
-			}
-		}
-	}
+	if (has_alpha && !heif_image_handle_is_premultiplied_alpha(handle))
+		fiv_io_premultiply_argb32(surface);
 
 	heif_item_id exif_id = 0;
 	if (heif_image_handle_get_list_of_metadata_block_IDs(
@@ -1703,7 +1758,8 @@ load_libheif_aux_images(const gchar *path, struct heif_image_handle *top,
 }
 
 static cairo_surface_t *
-open_libheif(const gchar *data, gsize len, const gchar *path, GError **error)
+open_libheif(const gchar *data, gsize len, const gchar *path,
+	FivIoProfile profile, GError **error)
 {
 	// libheif will throw C++ exceptions on allocation failures.
 	// The library is generally awful through and through.
@@ -1747,7 +1803,7 @@ open_libheif(const gchar *data, gsize len, const gchar *path, GError **error)
 	g_free(ids);
 fail_read:
 	heif_context_free(ctx);
-	return result;
+	return fiv_io_profile_finalize(result, profile);
 }
 
 #endif  // HAVE_LIBHEIF --------------------------------------------------------
@@ -2003,7 +2059,30 @@ fail:
 #ifdef HAVE_GDKPIXBUF  // ------------------------------------------------------
 
 static cairo_surface_t *
-open_gdkpixbuf(const gchar *data, gsize len, GError **error)
+load_gdkpixbuf_argb32_unpremultiplied(GdkPixbuf *pixbuf)
+{
+	int w = gdk_pixbuf_get_width(pixbuf);
+	int h = gdk_pixbuf_get_height(pixbuf);
+	cairo_surface_t *surface =
+		cairo_image_surface_create(CAIRO_FORMAT_ARGB32, w, h);
+
+	guint length = 0;
+	guchar *src = gdk_pixbuf_get_pixels_with_length(pixbuf, &length);
+	int src_stride = gdk_pixbuf_get_rowstride(pixbuf);
+	uint32_t *dst = (uint32_t *) cairo_image_surface_get_data(surface);
+	for (int y = 0; y < h; y++) {
+		const guchar *p = src + y * src_stride;
+		for (int x = 0; x < w; x++) {
+			*dst++ = (uint32_t) p[3] << 24 | p[0] << 16 | p[1] << 8 | p[2];
+			p += 4;
+		}
+	}
+	return surface;
+}
+
+static cairo_surface_t *
+open_gdkpixbuf(
+	const gchar *data, gsize len, FivIoProfile profile, GError **error)
 {
 	// gdk-pixbuf controls the playback itself, there is no reliable method of
 	// extracting individual frames (due to loops).
@@ -2013,8 +2092,24 @@ open_gdkpixbuf(const gchar *data, gsize len, GError **error)
 	if (!pixbuf)
 		return NULL;
 
-	cairo_surface_t *surface =
-		gdk_cairo_surface_create_from_pixbuf(pixbuf, 1, NULL);
+	bool custom_argb32 = profile && gdk_pixbuf_get_has_alpha(pixbuf) &&
+		gdk_pixbuf_get_colorspace(pixbuf) == GDK_COLORSPACE_RGB &&
+		gdk_pixbuf_get_n_channels(pixbuf) == 4 &&
+		gdk_pixbuf_get_bits_per_sample(pixbuf) == 8;
+
+	cairo_surface_t *surface = NULL;
+	if (custom_argb32)
+		surface = load_gdkpixbuf_argb32_unpremultiplied(pixbuf);
+	else
+		surface = gdk_cairo_surface_create_from_pixbuf(pixbuf, 1, NULL);
+
+	cairo_status_t surface_status = cairo_surface_status(surface);
+	if (surface_status != CAIRO_STATUS_SUCCESS) {
+		set_error(error, cairo_status_to_string(surface_status));
+		cairo_surface_destroy(surface);
+		g_object_unref(pixbuf);
+		return NULL;
+	}
 
 	const char *orientation = gdk_pixbuf_get_option(pixbuf, "orientation");
 	if (orientation && strlen(orientation) == 1) {
@@ -2036,6 +2131,12 @@ open_gdkpixbuf(const gchar *data, gsize len, GError **error)
 	}
 
 	g_object_unref(pixbuf);
+	if (custom_argb32) {
+		fiv_io_profile_xrgb32_page(surface, profile);
+		fiv_io_premultiply_argb32(surface);
+	} else {
+		surface = fiv_io_profile_finalize(surface, profile);
+	}
 	return surface;
 }
 
@@ -2097,17 +2198,17 @@ fiv_io_open_from_data(const char *data, size_t len, const gchar *path,
 		// which is so far unsupported here.
 		surface = open_wuffs_using(
 			wuffs_bmp__decoder__alloc_as__wuffs_base__image_decoder, data, len,
-			error);
+			profile, error);
 		break;
 	case WUFFS_BASE__FOURCC__GIF:
 		surface = open_wuffs_using(
 			wuffs_gif__decoder__alloc_as__wuffs_base__image_decoder, data, len,
-			error);
+			profile, error);
 		break;
 	case WUFFS_BASE__FOURCC__PNG:
 		surface = open_wuffs_using(
 			wuffs_png__decoder__alloc_as__wuffs_base__image_decoder, data, len,
-			error);
+			profile, error);
 		break;
 	case WUFFS_BASE__FOURCC__JPEG:
 		surface = enhance
@@ -2146,7 +2247,7 @@ fiv_io_open_from_data(const char *data, size_t len, const gchar *path,
 #endif  // HAVE_XCURSOR --------------------------------------------------------
 #ifdef HAVE_LIBWEBP  //---------------------------------------------------------
 		// TODO(p): https://github.com/google/wuffs/commit/4c04ac1
-		if ((surface = open_libwebp(data, len, path, error)))
+		if ((surface = open_libwebp(data, len, path, profile, error)))
 			break;
 		if (error) {
 			g_debug("%s", (*error)->message);
@@ -2154,7 +2255,7 @@ fiv_io_open_from_data(const char *data, size_t len, const gchar *path,
 		}
 #endif  // HAVE_LIBWEBP --------------------------------------------------------
 #ifdef HAVE_LIBHEIF  //---------------------------------------------------------
-		if ((surface = open_libheif(data, len, path, error)))
+		if ((surface = open_libheif(data, len, path, profile, error)))
 			break;
 		if (error) {
 			g_debug("%s", (*error)->message);
@@ -2172,7 +2273,7 @@ fiv_io_open_from_data(const char *data, size_t len, const gchar *path,
 #endif  // HAVE_LIBTIFF --------------------------------------------------------
 #ifdef HAVE_GDKPIXBUF  // ------------------------------------------------------
 		// This is only used as a last resort, the rest above is special-cased.
-		if ((surface = open_gdkpixbuf(data, len, error)))
+		if ((surface = open_gdkpixbuf(data, len, profile, error)))
 			break;
 		if (error && (*error)->code != GDK_PIXBUF_ERROR_UNKNOWN_TYPE)
 			break;

fiv-view.c

@@ -1060,12 +1060,16 @@ fiv_view_key_press_event(GtkWidget *widget, GdkEventKey *event)
 	case GDK_KEY_h:
 		return set_scale_to_fit_height(self);
 
+	case GDK_KEY_c:
+		return command(self, FIV_VIEW_COMMAND_TOGGLE_CMS);
 	case GDK_KEY_x:  // Inspired by gThumb, which has more such modes.
 		return command(self, FIV_VIEW_COMMAND_TOGGLE_SCALE_TO_FIT);
 	case GDK_KEY_i:
 		return command(self, FIV_VIEW_COMMAND_TOGGLE_FILTER);
 	case GDK_KEY_t:
 		return command(self, FIV_VIEW_COMMAND_TOGGLE_CHECKERBOARD);
+	case GDK_KEY_e:
+		return command(self, FIV_VIEW_COMMAND_TOGGLE_ENHANCE);
 
 	case GDK_KEY_less:
 		return command(self, FIV_VIEW_COMMAND_ROTATE_LEFT);