Allow to "reload" ThunarFiles in a more granular way

Currently, e.g. thunar_thumbnailer_thumbnailer_finished calls thunar_file_reload on all files for which thumbnailing finished successfully.

However, it is actually not required to reload all aspects of the file when it got a thumbnail … it would be sufficient to use a new "thumbnail ready" signal in order to signal subscribers that they need to redraw the file image. (Ideally with the used thumbnail size as an argument)

That is just one use-case ... to be checked if there are others.

added 2. Enhancement 5. Thumbnails labels

mentioned in merge request !408 (closed)

mentioned in merge request !411 (closed)

Tested with thunar_window_queue_redraw from !415 (merged) ... works well instead of thunar_file_reload. So I suppose it really should be fine to send a "thumbnail ready" in thunar-file and the view/widget which has a reference on the ThunarFile could just issue a redraw when received (maybe even throttled).

mentioned in merge request !416 (merged)

mentioned in issue #949 (closed)

mentioned in issue #737 (closed)

A signal "thumbnail-updated" was introduced for ThunarFile by !418 (merged). The tricky part now is, how to transfer this signal up to the view

It's a bit cumbersome, because standard-view holds a ThunarFile, not a ThunarFolder:

• ThunarFolder would need the listen to "thumbnail-updated" of all its ThunarFiles.
• Whenever received for one of its files, ThunarFolder would need to send e.g. "request redraw" on it's corresponding ThunarFile --> So a method to send this signal would need to be exposed to the header of ThunarFile --> not so nice.

This weird architecture could be improved:

• Each ThunarFolder could 'be' a ThunarFile (G_DEFINE_TYPE (ThunarFolder, thunar_folder, THUNAR_TYPE_FILE))
• Creation of new ThunarFiles would be done by some utility method, which automatically either creates a ThunarFolder or a ThunarFile under the hood.
• No need to call thunar_folder_get_for_file anymore, just casting to THUNAR_FOLDER would be sufficient
• current_directory, which is used in many locations could be a ThunarFolder, not a ThunarFile ... which semantically would make a lot of sense.
• It would simplify internal logic and might help to better understand some of the out of sync issues.

Downsides:

