Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: storing thumbnails on disk #201

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ebea027
pixmap: Add saving and loading to a file
Rentib Oct 18, 2024
920d258
Add thumbnail caching to image
Rentib Oct 18, 2024
b6d6986
refactor: extract getting thumbnail path to another function
Rentib Oct 19, 2024
afd395c
refactor: extract creating directories to another function
Rentib Oct 19, 2024
9a023e2
docs: add comments to get_image_thumb_path() and make_directories()
Rentib Oct 19, 2024
e38918e
fix: use fixed size integers
Rentib Oct 19, 2024
44db2d0
fix: make_directories() now returns false for NULL and empty path
Rentib Oct 19, 2024
919a8c9
fix: conditional include of <linux/limits>
Rentib Oct 19, 2024
87377e9
fix: remove array size from color[] in pixmap_save()
Rentib Oct 19, 2024
43dcd14
refactor: change PATH_MAX to sizeof(path)
Rentib Oct 19, 2024
d840a87
fix: change sizeof() of pointer to PATH_MAX
Rentib Oct 19, 2024
9b80899
feat: update cached thumbnails based on ctime
Rentib Oct 19, 2024
06b19ca
feat: enable turning caching thumbnails on disk on/off
Rentib Oct 19, 2024
99c7624
refactor!: extract thumbnail related things to thumbnail.c
Rentib Oct 20, 2024
0d4e79c
feat: separate loading and saving thumbnails from their creation
Rentib Oct 20, 2024
2035a9e
feat: add thumbnail params
Rentib Oct 20, 2024
25b7ab1
feat: recreate thumbnail if metadata changed
Rentib Oct 20, 2024
65f1ec4
chore: remove resolved NOTE comment
Rentib Oct 20, 2024
74b46a1
docs: fixed comment
Rentib Oct 20, 2024
b3a14f3
changed default to not cache thumbnails on dist
Rentib Oct 22, 2024
a975462
fixed get_thumb_path()
Rentib Oct 23, 2024
41a8a06
fix(thumbnail.c): Handle io errors.
Rentib Oct 24, 2024
de506dc
moved expand_path() to config.h header
Rentib Oct 24, 2024
37dde1a
chore: remove unused includes
Rentib Oct 26, 2024
7716523
add fallback for XDG_CACHE_HOME
Rentib Oct 26, 2024
88f2d10
fix design of make_directories()
Rentib Oct 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ sources = [
'src/memdata.c',
'src/pixmap.c',
'src/sway.c',
'src/thumbnail.c',
'src/ui.c',
'src/viewer.c',
'src/formats/bmp.c',
Expand Down
8 changes: 1 addition & 7 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,7 @@ static struct config* get_section(struct config* cfg, const char* name)
return NULL;
}

/**
* Expand path from environment variable.
* @param prefix_env path prefix (var name)
* @param postfix constant postfix
* @return allocated buffer with path, caller should free it after use
*/
static char* expand_path(const char* prefix_env, const char* postfix)
char* expand_path(const char* prefix_env, const char* postfix)
{
char* path;
const char* prefix;
Expand Down
8 changes: 8 additions & 0 deletions src/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,11 @@ void config_error_key(const char* section, const char* key);
* @param value configuration parameters
*/
void config_error_val(const char* section, const char* value);

/**
* Expand path from environment variable.
* @param prefix_env path prefix (var name)
* @param postfix constant postfix
* @return allocated buffer with path, caller should free it after use
*/
char* expand_path(const char* prefix_env, const char* postfix);
18 changes: 17 additions & 1 deletion src/gallery.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "imagelist.h"
#include "info.h"
#include "loader.h"
#include "thumbnail.h"
#include "ui.h"

#include <stdlib.h>
Expand Down Expand Up @@ -72,13 +73,28 @@ static struct gallery ctx;
static void add_thumbnail(struct image* image)
{
struct thumbnail* entry = malloc(sizeof(*entry));
struct pixmap thumb;
struct thumbnail_params params;

/* TODO: move to config */
bool thumbnails_disk_cache = false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be a part of the static context for the thumbnail entity.


if (!entry) {
image_free(image);
} else {
entry->width = image->frames[0].pm.width;
entry->height = image->frames[0].pm.height;
entry->image = image;
image_thumbnail(image, ctx.thumb_size, ctx.thumb_fill, ctx.thumb_aa);
thumbnail_params(&params, image, ctx.thumb_size, ctx.thumb_fill,
ctx.thumb_aa);
if (!thumbnails_disk_cache ||
!thumbnail_load(&thumb, image->source, &params)) {
thumbnail_create(&thumb, image, &params);
if (thumbnails_disk_cache) {
thumbnail_save(&thumb, image->source, &params);
}
}
image_thumbnail(image, &thumb);
ctx.thumbs = list_append(ctx.thumbs, entry);
}
}
Expand Down
37 changes: 2 additions & 35 deletions src/image.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,47 +51,14 @@ void image_rotate(struct image* ctx, size_t angle)
}
}

