search

flowkey / UIKit-cross-platform

Cross-platform Swift implementation of UIKit, mostly for Android
MIT License
598 stars 40 forks source link

prevent recursion when layouting parent from subview #328

Closed michaelknoch closed 3 years ago

michaelknoch commented 3 years ago

Type of change: performance improvement

Motivation (current vs expected behavior)

I initially introduced this "weird bottom top" layout dependency in this commit https://github.com/flowkey/UIKit-cross-platform/commit/3ed2303e53d514a5b36127483bf3bb73c160cf7b. But I realised that this dependency causes expensive recursive relayouts at least in our player at the moment. We added a check to only relayout if the size has changed, but in our player implementation the size will always change (look at the example below)

It happens that layoutSubviews of the LearnstepNotification gets called a few hundred times initially and during learning again. The recursion is able to stop at some point, but I don't really know why it stops.

layoutSubviews {
    button.frame = container // changes bounds
    button.sizeToFit() // changes bounds again

   ....

   bodyLabel.bounds.size = CGSize(width: container.width, height: 0)
   bodyLabel.sizeToFit()

}

Please check if the PR fulfills these requirements

codecov[bot] commented 3 years ago

Codecov Report

Merging #328 (3874c48) into master (d34725b) will increase coverage by 0.03%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   51.22%   51.25%   +0.03%     
==========================================
  Files          87       87              
  Lines        2899     2901       +2     
==========================================
+ Hits         1485     1487       +2     
  Misses       1414     1414
Impacted Files Coverage Δ
Sources/CALayer.swift 88.05% <100.00%> (-0.09%) :arrow_down:
Sources/UIView.swift 90.72% <100.00%> (+0.14%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d34725b...3874c48. Read the comment docs.

michaelknoch commented 3 years ago

@rikner @ephemer Do you remember why we added this? I remember this fixing an issue with the layout in the MIDI settings view maybe? Not entirely sure.

michaelknoch commented 3 years ago

closed in favour of https://github.com/flowkey/UIKit-cross-platform/pull/329