Enable partial repaint for iOS/Metal #28801

knopp · 2021-09-22T15:23:53Z

This enables partial redraw for Metal on iOS. The PR itself is quite big, but it's organized in smaller commits.

List which issues are fixed by this PR:

flutter/flutter#33939 (partially)

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

iskakaushik · 2021-09-26T18:56:19Z

flow/compositor_context.cc

+
+  std::optional<SkRect> clip_rect;
+
+  if (frame_damage && layer_tree.root_layer()) {


Consider making this a method of its own: std::optional<SkRect> ComputeClipRect(...).

iskakaushik · 2021-09-26T18:57:33Z

flow/compositor_context.cc

+
+    frame_damage->damage_out =
+        context.ComputeDamage(frame_damage->additional_damage);
+    clip_rect = SkRect::Make(frame_damage->damage_out.buffer_damage);


It will be good to log a trace event counter with the percentage of screen repainted. This will let users have the ability to see how their app behaves w.r.t partial redraw.

iskakaushik · 2021-09-26T19:02:51Z

flow/compositor_context.cc

@@ -85,9 +118,16 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
  if (post_preroll_result == PostPrerollResult::kSkipAndRetryFrame) {
    return RasterStatus::kSkipAndRetry;
  }
+
+  SkAutoCanvasRestore restore(canvas(), clip_rect.has_value());


why do we need this?

We don't if we always start with "fresh" canvas. Which seems to be the case right now for some reason, I'm not quite sure what has changed. I used to have the issue where canvas()->clipRect(*clip_rect); was making the rect smaller and smaller every frame until nothing was painted. Restoring the clip after rasterization fixed that of course.

It doesn't seem to be needed right now. Still I don't think removing the clip rect after rasterization is a bad thing.

flow/surface_frame.h

shell/gpu/gpu_surface_metal.mm

iskakaushik · 2021-09-26T19:21:57Z

flow/raster_cache.h

+  // used for this frame in order to not get evicted. This is needed during
+  // partial repaint for layers that are outside of current clip and are culled
+  // away.
+  void Touch(SkPicture* picture, const SkMatrix& transformation_matrix);


Can you please add tests for this behavior?

Have a frame where each of these layers is culled away but raster cache still has them. Can be in rasterizer_unittests.cc.

shell/common/rasterizer.cc

flow/surface_frame.h

iskakaushik · 2021-09-26T19:51:38Z

shell/gpu/gpu_surface_metal.h

@@ -37,6 +36,10 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetal : public Surface {
  // external view embedder is present.
  bool render_to_surface_;

+  // Accumulated damage for each framebuffer; Key is address of underlying
+  // MTLTexture for each drawable
+  std::map<uintptr_t, SkIRect> damage_;


If you were to create a class for this say MetalFrameDamage, which contains this map and has a couple of methods:

typedef uintptr_t MetalTextureId

AccumulateDamage(MetalTextureId texture_to_exclude)

ResetDamage(MetalTextureId texture)

std::optional<SkRect> GetDamage(MetalTextureId texture)

This would help us unit test these methods better and make it more readable IMO.

iskakaushik · 2021-10-12T19:32:44Z

Need to look into this, but there are some artifacts with the following example: https://gist.github.com/iskakaushik/219d9b16cc94bbaa8560fdb9361096f1

knopp · 2021-10-12T23:34:44Z

So, this is what is happening now: During diffing we only detects the rotating layer and repaint its bounds. The container ImageFilterLayer has same filter, so diffing assumes that it didn't change and doesn't repaint it. Only the area covered by rotating layer gets repainted.

The problem is that the rotating layer affects area outside of its bounds, because it get blurred by ImageFilterLayer. We don't currently have a good way to represent something like this. Closest to that we have a readback region, which is an area, which gets repainted whole if anything within this area changes. This is used to repaint BackdropFilterLayer, mostly because it's very difficult to repaint partial blur (because at the edges of clip we sample from outside of the clip).

We used to add readback region for ImageFilterLayer as well. This ensured that if anything changed in the area of ImageFilterLayer, the whole ImageFilterLayer got repainted. This should always give correct result, at the expense of possible unnecessary repaints in some situations.

After long discussion we decided to remove this here. It was not entirely correct decision though as this sample demonstrates (actually, removing the read back was perhaps correct thing to do, but not a complete solution). Repainting the entire ImageFilterLayer through readback region might be an overkill, but will give correct results.

I think ideal solution would be to detect the dirty area within the ImageFilterLayer and inflate it by ImageFilterLayer filter bounds. I'll try implementing this tomorrow - it should hopefully fix the issue without having to use readback region.

knopp · 2021-10-13T16:02:58Z

I added a prototype fix for ImageFilterLayer. Assuming this is the right way to go I'll submit a proper pull request just for the fix (with tests).

What this does is that every paint bounds of child layers within ImageFilterLayer are adjusted according to the filter. So if layer within ImageFilterLayer is dirty, we repaint the correct (inflated) region instead of just the unfiltered region.

Filter adjustments are stacked so this should work for nested filters as well.

This should preserve current behavior - the lifespan of raster cache entries should be identical with and without partial repaint.

This is necessary to not rasterize layers that will not be painted anyway.

iskakaushik

LGTM, there need to be some tests added for the new raster cache behavior, but these can be done as part of a follow up PR.

zanderso · 2021-11-02T15:46:42Z

Great to see this land. Thanks for the hard work on this @knopp @iskakaushik @flar!

eggfly · 2022-01-01T02:52:45Z

The ignore_raster_cache params is used.

ignore_raster_cache on CompositorContext::ScopedFrame::Raster seems unused. flutter#88323

wangying3426 · 2022-01-05T13:12:50Z

Good, any plan for mac/windows?

fzyzcjy · 2022-06-12T14:07:56Z

Hi, I see an interesting figure at official release https://medium.com/flutter/whats-new-in-flutter-2-10-5aafb0314b12

So may I know where I can find it? Thanks

flutter-dashboard bot added embedder Related to the embedder API platform-android platform-fuchsia platform-ios labels Sep 22, 2021

google-cla bot added the cla: yes label Sep 22, 2021

knopp force-pushed the partial_update_metal branch from ddea980 to c53e7fa Compare September 22, 2021 15:28

knopp requested a review from iskakaushik September 22, 2021 15:32

iskakaushik reviewed Sep 26, 2021

View reviewed changes

knopp force-pushed the partial_update_metal branch 3 times, most recently from aa45f17 to 9f4ebe0 Compare October 4, 2021 19:31

knopp mentioned this pull request Oct 7, 2021

WIP: Wiring required for partial update #28798

Closed

8 tasks

knopp force-pushed the partial_update_metal branch from 9f4ebe0 to 072069b Compare October 12, 2021 17:13

iskakaushik removed platform-android platform-fuchsia labels Oct 12, 2021

knopp changed the title ~~Enable partial repaint for iOS/Metal~~ WIP: Enable partial repaint for iOS/Metal Oct 13, 2021

flutter-dashboard bot added platform-android platform-fuchsia labels Oct 13, 2021

chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Oct 21, 2021

knopp force-pushed the partial_update_metal branch 3 times, most recently from 9260218 to cddf409 Compare October 29, 2021 21:57

knopp added 5 commits November 1, 2021 22:35

Unconditionally enable DiffContext

9278145

Do not evict culled-out layers from raster cache

5979a3c

This should preserve current behavior - the lifespan of raster cache entries should be identical with and without partial repaint.

Add FrameDamage to CompositorContext

b17f6cb

Add FramebufferInfo and SubmitInfo to SurfaceFrame

5a1a775

Flip surface dimensions

22dab17

knopp added 9 commits November 1, 2021 22:35

Enable partial repaint for Metal

2dd78fb

Disable partial redraw if external view embedder is involved

b4c9c34

Fix test

63acb30

Use single SkIRect for damage

2fc2710

Nit

62a93ea

Add CompositorContext::ScopedFrame::ComputeClipRect

c42dddf

Move ComputeClipRect to FrameDamage

787444a

Use clip rect for culling during preroll

fafe849

This is necessary to not rasterize layers that will not be painted anyway.

Check for null filter

1702715

knopp force-pushed the partial_update_metal branch from cddf409 to 1702715 Compare November 1, 2021 21:49

fix tests

3f59ea9

knopp force-pushed the partial_update_metal branch from 512b251 to 3f59ea9 Compare November 1, 2021 22:21

knopp changed the title ~~WIP: Enable partial repaint for iOS/Metal~~ Enable partial repaint for iOS/Metal Nov 1, 2021

knopp removed the Work in progress (WIP) Not ready (yet) for review! label Nov 2, 2021

iskakaushik approved these changes Nov 2, 2021

View reviewed changes

iskakaushik merged commit 4dbdc08 into flutter:master Nov 2, 2021

engine-flutter-autoroll mentioned this pull request Nov 2, 2021

Roll Engine from 6095a1367846 to 4dbdc0844425 (1 revision) flutter/flutter#92918

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 2, 2021

4dbdc08 Enable partial repaint for iOS/Metal (flutter/engine#28801)

3cc5542

engine-flutter-autoroll mentioned this pull request Nov 2, 2021

Roll Engine from 6095a1367846 to 11b553bfb297 (3 revisions) flutter/flutter#92941

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 2, 2021

4dbdc08 Enable partial repaint for iOS/Metal (flutter/engine#28801)

bf5e963

engine-flutter-autoroll mentioned this pull request Nov 3, 2021

Roll Engine from 6095a1367846 to f08694a57857 (15 revisions) flutter/flutter#92977

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2021

4dbdc08 Enable partial repaint for iOS/Metal (flutter/engine#28801)

1148d45

jmagman mentioned this pull request Nov 17, 2021

Popup menu items remain on screen after dismissing flutter/flutter#93758

Closed

dxvid-pts mentioned this pull request Nov 19, 2021

Blinking cursor should use less CPU flutter/flutter#59327

Open

jiyuujin mentioned this pull request Feb 4, 2022

2022/02/04 で喋ったこと Vol.183 jiyuujin/ohayo-developers#78

Open

xvrh mentioned this pull request Feb 18, 2022

[ios][regression] Visual glitch when scrolling a list in a Scaffold that has a Material and Container as bottomNavigationBar flutter/flutter#98739

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable partial repaint for iOS/Metal #28801

Enable partial repaint for iOS/Metal #28801

knopp commented Sep 22, 2021 •

edited

Loading

iskakaushik Sep 26, 2021

iskakaushik Sep 26, 2021

iskakaushik Sep 26, 2021

knopp Oct 4, 2021

iskakaushik Sep 26, 2021

iskakaushik Sep 26, 2021

iskakaushik commented Oct 12, 2021

knopp commented Oct 12, 2021 •

edited

Loading

knopp commented Oct 13, 2021

iskakaushik left a comment

zanderso commented Nov 2, 2021

eggfly commented Jan 1, 2022 •

edited

Loading

wangying3426 commented Jan 5, 2022

fzyzcjy commented Jun 12, 2022


		std::optional<SkRect> clip_rect;

		if (frame_damage && layer_tree.root_layer()) {

Enable partial repaint for iOS/Metal #28801

Enable partial repaint for iOS/Metal #28801

Conversation

knopp commented Sep 22, 2021 • edited Loading

Pre-launch Checklist

iskakaushik Sep 26, 2021

Choose a reason for hiding this comment

iskakaushik Sep 26, 2021

Choose a reason for hiding this comment

iskakaushik Sep 26, 2021

Choose a reason for hiding this comment

knopp Oct 4, 2021

Choose a reason for hiding this comment

iskakaushik Sep 26, 2021

Choose a reason for hiding this comment

iskakaushik Sep 26, 2021

Choose a reason for hiding this comment

iskakaushik commented Oct 12, 2021

knopp commented Oct 12, 2021 • edited Loading

knopp commented Oct 13, 2021

iskakaushik left a comment

Choose a reason for hiding this comment

zanderso commented Nov 2, 2021

eggfly commented Jan 1, 2022 • edited Loading

wangying3426 commented Jan 5, 2022

fzyzcjy commented Jun 12, 2022

knopp commented Sep 22, 2021 •

edited

Loading

knopp commented Oct 12, 2021 •

edited

Loading

eggfly commented Jan 1, 2022 •

edited

Loading