Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Changes from 1 commit
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
02556ae
++
Jul 11, 2024
b3f6670
move all state updates to end of compositing.
Jul 11, 2024
b31aae6
working but unsynchronized.
Jul 11, 2024
75f931a
hey it almost works.
Jul 12, 2024
4525bd8
disable partial repaint.
Jul 12, 2024
6ce3aa0
cleanups.
Jul 12, 2024
be03cf8
set presentsWithTransaction.
Jul 12, 2024
6770376
sanity refactor.
Jul 12, 2024
912cd40
linting.
Jul 12, 2024
1f65cc3
skip post message if there are no platform views.
Jul 12, 2024
53adc81
linting.
Jul 12, 2024
870b950
dispose views in callback.
Jul 15, 2024
f8267bd
++
Jul 15, 2024
80565d5
Merge branch 'main' of github.com:flutter/engine into three_phase_render
Jul 16, 2024
2b9a825
tear down.
Jul 16, 2024
0a7dc5a
reserve.
Jul 17, 2024
2745e88
++
Jul 17, 2024
6002aa0
++
Jul 17, 2024
e7a832f
++
Jul 17, 2024
f099d74
come on big money no whammies.
Jul 17, 2024
e2e6a2a
okay one more.
Jul 17, 2024
44d5b5b
debug printf
Jul 17, 2024
b53063a
++
Jul 19, 2024
9a84539
make test work.
Jul 23, 2024
65a5210
++
Jul 23, 2024
735ce22
testing fixes.
Jul 24, 2024
a4f523b
Merge branch 'main' of github.com:flutter/engine into three_phase_render
Jul 24, 2024
b6c3345
merge in view slicer.
Jul 24, 2024
a4b771e
Merge branch 'main' of github.com:flutter/engine into three_phase_render
Jul 24, 2024
cd8e1de
++
Jul 25, 2024
6603d18
minor cleanups.
Jul 25, 2024
27c0daa
formatting.
Jul 25, 2024
d794211
add back clear
Jul 26, 2024
2f4314b
++
Jul 26, 2024
25a4e06
++
Jul 26, 2024
ea2e66d
merge threads on SIM.
Jul 26, 2024
d220885
++
Jul 26, 2024
9b534cd
Update shell/platform/darwin/ios/ios_external_view_embedder.mm
Jul 26, 2024
970e13a
++
Jul 26, 2024
63ebe66
Merge branch 'three_phase_render' of github.com:jonahwilliams/engine …
Jul 26, 2024
ff4802a
Merge branch 'main' of github.com:flutter/engine into three_phase_render
Jul 26, 2024
e3fc731
does it blend?
Jul 26, 2024
c0fab3b
++
Jul 26, 2024
d73671e
test fixes and clang tidy.
Jul 27, 2024
7285907
simplify and clean up thread access for UIViews.
Jul 27, 2024
bcd8519
reset before clearning composition order.
Jul 27, 2024
b25a742
Merge branch 'main' of github.com:flutter/engine into three_phase_render
Jul 30, 2024
5f6a8d4
switch to SurfaceFrame API.
Jul 30, 2024
be7853d
++
Jul 30, 2024
b19b4d8
Remove extra include.
Jul 30, 2024
ab08739
clang tidy
Jul 30, 2024
6e6e5a2
bracken feedback
Jul 30, 2024
355ab30
more bracken review.
Aug 1, 2024
8cd1c74
++
Aug 1, 2024
30e7b97
review comments and remove dead code.
Aug 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
more bracken review.
  • Loading branch information
jonahwilliams committed Aug 1, 2024
commit 355ab30a68c4fb35c2d2ac8bfacaa0a11a7b81bc
58 changes: 27 additions & 31 deletions shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
FLUTTER_ASSERT_ARC

#ifdef FML_OS_IOS_SIMULATOR
// Note: this is an arbitrary number that attempts to account for cases
// where the platform view might be momentarily off the screen.
// The number of frames the rasterizer task runner will continue
// to run on the platform thread after no platform view is rendered.
//
// Note: this is an arbitrary number.
static const int kDefaultMergedLeaseDuration = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

ask to learn - are we still merging the thread (given this variable name?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are merging threads on the iOS simulator (technically we only need to when using the software backend but that is a harder condition to test). Once we remove skia and the software backend we can remove this condition.

Copy link
Contributor

Choose a reason for hiding this comment

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

May I ask what's the difference between the simulator and device that we have to merge the thread on simulator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its not the simulator, its the software backend. The software backend surfaces cannot be used from multiple threads, which this change requires.

#endif // FML_OS_IOS_SIMULATOR

Expand Down Expand Up @@ -105,6 +107,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
void FlutterPlatformViewLayerPool::CreateLayer(GrDirectContext* gr_context,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably the biggest PITA. Because I can only construct these on the platform thread, right now frame 0 with platform views will skip some number of overlay layers. Its possible that the FlutterMetalLayer solves this partially

Copy link
Member

@knopp knopp Jul 12, 2024

Choose a reason for hiding this comment

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

If we move away from CAMetalLayer (or even FlutterMetalLayer) and adopt custom composition, we can solve this easily because you can allocate IOSurfaces on any thread, when they are not tied to particular CALayer. We can also have much cheaper overlays. Right now we use up to 2 CAMetaLayers for each FlutterView overlay, which is 6 full screen surfaces. With custom composition it is possible to set a single iOSurface as content of as many CALayers as needed each showing different part, so you can many unobstructed platform view overlays with single layer.

On macOS surfaces are allocated on raster thread and the overlay sublayers are crated here.

Here are relevant files:

https://github.com/flutter/engine/blob/main/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm

https://github.com/flutter/engine/blob/main/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm

const std::shared_ptr<IOSContext>& ios_context,
MTLPixelFormat pixel_format) {
FML_DCHECK([[NSThread currentThread] isMainThread]);
std::shared_ptr<FlutterPlatformViewLayer> layer;
fml::scoped_nsobject<UIView> overlay_view;
fml::scoped_nsobject<UIView> overlay_view_wrapper;
Expand Down Expand Up @@ -266,12 +269,13 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
ChildClippingView* clipping_view = [[ChildClippingView alloc] initWithFrame:CGRectZero];
[clipping_view addSubview:touch_interceptor];

platform_views_[viewId] = PlatformViewData{
.view = fml::scoped_nsobject<NSObject<FlutterPlatformView>>(embedded_view), //
.touch_interceptor =
fml::scoped_nsobject<FlutterTouchInterceptingView>(touch_interceptor), //
.root_view = fml::scoped_nsobject<UIView>(clipping_view) //
};
platform_views_.emplace(
Copy link
Contributor

Choose a reason for hiding this comment

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

nice clean up with the struct

viewId, PlatformViewData{
.view = fml::scoped_nsobject<NSObject<FlutterPlatformView>>(embedded_view), //
.touch_interceptor =
fml::scoped_nsobject<FlutterTouchInterceptingView>(touch_interceptor), //
.root_view = fml::scoped_nsobject<UIView>(clipping_view) //
});

result(nil);
}
Expand Down Expand Up @@ -643,6 +647,9 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
}

void FlutterPlatformViewsController::Reset() {
// Reset will only be called from the raster thread or a merged raster/platform thread.
// platform_views_ must only be modified on the platform thread, and any operations that
// read or modify platform views should occur there.
fml::TaskRunner::RunNowOrPostTask(
platform_task_runner_, [&, composition_order = composition_order_]() {
for (int64_t view_id : composition_order_) {
Expand All @@ -664,6 +671,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
std::unique_ptr<SurfaceFrame> background_frame) {
TRACE_EVENT0("flutter", "FlutterPlatformViewsController::SubmitFrame");

// No platform views to render; we're done.
if (flutter_view_ == nullptr || (composition_order_.empty() && !had_platform_views_)) {
had_platform_views_ = false;
return background_frame->Submit();
Expand All @@ -673,6 +681,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
bool did_encode = true;
LayersMap platform_view_layers;
std::vector<std::unique_ptr<SurfaceFrame>> surface_frames;
surface_frames.reserve(composition_order_.size());
std::unordered_map<int64_t, SkRect> view_rects;

for (int64_t view_id : composition_order_) {
Expand All @@ -694,9 +703,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
// If there are not sufficient overlay layers, we must construct them on the platform
// thread, at least until we've refactored iOS surface creation to use IOSurfaces
// instead of CALayers.
if (required_overlay_layers > layer_pool_->size()) {
CreateMissingOverlays(gr_context, ios_context, required_overlay_layers);
}
CreateMissingOverlays(gr_context, ios_context, required_overlay_layers);

int64_t overlay_id = 0;
for (int64_t view_id : composition_order_) {
Expand Down Expand Up @@ -775,24 +782,15 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
size_t required_overlay_layers) {
TRACE_EVENT0("flutter", "FlutterPlatformViewsController::CreateMissingLayers");

auto missing_layer_count = required_overlay_layers - layer_pool_->size();

// If raster thread is merged because we're in a unit test or running on simulator, then
// we don't need to post a task to create an overlay layer.
if ([[NSThread currentThread] isMainThread]) {
// Create Missing Layers
for (auto i = 0u; i < missing_layer_count; i++) {
CreateLayer(gr_context, //
ios_context, //
((FlutterView*)flutter_view_.get()).pixelFormat //
);
}
if (required_overlay_layers <= layer_pool_->size()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like the layer_pool is accessed here on raster thread, but mutated below in CreateLayer on main thread (if im reading it right). I'm a bit concerned about race condition here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we block the raster thread on completion via a latch, there is no race

return;
}
auto missing_layer_count = required_overlay_layers - layer_pool_->size();

// If the raster thread isn't merged, create layers on the platform thread and block until
// complete.
auto latch = std::make_shared<fml::CountDownLatch>(1u);
platform_task_runner_->PostTask([&]() {
// Create Missing Layers
fml::TaskRunner::RunNowOrPostTask(platform_task_runner_, [&]() {
for (auto i = 0u; i < missing_layer_count; i++) {
CreateLayer(gr_context, //
ios_context, //
Expand All @@ -801,7 +799,9 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
}
latch->CountDown();
});
latch->Wait();
if (![[NSThread currentThread] isMainThread]) {
latch->Wait();
}
}

/// Update the buffers and mutate the platform views in CATransaction on the platform thread.
Expand Down Expand Up @@ -848,9 +848,6 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
// Organize the layers by their z indexes.
BringLayersIntoView(platform_view_layers, composition_order);

// If the frame is submitted with embedded platform views,
// there should be a |[CATransaction begin]| call in this frame prior to all the drawing.
// If that case, we need to commit the transaction.
[CATransaction commit];
}

Expand All @@ -862,8 +859,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,

previous_composition_order_.clear();
NSMutableArray* desired_platform_subviews = [NSMutableArray array];
for (size_t i = 0; i < composition_order.size(); i++) {
int64_t platform_view_id = composition_order[i];
for (int64_t platform_view_id : composition_order) {
UIView* platform_view_root = platform_views_[platform_view_id].root_view.get();
[desired_platform_subviews addObject:platform_view_root];

Expand Down