From 7975bfaae1caac46aee54b1a57d03a7ef6b98645 Mon Sep 17 00:00:00 2001 From: "hanno@schwalm-bremen.de" Date: Sun, 17 Mar 2024 17:14:52 +0100 Subject: [PATCH] Code maintenance related to view and mipmap cache No functionality changed - some formatting to wanted style - gboolean instead of int where appropriate - some int to int32_t - use dt_mipmap_size_t instead of int where that is correct - added a few const where spotted - fprintf replaced by dt_print - some improved logging for dt_view_image_get_surface --- src/common/mipmap_cache.c | 86 +++++++++++++++++++++++++----------- src/imageio/imageio.c | 40 ++++++++--------- src/imageio/imageio_common.h | 8 ++-- src/iop/demosaic.c | 2 +- src/iop/highlights.c | 2 +- src/views/view.c | 36 ++++++++------- 6 files changed, 106 insertions(+), 68 deletions(-) diff --git a/src/common/mipmap_cache.c b/src/common/mipmap_cache.c index 707496c05e5e..3d0a848b7bd9 100644 --- a/src/common/mipmap_cache.c +++ b/src/common/mipmap_cache.c @@ -1,6 +1,6 @@ /* This file is part of darktable, - Copyright (C) 2011-2023 darktable developers. + Copyright (C) 2011-2024 darktable developers. darktable is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -223,10 +223,18 @@ static int dt_mipmap_cache_get_filename(gchar *mipmapfilename, size_t size) return r; } -static void _init_f(dt_mipmap_buffer_t *mipmap_buf, float *buf, uint32_t *width, uint32_t *height, float *iscale, +static void _init_f(dt_mipmap_buffer_t *mipmap_buf, + float *buf, + uint32_t *width, + uint32_t *height, + float *iscale, const dt_imgid_t imgid); -static void _init_8(uint8_t *buf, uint32_t *width, uint32_t *height, float *iscale, - dt_colorspaces_color_profile_type_t *color_space, const dt_imgid_t imgid, +static void _init_8(uint8_t *buf, + uint32_t *width, + uint32_t *height, + float *iscale, + dt_colorspaces_color_profile_type_t *color_space, + const dt_imgid_t imgid, const dt_mipmap_size_t size); // callback for the imageio core to allocate memory. @@ -419,7 +427,9 @@ void dt_mipmap_cache_allocate_dynamic(void *data, dt_cache_entry_t *entry) entry->cost = cache->buffer_size[mip]; } -static void dt_mipmap_cache_unlink_ondisk_thumbnail(void *data, dt_imgid_t imgid, dt_mipmap_size_t mip) +static void dt_mipmap_cache_unlink_ondisk_thumbnail(void *data, + const dt_imgid_t imgid, + dt_mipmap_size_t mip) { dt_mipmap_cache_t *cache = (dt_mipmap_cache_t *)data; @@ -659,7 +669,8 @@ static gboolean _raise_signal_mipmap_updated(gpointer user_data) return FALSE; // only call once } -static dt_mipmap_cache_one_t *_get_cache(dt_mipmap_cache_t *cache, const dt_mipmap_size_t mip) +static dt_mipmap_cache_one_t *_get_cache(dt_mipmap_cache_t *cache, + const dt_mipmap_size_t mip) { switch(mip) { @@ -928,7 +939,9 @@ void dt_mipmap_cache_get_with_caller( } } -void dt_mipmap_cache_release_with_caller(dt_mipmap_cache_t *cache, dt_mipmap_buffer_t *buf, const char *file, +void dt_mipmap_cache_release_with_caller(dt_mipmap_cache_t *cache, + dt_mipmap_buffer_t *buf, + const char *file, int line) { if(buf->size == DT_MIPMAP_NONE) return; @@ -942,9 +955,10 @@ void dt_mipmap_cache_release_with_caller(dt_mipmap_cache_t *cache, dt_mipmap_buf } -// return index dt_mipmap_size_t having at least width & height requested instead of minimum combined diff +// return index dt_mipmap_size_t having at least width and height requested instead of minimum combined diff // please note that the requested size is in pixels not dots. -dt_mipmap_size_t dt_mipmap_cache_get_matching_size(const dt_mipmap_cache_t *cache, const int32_t width, +dt_mipmap_size_t dt_mipmap_cache_get_matching_size(const dt_mipmap_cache_t *cache, + const int32_t width, const int32_t height) { dt_mipmap_size_t best = DT_MIPMAP_NONE; @@ -970,7 +984,9 @@ dt_mipmap_size_t dt_mipmap_cache_get_min_mip_from_pref(const char *value) return DT_MIPMAP_NONE; } -void dt_mipmap_cache_remove_at_size(dt_mipmap_cache_t *cache, const dt_imgid_t imgid, const dt_mipmap_size_t mip) +void dt_mipmap_cache_remove_at_size(dt_mipmap_cache_t *cache, + const dt_imgid_t imgid, + const dt_mipmap_size_t mip) { if(mip > DT_MIPMAP_8 || mip < DT_MIPMAP_0) return; // get rid of all ldr thumbnails: @@ -993,23 +1009,26 @@ void dt_mipmap_cache_remove_at_size(dt_mipmap_cache_t *cache, const dt_imgid_t i } } -void dt_mipmap_cache_remove(dt_mipmap_cache_t *cache, const dt_imgid_t imgid) +void dt_mipmap_cache_remove(dt_mipmap_cache_t *cache, + const dt_imgid_t imgid) { - // get rid of all ldr thumbnails: - for(dt_mipmap_size_t k = DT_MIPMAP_0; k < DT_MIPMAP_F; k++) { dt_mipmap_cache_remove_at_size(cache, imgid, k); } } -void dt_mipmap_cache_evict_at_size(dt_mipmap_cache_t *cache, const dt_imgid_t imgid, const dt_mipmap_size_t mip) + +void dt_mipmap_cache_evict_at_size(dt_mipmap_cache_t *cache, + const dt_imgid_t imgid, + const dt_mipmap_size_t mip) { const uint32_t key = get_key(imgid, mip); // write thumbnail to disc if not existing there dt_cache_remove(&_get_cache(cache, mip)->cache, key); } -void dt_mimap_cache_evict(dt_mipmap_cache_t *cache, const dt_imgid_t imgid) +void dt_mimap_cache_evict(dt_mipmap_cache_t *cache, + const dt_imgid_t imgid) { for(dt_mipmap_size_t k = DT_MIPMAP_0; k < DT_MIPMAP_F; k++) { @@ -1020,7 +1039,11 @@ void dt_mimap_cache_evict(dt_mipmap_cache_t *cache, const dt_imgid_t imgid) } } -static void _init_f(dt_mipmap_buffer_t *mipmap_buf, float *out, uint32_t *width, uint32_t *height, float *iscale, +static void _init_f(dt_mipmap_buffer_t *mipmap_buf, + float *out, + uint32_t *width, + uint32_t *height, + float *iscale, const dt_imgid_t imgid) { const uint32_t wd = *width, ht = *height; @@ -1135,9 +1158,18 @@ static int _bpp(dt_imageio_module_data_t *data) return 8; } -static int _write_image(dt_imageio_module_data_t *data, const char *filename, const void *in, - dt_colorspaces_color_profile_type_t over_type, const char *over_filename, - void *exif, int exif_len, dt_imgid_t imgid, int num, int total, dt_dev_pixelpipe_t *pipe, + +static int _write_image(dt_imageio_module_data_t *data, + const char *filename, + const void *in, + dt_colorspaces_color_profile_type_t over_type, + const char *over_filename, + void *exif, + int exif_len, + dt_imgid_t imgid, + int num, + int total, + dt_dev_pixelpipe_t *pipe, const gboolean export_masks) { _dummy_data_t *d = (_dummy_data_t *)data; @@ -1145,8 +1177,12 @@ static int _write_image(dt_imageio_module_data_t *data, const char *filename, co return 0; } -static void _init_8(uint8_t *buf, uint32_t *width, uint32_t *height, float *iscale, - dt_colorspaces_color_profile_type_t *color_space, const dt_imgid_t imgid, +static void _init_8(uint8_t *buf, + uint32_t *width, + uint32_t *height, + float *iscale, + dt_colorspaces_color_profile_type_t *color_space, + const dt_imgid_t imgid, const dt_mipmap_size_t size) { *iscale = 1.0f; @@ -1165,7 +1201,7 @@ static void _init_8(uint8_t *buf, uint32_t *width, uint32_t *height, float *isca } const gboolean altered = dt_image_altered(imgid); - int res = 1; + gboolean res = TRUE; const dt_image_t *cimg = dt_image_cache_get(darktable.image_cache, imgid, 'r'); // the orientation for this camera is not read correctly from exiv2, so we need @@ -1202,7 +1238,7 @@ static void _init_8(uint8_t *buf, uint32_t *width, uint32_t *height, float *isca dt_print(DT_DEBUG_CACHE, "[mipmap_cache] generate mip %d for image %d from jpeg\n", size, imgid); dt_iop_flip_and_zoom_8(tmp, jpg.width, jpg.height, buf, wd, ht, orientation, width, height); - res = 0; + res = FALSE; } free(tmp); } @@ -1221,7 +1257,7 @@ static void _init_8(uint8_t *buf, uint32_t *width, uint32_t *height, float *isca dt_image_cache_read_release(darktable.image_cache, img2); if(thumb_width < wd && thumb_height < ht && thumb_width < imgwd - 4 && thumb_height < imght - 4) { - res = 1; + res = TRUE; } else { @@ -1253,7 +1289,7 @@ static void _init_8(uint8_t *buf, uint32_t *width, uint32_t *height, float *isca dt_iop_flip_and_zoom_8(tmp.buf, tmp.width, tmp.height, buf, wd, ht, ORIENTATION_NONE, width, height); dt_mipmap_cache_release(darktable.mipmap_cache, &tmp); - res = 0; + res = FALSE; break; } } diff --git a/src/imageio/imageio.c b/src/imageio/imageio.c index d538091b1965..668a395a82de 100644 --- a/src/imageio/imageio.c +++ b/src/imageio/imageio.c @@ -1,6 +1,6 @@ /* This file is part of darktable, - Copyright (C) 2009-2023 darktable developers. + Copyright (C) 2009-2024 darktable developers. darktable is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -147,13 +147,13 @@ dt_image_flags_t dt_imageio_get_type_from_extension(const char *extension) } // load a full-res thumbnail: -int dt_imageio_large_thumbnail(const char *filename, +gboolean dt_imageio_large_thumbnail(const char *filename, uint8_t **buffer, int32_t *width, int32_t *height, dt_colorspaces_color_profile_type_t *color_space) { - int res = 1; + int res = TRUE; uint8_t *buf = NULL; char *mime_type = NULL; @@ -185,7 +185,7 @@ int dt_imageio_large_thumbnail(const char *filename, goto error; } - res = 0; + res = FALSE; } else { @@ -236,7 +236,7 @@ int dt_imageio_large_thumbnail(const char *filename, } } - res = 0; + res = FALSE; error_gm: if(image) DestroyImage(image); @@ -287,11 +287,11 @@ int dt_imageio_large_thumbnail(const char *filename, goto error_im; } - res = 0; + res = FALSE; error_im: DestroyMagickWand(image); - if(res != 0) goto error; + if(res) goto error; #else dt_print(DT_DEBUG_ALWAYS, "[dt_imageio_large_thumbnail] error: The thumbnail image is not in " @@ -634,7 +634,7 @@ gboolean dt_imageio_is_ldr(const char *filename) return FALSE; } -int dt_imageio_is_hdr(const char *filename) +gboolean dt_imageio_is_hdr(const char *filename) { const char *c = filename + strlen(filename); while(c > filename && *c != '.') c--; @@ -644,8 +644,8 @@ int dt_imageio_is_hdr(const char *filename) || !strcasecmp(c, ".exr") #endif ) - return 1; - return 0; + return TRUE; + return FALSE; } // transparent read method to load ldr image to dt_raw_image_t with @@ -708,7 +708,7 @@ void dt_imageio_to_fractional(const float in, } } -int dt_imageio_export(const dt_imgid_t imgid, +gboolean dt_imageio_export(const dt_imgid_t imgid, const char *filename, dt_imageio_module_format_t *format, dt_imageio_module_data_t *format_params, @@ -727,9 +727,9 @@ int dt_imageio_export(const dt_imgid_t imgid, { if(strcmp(format->mime(format_params), "x-copy") == 0) /* This is a just a copy, skip process and just export */ - return format->write_image(format_params, filename, NULL, icc_type, + return (format->write_image(format_params, filename, NULL, icc_type, icc_filename, NULL, 0, imgid, num, total, NULL, - export_masks); + export_masks)) != 0; else { const gboolean is_scaling = @@ -746,7 +746,7 @@ int dt_imageio_export(const dt_imgid_t imgid, // internal function: to avoid exif blob reading + 8-bit byteorder // flag + high-quality override -int dt_imageio_export_with_flags(const dt_imgid_t imgid, +gboolean dt_imageio_export_with_flags(const dt_imgid_t imgid, const char *filename, dt_imageio_module_format_t *format, dt_imageio_module_data_t *format_params, @@ -1195,17 +1195,17 @@ int dt_imageio_export_with_flags(const dt_imgid_t imgid, const int length = dt_exif_read_blob(&exif_profile, pathname, imgid, sRGB, processed_width, processed_height, 0); - res = format->write_image(format_params, filename, outbuf, icc_type, + res = (format->write_image(format_params, filename, outbuf, icc_type, icc_filename, exif_profile, length, imgid, - num, total, &pipe, export_masks); + num, total, &pipe, export_masks)) != 0; free(exif_profile); } else { - res = format->write_image(format_params, filename, outbuf, icc_type, + res = (format->write_image(format_params, filename, outbuf, icc_type, icc_filename, NULL, 0, imgid, num, total, - &pipe, export_masks); + &pipe, export_masks)) != 0; } if(res) @@ -1255,7 +1255,7 @@ int dt_imageio_export_with_flags(const dt_imgid_t imgid, if(!thumbnail_export) dt_set_backthumb_time(5.0); - return 0; // success + return FALSE; // success error: dt_dev_pixelpipe_cleanup(&pipe); @@ -1265,7 +1265,7 @@ int dt_imageio_export_with_flags(const dt_imgid_t imgid, if(!thumbnail_export) dt_set_backthumb_time(5.0); - return 1; + return TRUE; } diff --git a/src/imageio/imageio_common.h b/src/imageio/imageio_common.h index 2be498ce1269..da9b90fc7024 100644 --- a/src/imageio/imageio_common.h +++ b/src/imageio/imageio_common.h @@ -1,6 +1,6 @@ /* This file is part of darktable, - Copyright (C) 2009-2023 darktable developers. + Copyright (C) 2009-2024 darktable developers. darktable is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -87,7 +87,7 @@ dt_imageio_retval_t dt_imageio_open_exotic(dt_image_t *img, const char *filename struct dt_imageio_module_format_t; struct dt_imageio_module_data_t; -int dt_imageio_export(const dt_imgid_t imgid, +gboolean dt_imageio_export(const dt_imgid_t imgid, const char *filename, struct dt_imageio_module_format_t *format, struct dt_imageio_module_data_t *format_params, @@ -104,7 +104,7 @@ int dt_imageio_export(const dt_imgid_t imgid, const int total, dt_export_metadata_t *metadata); -int dt_imageio_export_with_flags(const dt_imgid_t imgid, const char *filename, +gboolean dt_imageio_export_with_flags(const dt_imgid_t imgid, const char *filename, struct dt_imageio_module_format_t *format, struct dt_imageio_module_data_t *format_params, const gboolean ignore_exif, @@ -158,7 +158,7 @@ void dt_imageio_flip_buffers_ui8_to_float(float *out, const dt_image_orientation_t orientation); // allocate buffer and return 0 on success along with largest jpg thumbnail from raw. -int dt_imageio_large_thumbnail(const char *filename, +gboolean dt_imageio_large_thumbnail(const char *filename, uint8_t **buffer, int32_t *width, int32_t *height, diff --git a/src/iop/demosaic.c b/src/iop/demosaic.c index ddde62c9c386..81c8243623d1 100644 --- a/src/iop/demosaic.c +++ b/src/iop/demosaic.c @@ -208,7 +208,7 @@ typedef struct dt_iop_demosaic_data_t static gboolean get_thumb_quality(int width, int height) { // we check if we need ultra-high quality thumbnail for this size - const int level = dt_mipmap_cache_get_matching_size(darktable.mipmap_cache, width, height); + const dt_mipmap_size_t level = dt_mipmap_cache_get_matching_size(darktable.mipmap_cache, width, height); const char *min = dt_conf_get_string_const("plugins/lighttable/thumbnail_hq_min_level"); const dt_mipmap_size_t min_s = dt_mipmap_cache_get_min_mip_from_pref(min); diff --git a/src/iop/highlights.c b/src/iop/highlights.c index 84527775e45b..2651bd047213 100644 --- a/src/iop/highlights.c +++ b/src/iop/highlights.c @@ -718,7 +718,7 @@ void process(struct dt_iop_module_t *self, gboolean high_quality = TRUE; if(piece->pipe->type & DT_DEV_PIXELPIPE_THUMBNAIL) { - const int level = dt_mipmap_cache_get_matching_size(darktable.mipmap_cache, piece->pipe->final_width, piece->pipe->final_height); + const dt_mipmap_size_t level = dt_mipmap_cache_get_matching_size(darktable.mipmap_cache, piece->pipe->final_width, piece->pipe->final_height); const char *min = dt_conf_get_string_const("plugins/lighttable/thumbnail_hq_min_level"); const dt_mipmap_size_t min_s = dt_mipmap_cache_get_min_mip_from_pref(min); high_quality = (level >= min_s); diff --git a/src/views/view.c b/src/views/view.c index d6bdafd195b5..9cf40a93a8a7 100644 --- a/src/views/view.c +++ b/src/views/view.c @@ -1,6 +1,6 @@ /* This file is part of darktable, - Copyright (C) 2009-2023 darktable developers. + Copyright (C) 2009-2024 darktable developers. darktable is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -709,8 +709,8 @@ void dt_view_set_scrollbar(dt_view_t *view, } dt_view_surface_value_t dt_view_image_get_surface(const dt_imgid_t imgid, - const int width, - const int height, + const int32_t width, + const int32_t height, cairo_surface_t **surface, const gboolean quality) { @@ -726,15 +726,20 @@ dt_view_surface_value_t dt_view_image_get_surface(const dt_imgid_t imgid, // get mipmap cache image dt_mipmap_cache_t *cache = darktable.mipmap_cache; - dt_mipmap_size_t mip = dt_mipmap_cache_get_matching_size(cache, - width * darktable.gui->ppd, - height * darktable.gui->ppd); + const int32_t mipwidth = width * darktable.gui->ppd; + const int32_t mipheight = height * darktable.gui->ppd; + dt_mipmap_size_t mip = dt_mipmap_cache_get_matching_size(cache, mipwidth, mipheight); // if needed, we load the mimap buffer dt_mipmap_buffer_t buf; dt_mipmap_cache_get(cache, &buf, imgid, mip, DT_MIPMAP_BEST_EFFORT, 'r'); - const int buf_wd = buf.width; - const int buf_ht = buf.height; + + const int32_t buf_wd = buf.width; + const int32_t buf_ht = buf.height; + + dt_print(DT_DEBUG_LIGHTTABLE, + "dt_view_image_get_surface id %i, dots %ix%i -> mip %ix%i, found %ix%i\n", + imgid, mipwidth, mipheight, cache->max_width[mip], cache->max_height[mip], buf_wd, buf_ht); // if we don't get buffer, no image is awailable at the moment if(!buf.buf) @@ -746,8 +751,8 @@ dt_view_surface_value_t dt_view_image_get_surface(const dt_imgid_t imgid, // so we create a new image surface to return float scale = fminf(width / (float)buf_wd, height / (float)buf_ht) * darktable.gui->ppd_thb; - const int img_width = roundf(buf_wd * scale); - const int img_height = roundf(buf_ht * scale); + const int32_t img_width = roundf(buf_wd * scale); + const int32_t img_height = roundf(buf_ht * scale); // due to the forced rounding above, we need to recompute scaling scale = fmaxf(img_width / (float)buf_wd, img_height / (float)buf_ht); *surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, img_width, img_height); @@ -783,14 +788,14 @@ dt_view_surface_value_t dt_view_image_get_surface(const dt_imgid_t imgid, have_lock = FALSE; if(buf.color_space == DT_COLORSPACE_NONE) { - fprintf(stderr, + dt_print(DT_DEBUG_ALWAYS, "oops, there seems to be a code path not setting the" " color space of thumbnails!\n"); } else if(buf.color_space != DT_COLORSPACE_DISPLAY && buf.color_space != DT_COLORSPACE_DISPLAY2) { - fprintf(stderr, + dt_print(DT_DEBUG_ALWAYS, "oops, there seems to be a code path setting an" " unhandled color space of thumbnails (%s)!\n", dt_colorspaces_get_name(buf.color_space, "from file")); @@ -887,14 +892,11 @@ dt_view_surface_value_t dt_view_image_get_surface(const dt_imgid_t imgid, // logs if(darktable.unmuted & DT_DEBUG_PERF) dt_print(DT_DEBUG_LIGHTTABLE | DT_DEBUG_PERF, - "[dt_view_image_get_surface] id %i, dots %ix%i, mip %ix%i," - " surf %ix%i created in %0.04f sec\n", - imgid, width, height, buf_wd, buf_ht, + "got surface %ix%i created in %0.04f sec\n", img_width, img_height, dt_get_wtime() - tt); else dt_print(DT_DEBUG_LIGHTTABLE, - "[dt_view_image_get_surface] id %i, dots %ix%i, mip %ix%i, surf %ix%i\n", - imgid, width, height, buf_wd, buf_ht, img_width, img_height); + "got surface %ix%i\n", img_width, img_height); // we consider skull as ok as the image hasn't to be reload return ret;