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

Invalidate image cache when modified on disk #703

Closed
1 task done
fxha opened this issue Mar 1, 2019 · 13 comments · Fixed by #2875
Closed
1 task done

Invalidate image cache when modified on disk #703

fxha opened this issue Mar 1, 2019 · 13 comments · Fixed by #2875

Comments

@fxha
Copy link
Contributor

fxha commented Mar 1, 2019

Description

An image is not correctly reloaded when the image is modified on disk.

  • Can you reproduce the issue?

Steps to reproduce

  1. Create image: ![](a.png)
  2. Change a.png on disk

Expected behavior:

The modified image is displayed.

Actual behavior:

The image is not reloaded because it's fetched from cache.

Versions

  • Mark Text: all
  • Operating system: all
@Jocs
Copy link
Member

Jocs commented Mar 1, 2019

Change a.png on disk

Do you mean change the name of a.png or just replace it to another image with the same name?

@fxha
Copy link
Contributor Author

fxha commented Mar 1, 2019

Do you mean change the name of a.png or just replace it to another image with the same name?

Just modify the existing image. E.g. change the color from red to black.

@Jocs
Copy link
Member

Jocs commented Mar 1, 2019

Is this issue the same with #656 ?

@fxha
Copy link
Contributor Author

fxha commented Mar 1, 2019

I don't think so because we add all images to an "image cache", so we don't have to make continuous http requests but local files may be modified. I think the issue is that we add local files (file://%path%) to the cache as well.

@Jocs
Copy link
Member

Jocs commented Mar 1, 2019

After listening to (after we'll do) the local image modification, you also need to inform the editor to reload the markdown file. At this same time, we also need to clear the image cache to ensure the image is loaded?

@fabiospampinato
Copy link

@fxha Isn't Electron just caching file:// urls on its own? 🤔

@fxha
Copy link
Contributor Author

fxha commented Jan 30, 2020

Isn't Electron just caching file:// urls on its own?

@fabiospampinato I don't think so because it's only locally, so it wouldn't make any sense, but Chromium may prevent to reload the image because the URL is the same as before on the same HTML element and we may need to add a semi-random suffix like ?r=0. However, the main issue is that we don't listen for image changes on disk.

@fabiospampinato
Copy link

but Chromium may prevent to reload the image because the URL is the same as before on the same HTML element and we may need to add a semi-random suffix like ?r=0

That's what I meant, i.e. the following:

I think the issue is that we add local files (file://%path%) to the cache as well.

I don't think is correct, you don't need to do anything to get this behavior.

@fxha
Copy link
Contributor Author

fxha commented Jan 30, 2020

I think the issue is that we add local files (file://%path%) to the cache as well.

I don't think is correct, you don't need to do anything to get this behavior.

If I remember correctly, we're using a map/cache that maps paths/URL's to images for better render performance, so we can reuse the old DOM element instead making another HTTP request. But I'm not totally sure and currently don't have any time to invest the issue.

@fabiospampinato Do you have a similar issue in Notable? That would mean Chromium does caching.

@fabiospampinato
Copy link

If I remember correctly, we're using a map/cache that maps paths/URL's to images for better render performance, so we can reuse the old DOM element instead making another HTTP request.

That's interesting, does that help performance a lot? 🤔

@fabiospampinato Do you have a similar issue in Notable? That would mean Chromium does caching.

Yes, and I'm not doing any kind of manual caching.

@kiyoka
Copy link
Contributor

kiyoka commented Dec 31, 2021

I think it is possible to use a periodic timer to check the update time of image files.

@rugk
Copy link

rugk commented Dec 31, 2021

Same issue in #2320

@kiyoka
Copy link
Contributor

kiyoka commented Jan 3, 2022

I have experimented with this issue and found it to be quite difficult.
It seems that Chromium caches the same URL even if I disable the caching of the loadImageMap variable.

kiyoka added a commit to kiyoka/marktext that referenced this issue Jan 10, 2022
This is PoC code.
Since it is difficult to automatically detect updates to image files, the F5 key is used to refresh and redisplay the image.
kiyoka added a commit to kiyoka/marktext that referenced this issue Jan 10, 2022
define keybindings for 'Reload Images'.
kiyoka added a commit to kiyoka/marktext that referenced this issue Jan 10, 2022
Only local files were invalidated.
kiyoka added a commit to kiyoka/marktext that referenced this issue Jan 12, 2022
Reload images in local files with F5 key.
(No file update detection)
kiyoka added a commit to kiyoka/marktext that referenced this issue Jan 15, 2022
On macOS, the Command+R key was duplicated, so I changed the key for view.dev-reload to Command+Alt+R.
kiyoka added a commit to kiyoka/marktext that referenced this issue Jan 22, 2022
Applied the review remarks in the pull request.
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 a pull request may close this issue.

5 participants