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

update images on modifications #137

Merged
merged 1 commit into from
Mar 30, 2024
Merged

update images on modifications #137

merged 1 commit into from
Mar 30, 2024

Conversation

jonboh
Copy link
Contributor

@jonboh jonboh commented Mar 29, 2024

Currently image.nvim does not have a way to detect that an image changed since the last time it was rendered. This PR should address that case.
I've added added a check for the date:modify property which will clear the image and force a re-render when it has been modified since the last render.
It seems that the initialization of self.resize_hash was duplicated so I've renamed one of them to self.date_hash.
I've modified the cache and tmp files generated, which will now also have the modification date of the file.

Here's the behavior of the master branch right now:
https://github.com/3rd/image.nvim/assets/31407988/c258801f-ac85-491b-a7b3-e2dadcefb0ad
Note that it is necessary to open and close nvim in order for it to render the updated image (I think closing the buffer and reopening also works)

And here the behavior with these changes:
https://github.com/3rd/image.nvim/assets/31407988/c43c1e1f-b658-4d0b-8f8e-b804adfb3b55

@3rd 3rd merged commit c0ecc5d into 3rd:master Mar 30, 2024
1 check passed
3rd added a commit that referenced this pull request Apr 16, 2024
This reverts commit c0ecc5d, reversing
changes made to 13f56f4.
@3rd
Copy link
Owner

3rd commented Apr 16, 2024

@jonboh reverted this because it was killing performance, probably because of the constant magick load, could be worked around by checking the file modification time, exif isn't really relevant anyway

@jonboh
Copy link
Contributor Author

jonboh commented Apr 17, 2024

Ok, do you have a workflow/scenario in which it was specially noticeable? I would like to try and find a fix for both the updates on change and the performance.
A different approach could be to provide the user with a way to refresh all (or a specific) image.

@jonboh
Copy link
Contributor Author

jonboh commented Apr 17, 2024

I've tried using stat -c '%Y' with io.popen to check the date, and with an image on each line there's a noticeable performance drop (even worse than with magick get_property I think).

Maybe it would be ok to add a flag for this update behavior (similar to only_render_image_at_cursor), and to limit it to just upadet the image under the cursor so check_image_changes_under_cursor (otherwise don't check anything, so the current behavior). In this case I don't think the performance hit from loading a single image would be noticeable.
What do you think? I can prepare a PR with these changes if you think that would we acceptable

@jonboh
Copy link
Contributor Author

jonboh commented Apr 17, 2024

I've implemented the change in #150
Let me know if you have any issue with the solution @3rd :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants