microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.71k stars 29.08k forks source link

preview markdown file, image can not reload when modified with other tool #65258

Closed maggiedot closed 3 years ago

maggiedot commented 5 years ago

Issue Type: Bug

preview a markdown file which contains png image, when I modify the image file with mspaint., the preview window does not refresh to show the new picture, even if I set another name and set back.

VS Code version: Code 1.30.0 (c6e592b2b5770e40a98cb9c2715a8ef89aec3d74, 2018-12-11T22:29:11.253Z) OS version: Windows_NT x64 10.0.10240

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (4 x 3192)| |GPU Status|2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
rasterization: enabled
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled| |Memory (System)|7.89GB (1.41GB free)| |Process Argv|C:\Users\xxxx\Desktop\test-docs\README.md| |Screen Reader|no| |VM|0%|
Extensions (1) Extension|Author (truncated)|Version ---|---|--- vscode-language-pack-zh-hans|MS-|1.30.2
vscodebot[bot] commented 5 years ago

(Experimental duplicate detection) Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

FokkoVeegens commented 5 years ago

I can confirm this.

Version: 1.30.2 (user setup) Commit: 61122f88f0bf01e2ac16bdb9e1bc4571755f5bd8 Date: 2019-01-07T22:54:13.295Z Electron: 2.0.12 Chrome: 61.0.3163.100 Node.js: 8.9.3 V8: 6.1.534.41 OS: Windows_NT x64 10.0.17134

Even when using the "Refresh Preview" functionality, it doesn't refresh externally changed images. image

sbstp commented 5 years ago

I have a similar issue, if an external program modifies a markdown file, it won't reload even with the "refresh preview" menu item. To get it to refresh I had to modify the markdown file within code.

tomasaschan commented 4 years ago

This is tagged "help wanted", and I'd like to help solve it if I can. Are there any pointers/guesses on where the problem might lie?

mjbvz commented 4 years ago

Actually considering this only has 3 upvotes since December 2018, let's wait to see if there is enough interest in it from the community. I don't think it would be all that trivial to get right

sbstp commented 4 years ago

Feels more like a bug than a feature for me. The issue is that the image seems to get cached in memory permanently and cannot be refreshed. The refresh preview menu item should at least clear this cache, shouldn't be too hard to implement.

qt1 commented 3 years ago

Same here, The best solution would be to auto detect changed images and auto refresh. Limiting to images in same workspace may reduce load on the change detection service, if that is the issue.

Another solution would be to refresh when the refresh command is activated.

At the moment only closing and re-openning the preview works..

IMHO the different behaviour between the "refresh" command and open-close looks like a bug.

Thanks :)

hediet commented 3 years ago

Actually considering this only has 3 upvotes since December 2018, let's wait to see if there is enough interest in it from the community. I don't think it would be all that trivial to get right

I think (but I might be wrong) it shouldn't be that complex to implement:

The MarkdownEngine could be extended to return not only a string, but also a list of images with their src. It could also be extended to accept some cache information:

interface RenderOutput {
    html: string;
    containingImages: { src: string }[];
}
interface RenderContext {
    /**
     * Each image is identified by its "src".
     * By default, an image has cache key `0`.
     * If the cache key of an image increases between two render calls,
     * the image must be reloaded.
     */
    imageCacheKeyBySrc: Map</* src: */ string, number>;
}
...
class MarkdownEngine {
    ...
    public async render(input: SkinnyTextDocument | string, context: RenderContext): Promise<RenderOutput>
    ...
}

containingImages is used by the MarkdownPreview instance to manage individual file system watchers using createFileSystemWatcher (only for relative images - the absolute file path would need to be computed there). The MarkdownPreview then manages some imageCacheKeyBySrc map. Whenever such a file system watchers triggers, the key of the watched image is increased and the preview is rerendered with the updated imageCacheKeyBySrc map.

The MarkdownEngine uses the image key to invalidate the cache. This is implemented by using a query parameter ala token.setAttr("src", `${src}&cacheKey=${imageCacheKeyBySrc.get(img.src) || 0}`). This must not affect the reported src though.

However, this might conflict with the image size cache so one needs to be careful there.

Can you explain which problem the image size cache solves? I tried to disable the image size cache to see it's effect and created a document with many images loaded from https and things were still very smooth. It seems like the webview itself caches the size already.

What do you think? I'd submit a PR if you are fine with this approach! Thanks 😉

hediet commented 3 years ago

With the risk of wasting time, I couldn't resist to implement it quickly:

e61b881d-2886-4e15-9fa0-e27c05089125

I'll open a PR soon!

isidorn commented 3 years ago

Works great, nice work @hediet Adding verified label