void image_thumbnail(struct image* image, size_t size, bool fill,
bool antialias)
void image_thumbnail(struct image* image, struct pixmap* thumbnail)
{
struct pixmap thumb;
struct image_frame* frame;
const struct pixmap* full = &image->frames[0].pm;
const float scale_width = 1.0 / ((float)full->width / size);
const float scale_height = 1.0 / ((float)full->height / size);
const float scale =
fill ? max(scale_width, scale_height) : min(scale_width, scale_height);
size_t thumb_width = scale * full->width;
size_t thumb_height = scale * full->height;
ssize_t offset_x, offset_y;
enum pixmap_scale scaler;

if (antialias) {
scaler = (scale > 1.0) ? pixmap_bicubic : pixmap_average;
} else {
scaler = pixmap_nearest;
}

if (fill) {
offset_x = size / 2 - thumb_width / 2;
offset_y = size / 2 - thumb_height / 2;
thumb_width = size;
thumb_height = size;
} else {
offset_x = 0;
offset_y = 0;
}

// create thumbnail
if (!pixmap_create(&thumb, thumb_width, thumb_height)) {
return;
}
pixmap_scale(scaler, full, &thumb, offset_x, offset_y, scale, image->alpha);

image_free_frames(image);
frame = image_create_frames(image, 1);
if (frame) {
frame->pm = thumb;
frame->pm = *thumbnail;
}
}

Expand Down
7 changes: 2 additions & 5 deletions src/image.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,9 @@ void image_rotate(struct image* ctx, size_t angle);
/**
* Create thumbnail from full size image.
* @param image original image
* @param size thumbnail size in pixels
* @param fill thumbnail scale mode (fill/fit)
* @param antialias use antialiasing
* @param thumbnail pixmap with thumbnail of the image
*/
void image_thumbnail(struct image* image, size_t size, bool fill,
bool antialias);
void image_thumbnail(struct image* image, struct pixmap *thumbnail);

/**
* Set image format description.
Expand Down
238 changes: 238 additions & 0 deletions src/thumbnail.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
// SPDX-License-Identifier: MIT
// Image instance: pixel data and meta info.
// Copyright (C) 2021 Artem Senichev <[email protected]>
// Copyright (C) 2024 Rentib <[email protected]>

#include "thumbnail.h"

#include "config.h"
#include "image.h"
#include "memdata.h"
#include "pixmap.h"

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <time.h>
#include <unistd.h>

/**
* Makes directories like `mkdir -p`.
* @param path absolute path to the directory to be created, should not be null
* or empty
* @return true if successful
*/
static bool make_directories(const char* path)
{
char* path_copy = str_dup(path, NULL); // maybe use [PATH_MAX] buffer
char* slash = path_copy;

while (true) {
slash = strchr(slash + 1, '/');
Rentib marked this conversation as resolved.
Show resolved Hide resolved
if (!slash) {
break;
}
*slash = '\0';
if (mkdir(path_copy, 0755) && errno != EEXIST) {
free(path_copy);
return false;
}
*slash = '/';
}

free(path_copy);
return true;
}

static bool get_thumb_path(char** path, const char* source)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static chat* get_thumb_path(const char* source) would be clearer.
Bu the way, source can contain prefix exec:// or starts with ../../../../../../etc/passwd, so which path you finally create?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this is a problem. I will fix it to not pass source, but the absolute file path.

{
const char* prefix = "XDG_CACHE_HOME";
Rentib marked this conversation as resolved.
Show resolved Hide resolved
const char* postfix = "/swayimg";
const char* prefix_fallback = "HOME";
const char* postfix_fallback = "/.cache/swayimg";
if ((!(*path = expand_path(prefix, postfix)) &&
!(*path = expand_path(prefix_fallback, postfix_fallback))) ||
!(*path = str_append(source, 0, path))) {
return false;
}
return true;
}

void thumbnail_params(struct thumbnail_params* params,
const struct image* image, size_t size, bool fill,
bool antialias)
{
const struct pixmap* full = &image->frames[0].pm;
const float scale_width = 1.0 / ((float)full->width / size);
const float scale_height = 1.0 / ((float)full->height / size);
const float scale =
fill ? max(scale_width, scale_height) : min(scale_width, scale_height);
size_t thumb_width = scale * full->width;
size_t thumb_height = scale * full->height;
ssize_t offset_x, offset_y;

if (fill) {
offset_x = size / 2 - thumb_width / 2;
offset_y = size / 2 - thumb_height / 2;
thumb_width = size;
thumb_height = size;
} else {
offset_x = 0;
offset_y = 0;
}

*params = (struct thumbnail_params) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to create temporary instance of thumbnail_params on the stack, it is easier to directly set parameters for params.

.thumb_width = thumb_width,
.thumb_height = thumb_height,
.offset_x = offset_x,
.offset_y = offset_y,
.fill = fill,
.antialias = antialias,
.scale = scale,
};
}

