Add full-screen ImageViewer widget. Support storing original files and thumbnails in the MediaCache

[aaravlu]

The difference between this pr and #341 is: when a image does not possess a thumbnail uri, this pr would put Original & 400x400 into image viewer while #341 would put Original & File into image viewer.

Note the change has nothing to do with timeline, only image viewer.

[aaravlu]

please restore my feature that returns the MediaFormat of the image that was requested, such that the caller can determine whether the thumbnail or full-size image was returned.

It is almost impossible unless we restore the input value (MediaFormat).

Note here, we just dont know which format we queried:

robrix/src/media_cache.rs

Lines 92 to 94 in 4dc2fae

	MediaCacheEntry::Loaded(_) | MediaCacheEntry::Failed => {
	return er;
	}

But the fact is the function caller doesnot need to know which format he wants.

[Copilot]

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/shared/text_or_image.rs:107

• Changing 'show_text' from public to private might break external usage if it was intended to be part of the public API.

fn show_text<T: AsRef<str>>(&mut self, cx: &mut Cx, text: T) {

src/home/room_screen.rs:3516

• The formatted string uses '{body}' but there is no 'body' variable in scope, which may result in a compile-time error.

format("{body}\n\n[TODO] fetch encrypted image at {:?}", encrypted.url)

src/image_viewer.rs:116

• [nitpick] Consider rephrasing the error message to be clearer (e.g., 'Error loading image: {e}').

log!("Error to load image: {e}");

[Copilot]

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/shared/text_or_image.rs:107

• Changing the visibility of show_text from public to private may break external calls to this method; consider reverting it to 'pub fn' if it is part of the widget's intended public API.

fn show_text<T: AsRef<str>>(&mut self, cx: &mut Cx, text: T) {

src/shared/text_or_image.rs:43

• [nitpick] Leaving the image fit property set to 'Size' with a temporary comment may lead to unexpected display behavior; ensure this is intentionally tested and reverted before final merge.

fit: Size, // Only for a comfortable test, would set back to `Smallest` if this pr OK.

[aaravlu]

@kevinaboos
See aaravlu#2 and it's diff to know why we cant set_image_keys when occupied:

it's a test based on branch fix327-2

[kevinaboos]

@kevinaboos See aaravlu#2 and it's diff to know why we cant set_image_keys when occupied:

it's a test based on branch fix327-2

huh? I don't understand, and I don't think that's correct either.

It looks like it simply cannot be fetched because you're unconditionally overwriting the entire entry in the cache. So if it was already fetched, you've now deleted it....

Setting the relationship between image URI keys (i.e., establishing the relationship that a thumbnail URI and a full-size image URI are referring to the same image) is completely independent of setting the actual cached content of those images.

[kevinaboos]

At this point, I think it's best if we move on from this debate, as it has been ongoing for far too long. I will gladly accept your contributions for the ImageViewer widget and the design of the actions there (since those are now all good and correct), but the cache design simply isn't right, and that's an objective fact, not just my opinion.

I would recommend that you select another topic to work on. I will make the necessary changes to the remainder of this PR and merge it in on my own time later.

[aaravlu]

I will make the necessary changes to the remainder of this PR and merge it in on my own time later.

Thanks, it's supposed to be my job, but now it really bothering you.

because you're unconditionally overwriting the entire entry in the cache

Yeah, Sorry for my incorrect explanation.

It just keeps being overwrited rather than not fetched.

I would recommend that you select another topic to work on.

I am working on #396 recently

I think it's best if we move on from this debate

Fully agree.

[aaravlu]

@kevinaboos

Please rebase this pr's commit messages to one before you merge this pr.
Thanks.

[kevinaboos]

this is blocked on me, so i'll add the waiting-on-review label to remind myself. Will be a while before I get to it, though.

[kevinaboos]

This is still waiting on me, so let's leave the waiting-on-review label in place. However, it'll be a while before I can get to it, probably early June.