currently the autoreload feature of nsxiv is a bit unreliable because we
try to load at the very first event we received. however, the writer
might not be done writing and so we might try to load a truncated image
(and fail).
in the following ascii diagram, function S represents sleep and `+` sign
represents writes by the writer. because we set the sleep (of 10ms) at
the first event, subsequent writes by the writer doesn't influence our
reload logic:
S(10) load()
nsxiv | |
writer + + + + (done)
time(ms): 00 05 10 15
after this patch, (assuming function T (re)sets a timeout), we will keep
(re)setting a timeout on new events giving the writer more time to
finish:
T(10) T(10) T(10) T(10) load()
nsxiv | | | | |
writer + + + + (done)
time(ms): 00 05 10 15 20 25
while this patch makes things significantly more robust, the problem
here is inherently unsolvable since there's no way to tell whether the
writer is done writing or not.
for example, if user does something like `curl 'some.png' > test.png`
then curl might stop for a second or two in the middle of writing due to
internet issues - which will make nsxiv drop the image.
this patch also increases the autoreload delay from 10ms to now 128ms
instead to decrease chances of false failures.
ref: 6ae2df6ed5
partially-fixes: https://codeberg.org/nsxiv/nsxiv/issues/456
commit-message-by: NRK
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/459
Reviewed-by: eylles <eylles@noreply.codeberg.org>
these document improvements and/or removal of unnecessary code for when
we will require a higher minimum version of Imlib2.
all the comments have been prefixed with "UPGRADE: " for easy grepping.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/457
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
a lot of application which allow selecting features at build time seem
to output the build config with `--version` or similar (e.g ffmpeg).
aside from giving the user information about the feature set the binary
was compiled with (in case the user didn't compile it themselves, e.g on
a binary distro) it can also (possibly) help when submitting bug
reports.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/462
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
gif spec [0] doesn't mention what to do when "Delay Time" is 0.
apng spec [1] states:
| If the the value of the numerator is 0 the decoder should render the
| next frame as quickly as possible, though viewers may impose a
| reasonable lower bound.
webp spec [2]:
| the interpretation of frame duration of 0 (and often <= 10) is
| implementation defined.
so it seems that it's safe to set a default delay for 0 delay frames,
which is what the older gif and webp loaders were already doing. do the
same for the imlib2 multi-frame loader as well.
[0]: https://www.w3.org/Graphics/GIF/spec-gif89a.txt
[1]: https://wiki.mozilla.org/APNG_Specification
[2]: https://developers.google.com/speed/webp/docs/riff_container#animation
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/445
Reviewed-by: eylles <eylles@noreply.codeberg.org>
the multiframe loaders sets the color modifier to NULL but doesn't
restore it before returning. this results in a imlib2 developer warning
if you try to change brightness/contrast on a multiframe image (which
doesn't have alpha).
ensure that the color modifier is restored before returning under all
paths.
Reported-by: Madhu
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/440
Reviewed-by: eylles <eylles@noreply.codeberg.org>
by default imlib2 doesn't check the file's timestamp to avoid disk
activity when loading from cache. however, this ends up breaking our
autoreload functionality on multi-frame images.
the reason why single frame images weren't broken was because
`img_load()` calls `imlib_image_set_changes_on_disk()`, which tells
imlib2 to check the timestamp before loading from cache.
do the same thing for the multi-frame loader as well.
additionally add a comment to img_load() explaining what's going on.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/436
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/437
Reviewed-by: eylles <eylles@noreply.codeberg.org>
now that we have long-opts, we don't have to worry about exhausting the
alphabet list for short-opts. so adding a cli flag to set/unset the
checker background makes sense.
ref: https://codeberg.org/nsxiv/nsxiv/issues/404
this will be a massive change compared to the usual stuff. however the
gains will be worth it:
* we gain lots of additional animated image support.
* and we'll gain _even_ more format support as imlib2 adds them, without needing
any change in our code-base.
* about ~300 LoC will be purged once we remove our internal gif and webp loader.
as for when to remove the internal loaders, a good time might be when debian
upgrades their imlib2, currently it seems to be at v1.7.5, which doesn't support
animated images.
as of now, nsxiv will continue to build with the internal gif/webp loaders
(assuming they were enabled in config.mk) if imlib2 version is below 1.8.0 and
will print out a deprecation notice.
and if imlib2 version supports multi-frame then it will simply ignore the
internal loaders and use the imlib2 one.
in other words, users shouldn't need to do anything on their side. everything
that previously functioned will continue to function regardless of the user's
imlib2 version (though they might see the annoying deprecation notice if the
imlib2 version doesn't support multi-frame images).
known issue:
* image loading performance can be noticeably worse in
imlib2 versions below 1.9.0
Closes: https://codeberg.org/nsxiv/nsxiv/issues/301
Closes: https://codeberg.org/nsxiv/nsxiv/issues/300
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/373
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
* Imlib2 supports modifying gamma, brightness and contrast directly
while sxiv only supports gamma. Makes sense to extend it to brightness
and contrast as well.
* Since color corrections need to be aware of each other, they have been
refactored into one centralized function.
* This also makes the code more hackable as it makes it easier to add
more color correction functions without them interfering with each
other.
Co-authored-by: 0ion9 <finticemo@gmail.com>
Co-authored-by: NRK <nrk@disroot.org>
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/396
Reviewed-by: NRK <nrk@disroot.org>
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
Co-authored-by: Berke Kocaoğlu <kberke@metu.edu.tr>
Co-committed-by: Berke Kocaoğlu <kberke@metu.edu.tr>
the length member needed to be zero-ed before we started decoding.
this was causing unintended behavior of playing an animated webp longer
than it should under slideshow mode.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/381
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
the way `imlib_load_image()` works, is that it only does a lightweight
signature/metadata check. it doesn't actually decode the image. which
means that a file that has valid metadata but invalid content would get
loaded successfully.
`imlib_image_get_data_for_reading_only()` basically forces imlib to
decode the data, and thus reveal any malformed images so we can reject
it (see commit f0266187).
however, this is a spurious way of achieving the goal at hand. imlib2
already offers an `_immediately` variant which decodes the data
immediately. so just use that instead of spuriously using the "get_data"
function to force a decode.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/380
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
* ensure static variables comes after non-static ones
* remove depreciated DATA32 type
* prefer `sizeof(expression)` over `sizeof(Type)`.
* silence a -Wsign warning
* {gif,webp} loader: use a pointer to reduce code-noise
* gif loader: allocate in one place
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/374
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
simply running nsxiv with `--anti-alias` will enable anti-aliasing, and
running it with `--anti-alias=no` will disable it.
the cli flag will overwrite the config.h default.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/349
_SC_PHYS_PAGES isn't POSIX and might not be defined. in such case, just
return back `CACHE_SIZE_FALLBACK`.
NOTE: POSIX says the `names` in `sysconf()` are "symbolic constants" not
necessarily macros. So we might end up returning the fallback in some
cases where `_SC_PHYS_PAGES` *was* available, but not defined as a
macro. which is not ideal, but nothing fatal.
in practice, this shouldn't be an issue since most systems seems to
define them to be macros, i've checked Glibc, Musl, OpenBSD, FreeBSD and
Haiku.
also add a (useful) comment on `config.h` describing the effect higher
cache size has.
Closes: https://codeberg.org/nsxiv/nsxiv/issues/354
mixing signed and unsigned types in comparison can end up having
unintended results. for example:
if (-1 < 1U)
printf("true\n");
else
printf("false\n");
previously we silenced these warnings, instead just fix them properly
via necessary casting, and in cases where the value cannot be negative
(e.g width/height members) make them unsigned.
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/336
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
the following combination currently fails to build:
$ make HAVE_LIBFONTS=0 HAVE_LIBWEBP=1
this is probably because one of the font library was including <stdio.h>
Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/337
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
Reviewed-by: TAAPArthur <taaparthur@noreply.codeberg.org>
Co-authored-by: NRK <nrk@disroot.org>
Co-committed-by: NRK <nrk@disroot.org>
* rm unused include <sys/types.h>
* move <sys/time.h> to main.c, it's the only file that needs it.
* move TV_* macros to main.c
* let *.c files explicitly include what they need instead of including
them at nsxiv.h
the warnings on r_readdir(), img_load_gif() and strcpy seems to be false
positives. the warning about fmt being unused is valid, but not worth
fixing with additional #ifdef guards.
use `assert` to silence the false positive cases when possible,
otherwise use a NOLINT comment with an explanation.
rather than calling the script unconditionally per redraw, we now have
a `title_dirty` flag and keep track of when any of the relavent
information changes.
Co-authored-by: Arthur Williams <taaparthur@gmail.com>
Partially fixes: https://github.com/nsxiv/nsxiv/issues/258
fixes all -Wshadow related warnings (on gcc). this would allow us to use
`-Wshadow` in github workflow (https://github.com/nsxiv/nsxiv/pull/195).
i've thought about adding `-Wshadow` to our Makefile as well, but
decided against it to keep the Makefile CFLAGS barebore/minimal.
since Imlib2 v1.7.5, it is capable of parsing exif data on jpeg files
and auto orienting them. this caused nsxiv to rotate the image twice.
Closes: https://github.com/nsxiv/nsxiv/issues/187
The problem:
1. For the most people imlib2's default 4MiB is unreasonably low;
2. Hardcoding cache size to ~256MiB has performance benefits and doesn't
increase RAM usage too much on relatively modern systems;
3. But we can't do that, because that would be detrimental to low spec systems
that (apparently) not (?) building nsxiv from source, as been discussed
#171
Solution:
Calculate cache size based on total memory.
Default is set as 3%, which means:
* ~245MiB for 8GiB
* ~30MiB for 1GiB
* and so on
CACHE_SIZE_LIMIT (256MiB by default) sets the highest possible value. And in
case we cannot determine the total memory (e.g since _SC_PHYS_PAGES isn't
POSIX) use CACHE_SIZE_FALLBACK (32MiB by default) instead.
Co-authored-by: NRK <nrk@disroot.org>
now that imlib2 (v1.7.5) is able to load the first frame of an
animated-webp file, we no longer need the `is_webp` check to bypass
imlib2.
ref: https://phab.enlightenment.org/T8964
* fix compile error
* use variable instead of macro
* Revert "use variable instead of macro"
This reverts commit a14ef0b231c50e49906761010a4d4231ce4e3e36.
* use local variable instead of macro
* Revert "use local variable instead of macro"
This reverts commit 7e049d55d94f5c003d90e1a10187356f6a7f54b2.
When rendering images, destination image width and height may become
zero due to float-to-int conversion after zoom calculation, later
crashing when creating an image using those dimensions. This sets
a minimum of 1 to both variables.
Closes#82
with this patch certain gif images will fail to play. one other problem
here is that it suddenly breaks the loop without free-ing data and rows,
leading to a memory leak.
regardless, this needs to be investigated further.
here's an example image where this happens:
https://i.postimg.cc/SQf1TJJg/awoo-study.gif
This reverts commit cca7834e67.
by default imlib2 uses a 4mb cache, which is quite small. this allows
users who have more memory to spare to set a bigger cache size and avoid
reloading an already viewed image if it fits into the cache.
Co-authored-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>
the goal here to mark functions and variables not used outside the
translation unit as static. main reason for this is cleanliness. however
as a side-effect this can help compilers optimize better as it now has
guarantee that a certain function won't be called outside of that
translation unit.
one other side-effect of this is that accessing these vars/function from
config.h is now different.
if one wants to access a static var/func from different translation unit
in config.h, he would have to create a wrapper function under the right
ifdef. for static functions one would also need to forward declare it.
here's a dummy example of accessing the function `run_key_handler` from
config.h under _MAPPINGS_CONFIG
```
static void run_key_handler(const char *, unsigned);
bool send_with_ctrl(arg_t key) {
run_key_handler(XKeysymToString(key), ControlMask);
return false;
}
```
* tns_clean_cache: remove unused function arg
* remove malloc casting
* improve consistency
use sizeof(T) at the end
* avoid comparing integers of different signedness
* use Window type for embed and parent
* remove unnecessary comparisons
* remove cpp style comments
* improve consistency: remove comma from the end of enumerator list
* Removed useless _IMAGE_CONFIG defines
* consistency: use the same order as snprintf
* Resolve c89 warnings
Co-authored-by: uidops <uidops@protonmail.com>
Co-authored-by: Arthur Williams <taaparthur@gmail.com>
this code snippet was introduced in
2703809a23
but does not seem to be needed.
from what i can tell this is some sort of hack that might've been needed
back when we didn't bypass imlib2 when loading webp.
David Gowers
NRK
Berke Kocaoğlu
explosion-mental
N-R-K
Nick Hanley
MahouShoujoMivutilde
mamg22
eylles
