FixTweet icon indicating copy to clipboard operation
FixTweet copied to clipboard

Mosaic always puts 2 images side by side?

Open Deer-Spangle opened this issue 1 year ago • 14 comments

When images are wider than they are tall, it seems like it would be better for the mosaic to stack them on top of each other, rather than putting them side by side?

An example: https://twitter.com/oryxspioenkop/status/1562177315529973761

Not sure whether to file this against the mosaic package though, I can move it over there if you prefer

Deer-Spangle avatar Aug 23 '22 21:08 Deer-Spangle

I think this is related, but this one doesn't embed in Telegram. It has 3 wide images https://fxtwitter.com/emollick/status/1562143652180365323 I would guess that the 3 side by side images means it ends up so much wider than it is tall, that telegram won't embed? From my experiments, I think the maximum ratio is 10:1 in width:height, or vice versa.

I might be totally wrong, and that embed might be failing for a totally different reason though. (I'm not even 100% confident that 3 image tweets all go side by side)

Deer-Spangle avatar Aug 23 '22 22:08 Deer-Spangle

Yeah, the current method is not great for some images, especially on Telegram as image embeds are compressed so zooming in won’t give you full quality. Maybe we could do it like vxTwitter:

Screenshot

0xallie avatar Aug 24 '22 10:08 0xallie

Yeah, personally, I'm not super fond of the blurry background thing it does, and I usually like the clean edges of the fxtwitter mosaic, but it might be nice if it was a bit smarter about arrangements.

Maybe it's time I tried to learn some Rust!

Deer-Spangle avatar Aug 24 '22 10:08 Deer-Spangle

Yeah, it usually works well with images with similar aspect ratios but it struggles in cases like these. Which is something I didn't take into account when I wrote my original mosaic reference, and it persisted into the rust rewrite. What do you think @Antonio32A?

dangeredwolf avatar Aug 25 '22 07:08 dangeredwolf

I've not browsed through the current mosaic implementation to be honest, as I've got effectively no experience with rust. I would be happy to help out if you want, would maybe give me a chance to poke at rust and learn a little bit.

I was thinking through the problem this morning, and how it would be best solved, and I threw together the start of an almost pseudo-code python implementation on the train this morning. 2022-08-26-mosaic.py.txt (It is very much not runnable code in current state, it was thrown together in a text editor, it's missing most the arrangement implementations, and it's missing a bunch of type hints, and some default args in method implementations.)

It's very OOP though, with the idea of having abstract Mosaic, and then abstract classes for each count of images, and concrete implementations for each arrangement. When given a list of images, it creates a mosaic object for each arrangement, and checks which is closest to a square, and builds that as an image fitting the max dimensions.

The combinatorics get a little tricky, on how many arrangement implementations are needed, but luckily Twitter only goes up to 4 images. So there's these arrangements:

  • 1 image: (1) Trivial
  • 2 images: (2) LeftRight, TopBottom
  • 3 images: (6) Row, TopTopBottom, LeftRightRight, LeftLeftRight, TopBottomBottom, Column
  • 4 images: (14) 4Row, 4Col, 22Rows, 22Cols, 31Rows, 13Rows, 31Cols, 13Cols, 211Rows, 211Cols, 121Rows, 121Cols, 112Rows, 112Cols (Not the best naming on my part, but hopefully gets the idea across)

I'm not sure whether there's some better algorithm, or how that one would be best implemented in Rust

Deer-Spangle avatar Aug 25 '22 09:08 Deer-Spangle

It shouldn't be too hard to change the algorithm, especially if you're only changing the part which handles 2 images. You just have to make sure to edit it here and here. If you want you don't even have to write rust since somebody else here (probably me) can just port the size calculations from typescript to rust.

Regarding the complete algorithm rewrite: I do think this is a good idea, but I'm not sure how doable is it due to the pure amount of combinations. You also have to consider the fact that it needs to be heavily tested and that both the typescript and rust versions need to match each other. While writing a few tests isn't really hard, you still have to somehow gather the images which match all 23 combinations.

Antonio32A avatar Aug 25 '22 10:08 Antonio32A

both the typescript and rust versions need to match each other

Oh? Are both used? I had thought that the rust implementation replaced the typescript one.

Some tests would certainly be wise! And wouldn't be too hard, could just generate solid colour images and feed them in, no need to actually find tweets.

Deer-Spangle avatar Aug 25 '22 10:08 Deer-Spangle

Currently rust does the heavy work of downloading, resizing and actually creating the image. Typescript is only used for calculating the size because we need to know that information when we send the embed on Discord/Telegram. Sadly this means that we have to implement the same algorithm in both languages.

You could I guess just use random Twitter images as well, both work well.

Antonio32A avatar Aug 25 '22 11:08 Antonio32A

I was doing some additional testing @Antonio32A and I found that if we just don't give height and width values to Discord at all it renders okay (which I was kinda surprised by), unlike before where mismatched heights/widths would cause wonky behavior. Do you want to just scrap the TypeScript implementation and just tweak the Rust to make it handle multi-image better? It does mean I'll be removing Mosaic height and width from the API, but hopefully that is not an issue for anyone.

dangeredwolf avatar Aug 28 '22 18:08 dangeredwolf

I'd say yeah, if you're sure that it works on desktop, web, android and ios go for it. What about telegram?

Antonio32A avatar Aug 28 '22 19:08 Antonio32A

On Telegram Desktop it doesn't seem to affect it either @Antonio32A. I'll test on mobile as well and if so I might just say we drop it just to make both of our lives easier and not having to maintain 2 mosaic implementations.

dangeredwolf avatar Aug 28 '22 19:08 dangeredwolf

I've written most the implementation for this on my fork now, by the way! Just trying to untangle some Rust things as this is my first time writing any. I've been chatting with various rust furries on telegram trying to get some pointers to fixing that up, so hopefully shouldn't be long, hopefully!

I've not started on tests yet, though ^_^;

I threw together a quick ugly diagram of all the arrangements too, to try and clarify which are which, and to better see what shapes I would need for testing: image They should all be read as Red, Blue, Green, Purple. I'm a bit unsure about the 4th one along on the 3rd row: image which doesn't seem like it reads in any particular order, and so maybe I should omit that mosaic arrangement?

Deer-Spangle avatar Aug 31 '22 13:08 Deer-Spangle

ooh this looks really cool! I'll be looking forward to trying out your fork if you implement some of those @Deer-Spangle

dangeredwolf avatar Aug 31 '22 20:08 dangeredwolf

It should all be implemented now! It seems to compile, and I've thrown in 21 tests (Running the tests will create a directory called "mosaic_tests" and populate it with example mosaics which should look like this: image

I actually removed the three 3-column options for 4-column mosaics too, as they seemed not very readable also. I changed the scaling, so that it will always try and avoid scaling any image down, scaling the other images to match it, unless that takes it over the maximum image size. (I believe before, if the second image was bigger than the first, it would scale it down to match it, rather than scale the first up) I've changed the mosaic selection logic too, so that it actually initially selects the best mosaic based on how similar the scaling factors are between images*, and then selects the most square mosaic that has a similar scaling factor ratio.

(*While writing tests, I discovered that if you passed it 4 images: 100x100, 300x100, 300x100, 100x100, that it would rather scale up the first image 700% and put it above the other three, making a 700x800 mosaic, rather than putting them into two rows, keeping the scaling the same, and making a 400x200 image. So I added this first check of scaling factor ratios!)

But yup, should be all good for testing if you want to spin up a copy? I'll make a PR in the meanwhile

Deer-Spangle avatar Sep 01 '22 08:09 Deer-Spangle