• Currently, file-monitoring is enabled by default for each new ThunarFolder. That would probably have a performance impact (E.g. if I open a directory with many subfolders, I don't automatically want a folder-monitor on each subfolder)
  • It would be required to enable monitoring only optionally
• It would be a major change for a very minor (rather aestetical) problem, which might introduce regressions

@Elessar1802, what is your opinion on such a major change for ThunarFolder ? You think it would be worth the trouble ?

My exams are over so I will look into it in detail.

Great, I hope you did well!

I hope you did well!

It went well

The suggested changes could simplify a lot of code but then there would be a need to make many many changes.

I have an idea that might potentially solve the issue. I haven't tried it out yet thought.

• Change icon-renderer column binding from ThunarFile to a string signifying the icon-name or icon-path (thumbnail path).
• The rationale behind the above is that, when we trigger a row_changed on model, a render update should occur on the view but it doesn't since the column binding of IconRenderer is the underlying ThunarFile which remains the same. So behind the scenes GtkCellRenderer is probably caching which causes it to not notice any changes. If we instead changed the binding from ThunarFile to String, it would notice the change and trigger a cell_renderer_render call....

The rationale behind the above is that, when we trigger a row_changed on model, a render update should occur on the view but it doesn't since the column binding of IconRenderer is the underlying ThunarFile which remains the same.

I don't fully understand which binding you want to swap .. though please go aheadand take a try, if you think it makes sense!

(While having the icon-name/thumbnail-path as key probably is a bad idea ... even if the thumbnail path stays the same, the thumbnail might be an updated one)

(While having the icon-name/thumbnail-path as key probably is a bad idea ... even if the thumbnail path stays the same, the thumbnail might be an updated one)

Yeah that would be problem. I didn't think about that.

I have got a patch that should solve the problem of view not redrawing on thumbnail_update. Does it solve the problem?redraw_on_row_change.diff

I have got a patch that should solve the problem of view not redrawing on thumbnail_update. Does it solve the problem?redraw_on_row_change.diff

Since !416 (merged) I actually don't have that issue anymore. So with and without your patch, things work fine for me . This issue is just about optimization.

Hmm, so your patch will emit "file-changed" on the monitor, which then will trigger an action on various widgets which listen to the file-monitor. E.g. it might trigger a folder-reload on thunar_folder_file_changed or loop on all rows in thunar_list_model_file_changed (for some reason that seems not to happen, dont know). As well there is thunar_tree_view_model_file_changed which e.g. checks if resorting is required and calls gtk_tree_model_row_changed on the row.

My point is: I think it makes sense to logically differ between:

• "file changed" aka the file on the disk (or its metadata) changed in some way
  • --> might trigger different expensive actions like reload/resort
• "thumbnail updated" aka only the thumbnail of the file changed
  • --> only should trigger a redraw of the widgets showing the file (its thumbnail).

If we differ between both, it will be easy to prevent necessary reloads. As long as we ue the same signal for both, we never know.

Something different: It looks like we don't need to throttle queue_redraw. According to the doc, it only will invalidate the widget area, in a way that the widget will be redrawn the next time the mainloop goes idle. So there should be no problem to call it multiple times. The widget only will be redrawn once afterward.

Hmm, yes, file-changed will cause a lot of unnecessary sorts and function calls. So having a separate thumbnail-updated definitely makes sense.

Would it make sense if we add a "thumbnail-updated" to file-monitor instead of each ThunarFile ? The logic would be that whenever a file has an updated thumbnail we tell file-monitor to emit "thumbnail-updated". But instead of firing it immediately, we would make file-monitor fire "thumbnail-updated" in batches of 10ms. When eventually file-monitor emits "thumbnail-updated" the signal will have a Glist of the concerning files. This signal can be listened by the view itself or maybe the models.

Would the above be efficient & feasible?

I have put together this patch implementing the above.fix-reload.diff. It's just a WIP atm.

Something different: It looks like we don't need to throttle queue_redraw.

For some reason if I don't throttle it the widget will flicker. It will dim for a few ms. It's noticeable.

Edit: with the the above patch, the flickering does not occur due to the inherent throttling when sending "thumbnail-updated"

Thank you for working on this! I think it goes into the right direction now.

Though still one thing to get right:

1. When using image preview, on some folder which has no XXL-thumbnails yet, clicking some file will not show the preview (only after thumbnails got loaded, though the preview will not update if it was clicked before)

That's probably a direct consequence of thunar file not sending the "thumbnail-updated" siganl anymore. I suppose that would as well break fef2a1a3, since the dialog does not listen to the thunar-file-monitor (and it should not).

I suppose it makes sense to just offer both, the signal on ThunarFile, and the Signal on the FileMonitor

2. If we keep both signals, they should share the same name ... THUMBNAIL_CHANGED or THUMBNAIL_UPDATED both would be fine for me.

For some reason if I don't throttle it the widget will flicker. It will dim for a few ms. It's noticeable.

Meh, not that nice ... then I probably misunderstood the queue redraw mechanics. Ok, then let's throttle.

Edit: One more note: as it looks like we anyhow will keep a thumbnail-update related signal on ThunarFile, it would be better to have ThunarFileMonitor listen to that signal for each of its files, instead of triggering FileMonitor from within ThunarFile (The monitor should depend on ThunarFile, not the other way around). To keep the overall architecture a bit cleaner.

Edit2: Striketrough ... ThunarFileMonitor does not track files

I have opened MR !421 (merged) for the same with a signal - "thumbnail-updated" on file-monitor and keeping "thumbnail-updated" on ThunarFile intact.

mentioned in merge request !421 (merged)

closed with commit 44a5c6f3

changed milestone to %4.19.1