TokamakUI / Tokamak

SwiftUI-compatible framework for building browser apps with WebAssembly and native apps for other platforms
Apache License 2.0
2.62k stars 111 forks source link

Custom Layout Engine for Fiber Reconciler #472

Closed carson-katri closed 2 years ago

carson-katri commented 2 years ago

This adds a layout pass to the WIP fiber reconciler, that should in theory be able to replicate SwiftUI layout 1:1 as it’s developed.

Screen Shot 2022-02-21 at 4 13 11 PM

In this example, Text is not measured and is a fixed size of 100x50.

Here are some things that are missing, but should be implemented (perhaps as separate PRs targeting this branch):

carson-katri commented 2 years ago

I've made some revisions, and it now seems to work well with simple examples:

https://user-images.githubusercontent.com/13581484/156600586-894ababe-fe06-42ba-a4a5-c23487a55b75.mov

MaxDesiatov commented 2 years ago

Weird to see tests hanging, maybe it's worth restarting them?

carson-katri commented 2 years ago

Hm, still hung...

MaxDesiatov commented 2 years ago

Do these tests hang for you locally as well?

carson-katri commented 2 years ago

Oh, I thought it was one of the tests but its the benchmark that's hanging. It's taking a long time locally as well, so I probably broke something.

carson-katri commented 2 years ago

Tests are passing now! 🎉 For reference, here are the results I get from TokamakCore benchmarks with the layout engine enabled vs. disabled:

Layout Enabled

name                             time      std        iterations
----------------------------------------------------------------
mangledName Runtime               0.002 ms ±  24.29 %     836412
typeConstructorName TokamakCore   0.001 ms ±  16.81 %     965430
update wide (StackReconciler)    46.891 ms ±   1.67 %         30
update wide (FiberReconciler)    33.054 ms ±   1.82 %         42
update narrow (StackReconciler)  46.437 ms ±   2.98 %         30
update narrow (FiberReconciler)  33.150 ms ±   3.61 %         42
update deep (StackReconciler)    17.587 ms ±   1.54 %         80
update deep (FiberReconciler)     6.135 ms ±   3.47 %        227
update shallow (StackReconciler)  8.952 ms ±   1.56 %        156
update shallow (FiberReconciler)  3.854 ms ±   3.17 %        363

Layout Disabled

name                             time      std        iterations
----------------------------------------------------------------
mangledName Runtime               0.002 ms ±  24.98 %     831827
typeConstructorName TokamakCore   0.001 ms ±  23.00 %     975591
update wide (StackReconciler)    49.103 ms ±   3.19 %         28
update wide (FiberReconciler)    21.304 ms ±   5.15 %         68
update narrow (StackReconciler)  48.522 ms ±   3.33 %         28
update narrow (FiberReconciler)  20.052 ms ±   4.09 %         69
update deep (StackReconciler)    18.120 ms ±   2.90 %         76
update deep (FiberReconciler)     6.254 ms ±   5.24 %        220
update shallow (StackReconciler)  8.950 ms ±   2.11 %        154
update shallow (FiberReconciler)  3.726 ms ±   3.41 %        371
carson-katri commented 2 years ago

It's not quite ready for review though, still investigating an issue that I think is caused by ViewModifiers. If you have a layout like this, the view inside the modifier (specifically a modifier that has a DOM representation) won't be laid out:

var body: some View {
  Text("View with modifier") // child of modifier not laid out
    .frame(width: 10, height: 10) // this frame element is laid out
  Text("Sibling View without modifier") // sibling correctly laid out
}

However, a View body like this (with no sibling) works fine:

var body: some View {
  Text("View with modifier (is correctly laid out)") // child of modifier correctly laid out
    .frame(width: 10, height: 10) // frame element laid out
}
carson-katri commented 2 years ago

I would like to add some documentation in this PR, then its process can be more easily reviewed now rather than later

carson-katri commented 2 years ago

This should also resolve #464