Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Conversation

@Adlai-Holler
Copy link
Contributor

@nguyenhuy @appleguy Experimenting with Kittens shows that when you enlarge the image this still correctly updates the table view, but when you swap the image and text it correctly does not bother the table view.

  • Change delegate method nodeDidRelayout: to nodeDidRelayoutWithSizeChange: to filter layouts that don't affect height.
  • In ASTableView, ASCollectionView, enqueue beginUpdates()/endUpdates() in the run loop. These calls can be expensive – querying 100 nodes for their heights and enqueuing an animation – so this should be worth the effort.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest leaving this naming as didRelayoutNode:, as it can be used by other future layout invalidations other than size changes. If you need specifics for your current usage of the API, might I suggest adding a boolean flag: didRelayoutNode:sizeChange:.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's way better.

@nguyenhuy
Copy link
Contributor

@Adlai-Holler Thanks for bringing this up! It's absolutely true that after a relayout that doesn't result in a size change, the container view (i.e table view or collection view) doesn't need to be bothered. The reason it is still notified was that I wanted to make use of built-in reload animations (fb18e76). But after we replaced reloads with empty transactions in 5701cbb, that reason is no longer valid! Unnecessary empty transactions are inefficient. Sorry for triggering them in the first place :)

@Adlai-Holler
Copy link
Contributor Author

Ah! That makes sense @nguyenhuy. To be clear, this hasn't become a practical issue for me yet but I do it so I can sleep better =)

appleguy added a commit that referenced this pull request Dec 4, 2015
Reduce Frequency of beginUpdates/endUpdates Due to Node Relayout
@appleguy appleguy merged commit 3a04cb7 into facebookarchive:master Dec 4, 2015
@appleguy
Copy link
Contributor

appleguy commented Dec 4, 2015

Let us all achieve deeper sleep in contemplation of new efficiencies. <3

@Adlai-Holler Adlai-Holler deleted the OptimizeNodeDidRelayout branch December 4, 2015 05:03
@Adlai-Holler
Copy link
Contributor Author

Amen. 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants