Skip to content

Port the media layout algorithm from the Android app#992

Open
grishka wants to merge 10 commits into
mastodon:developfrom
grishka:media-layout
Open

Port the media layout algorithm from the Android app#992
grishka wants to merge 10 commits into
mastodon:developfrom
grishka:media-layout

Conversation

@grishka

@grishka grishka commented Mar 27, 2023

Copy link
Copy Markdown
Member

As discussed, I tried to port the media layout algorithm from the Android app. Android sources: the layout algorithm itself and the custom ViewGroup that actually puts the views on the screen. This new layout algorithm works with up to 10 media attachments.

IMG_0876

I'm not familiar with Swift and UIKit, and definitely not with ReactiveSwift that permeates the entire codebase of the app. I might have done something stupid.

This PR isn't quite ready for merging. There's something inside MediaView that doesn't respect its frame and won't crop vertically, resulting in overlapping views:
IMG_0874
IMG_0875

There's also a bug I don't know how to fix because I don't know where to hook into either the app or UIKit to re-run the layout with new maxSize when the view size changes. Android would automatically measure and then lay out the view subtree when the dimensions of its parent ViewGroup change, but iOS apparently doesn't do that? To reproduce, open an attachment, rotate the device to landscape orientation, then close the attachment.

@CLAassistant

CLAassistant commented Mar 27, 2023

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@grishka

grishka commented Mar 27, 2023

Copy link
Copy Markdown
Member Author

Also, teeny-tiny gifs. I'm definitely missing some crucial piece of understanding of how UIKit views work.

IMG_0877

@j-f1

j-f1 commented Mar 27, 2023

Copy link
Copy Markdown
Contributor

I think you need to call super.layoutSubviews() in your layoutSubviews otherwise the subviews won’t layout their own subviews.

@grishka

grishka commented Mar 27, 2023

Copy link
Copy Markdown
Member Author

Nope, it made no difference =/

Comment thread MastodonSDK/Sources/MastodonUI/Helper/MediaLayoutHelper.swift Outdated
Comment thread MastodonSDK/Sources/MastodonUI/Helper/MediaLayoutHelper.swift Outdated
Comment thread MastodonSDK/Sources/MastodonUI/Helper/MediaLayoutHelper.swift Outdated
MediaLayoutResult.Tile(colSpan: 1, rowSpan: 1, startCol: 2, startRow: 1)
])
} else {
// One on the left, three smaller ones on the right

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean 4 attachments will never be rendered as a 2x2 grid?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's always 1 bigger one and 3 smaller ones

Comment thread MastodonSDK/Sources/MastodonUI/Helper/MediaLayoutHelper.swift Outdated
Comment thread MastodonSDK/Sources/MastodonUI/View/Container/MediaGridContainerView.swift Outdated
Comment thread MastodonSDK/Sources/MastodonUI/View/Container/MediaGridContainerView.swift Outdated
Comment thread MastodonSDK/Sources/MastodonUI/View/Container/MediaGridContainerView.swift Outdated
@grishka

grishka commented Mar 30, 2023

Copy link
Copy Markdown
Member Author

I limited the aspect ratio to 9:20 to avoid weird skinny layouts with images of extreme aspect ratios. I chose 9:20 specifically so that vertical screenshots from modern phones don't end up getting cropped. Modern "bezel-less" phones have 9:18-ish screens.

Simulator Screen Shot - iPhone 14 Pro - 2023-03-30 at 18 56 43

@grishka

grishka commented Apr 2, 2023

Copy link
Copy Markdown
Member Author

Another interesting observation — if a post has a broken layout, but I then tap it to open separately, that screen has a correct layout.

Might it be that images loading asynchronously affect the layout inside MediaView somehow?

@grishka

grishka commented Mar 30, 2024

Copy link
Copy Markdown
Member Author

I decided to take another look at it and remembered that the layout inspector exists. So something is adding these constraints to the UIImageView, making the layout break when I set frames manually. Now it's only a matter of finding what it is.
Снимок экрана 2024-03-30 в 12 50 12

@tvrrp

tvrrp commented Jan 2, 2025

Copy link
Copy Markdown
Contributor

@grishka
Constraints inside MediaView should not affect the size of the MediaView itself.

The behavior where the image extends beyond the bounds occurs because MediaView instances are reused in statusView.mediaGridContainerView.dequeueMediaView. When using AdaptiveLayout, translatesAutoresizingMaskIntoConstraints is disabled, but it is not re-enabled when using GridLayout. This likely causes UIKit to calculate the size of MediaView based on the image dimensions.

You can add view.translatesAutoresizingMaskIntoConstraints = true in MediaGridContainerView.prepareForReuse(). Or, use frame-based layout for AdaptiveLayout as well.

@grishka

grishka commented Jan 2, 2025

Copy link
Copy Markdown
Member Author

Yeah, I figured that out. The problem was that, iirc, when there was a single attachment, view.translatesAutoresizingMaskIntoConstraints = true was being set on it, which I didn't clear, which broke my manual setting of view frames next time when that view got reused. It all works now.

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 this pull request may close these issues.

4 participants