bool thumbnail_save(const struct pixmap* thumb, const char* source,
const struct thumbnail_params* params)
{
FILE* fp = NULL;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useless initialization, it looks like this:

FILE* fp;
fp = NULL;
fp = fopen(...

uint32_t i;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This var is used only in the for loop, you can reduce the scope of this variable.
  2. Use size_t for counters.

char* path = NULL;

if (!get_thumb_path(&path, source)) {
goto fail;
}

if (!make_directories(path)) {
goto fail;
}

if (!(fp = fopen(path, "wb"))) {
goto fail;
}

if (fprintf(fp, "P6\n%zu %zu\n255\n", thumb->width, thumb->height) < 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to use PNM format, then please move load/save function to the pnm.c.
Also, it wild be nice if swayimg can open such images like others PNM.

goto fail;
}

/* comment to store params */
if (fputc('#', fp) == EOF ||
fwrite(params, sizeof(struct thumbnail_params), 1, fp) != 1 ||
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a binary data struct, It may contain many \n and this breaks your format.

fputc('\n', fp) == EOF) {
goto fail;
}

// TODO: add alpha channel
for (i = 0; i < thumb->width * thumb->height; ++i) {
uint8_t color[] = { (((thumb->data[i] >> (8 * 2)) & 0xff)),
(((thumb->data[i] >> (8 * 1)) & 0xff)),
(((thumb->data[i] >> (8 * 0)) & 0xff)) };
fwrite(color, 3, 1, fp);
}

free(path);
fclose(fp);
return true;

fail:
free(path);
if (fp) {
fclose(fp);
}
return false;
}

/**
* Allocate/reallocate pixel map.
* @param thumb pixmap context
* @param path path to load from
* @return true pixmap was loaded
*/
bool thumbnail_load(struct pixmap* thumb, const char* source,
const struct thumbnail_params* params)
{
FILE* fp = NULL;
char* path = NULL;
uint32_t i;
struct stat attr_img, attr_thumb;
struct thumbnail_params saved_params;

if (!get_thumb_path(&path, source) || stat(source, &attr_img) ||
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we have many thumbnail parameters to save/load, so it would be better to have one place for all of them. Lets store the date/time in thumbnail_params.

stat(path, &attr_thumb) ||
difftime(attr_img.st_ctime, attr_thumb.st_ctime) > 0) {
goto fail;
}

if (!(fp = fopen(path, "rb"))) {
goto fail;
}

char header[3];
if (fscanf(fp, "%2s\n%zu %zu\n255\n", header, &thumb->width,
&thumb->height) != 3) {
goto fail;
Rentib marked this conversation as resolved.
Show resolved Hide resolved
}

if (strcmp(header, "P6")) {
goto fail;
}

/* comment with stored params */
if (fgetc(fp) != '#' ||
fread(&saved_params, sizeof(struct thumbnail_params), 1, fp) != 1 ||
fgetc(fp) != '\n') {
goto fail;
}

if (memcmp(params, &saved_params, sizeof(struct thumbnail_params))) {
goto fail;
}

if (!pixmap_create(thumb, thumb->width, thumb->height)) {
goto fail;
}

for (i = 0; i < thumb->width * thumb->height; ++i) {
uint8_t color[3];
if (fread(color, 3, 1, fp) != 1) {
pixmap_free(thumb);
goto fail;
}
thumb->data[i] = ARGB(0xff, color[0], color[1], color[2]);
}

free(path);
fclose(fp);
return true;

fail:
free(path);
if (fp) {
fclose(fp);
}
return false;
}

bool thumbnail_create(struct pixmap* thumb, const struct image* image,
const struct thumbnail_params* params)
{
const struct pixmap* full = &image->frames[0].pm;
enum pixmap_scale scaler;

if (params->antialias) {
scaler = (params->scale > 1.0) ? pixmap_bicubic : pixmap_average;
} else {
scaler = pixmap_nearest;
}

// create thumbnail
if (!pixmap_create(thumb, params->thumb_width, params->thumb_height)) {
return false;
}
pixmap_scale(scaler, full, thumb, params->offset_x, params->offset_y,
params->scale, image->alpha);

return true;
}
Loading
Loading