Skip to content

Commit

Permalink
Reverts "[Impeller] Reland: Implement draw order optimization. (flutt…
Browse files Browse the repository at this point in the history
…er#54215)" (flutter#54261)

Reverts: flutter#54215
Initiated by: bdero
Reason for reverting: Causing golden diffs in framework roll https://flutter-gold.skia.org/search?issue=152633&crs=github&patchsets=2&corpus=flutter
Original PR Author: bdero

Reviewed By: {jonahwilliams}

This change reverts the following previous change:
Original PR: flutter#54136
Revert PR: flutter#54067

Includes fixes for issue seen in [these goldens](https://flutter-gold.skia.org/search?issue=152354&crs=github&patchsets=2&corpus=flutter).

The problem was that the scissor was ending up wrong after clip replay for opaque draws that are supposed to occur outside the parent clip scope(s).

To fix it, I made clip replay draws as well as the subpass texture draw apply lazily.
For the clip replay, there's no need to apply a given clip until we come across an entity that has a depth less than or equal to the clip.
And for the subpass texture (which is often translucent), we can defer drawing it until we come across another translucent draw. Deferring the subpass texture is important because if we draw it immediately, then all of the replay clips need to be drawn immediately too.

## Description
For each clip scope, draw opaque items in reverse order and translucent/backdrop-independent items in their original order afterwards. Clips are treated as translucent by the parent scope.

Respects clips, subpass collapse, and the clear color optimization.

### Local new_gallery before/after (iPhone 12 mini):
```
cd ~/projects/flutter/flutter/dev/integration_tests/new_gallery
flutter drive --profile --local-engine-src-path ~/projects/flutter/engine/src --local-engine=ios_profile --local-engine-host=host_profile_arm64 --trace-startup -t test_driver/transitions_perf.dart -d 00008101-000A59A93C10001E
```
![image](https://github.com/user-attachments/assets/7372c128-ca71-44a6-8e6c-b0043025f751)
  • Loading branch information
auto-submit[bot] authored Jul 31, 2024
1 parent 8130df2 commit 6bf9f63
Show file tree
Hide file tree
Showing 15 changed files with 88 additions and 629 deletions.
1 change: 0 additions & 1 deletion ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@
../../../flutter/impeller/entity/contents/host_buffer_unittests.cc
../../../flutter/impeller/entity/contents/test
../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc
../../../flutter/impeller/entity/draw_order_resolver_unittests.cc
../../../flutter/impeller/entity/entity_pass_target_unittests.cc
../../../flutter/impeller/entity/entity_pass_unittests.cc
../../../flutter/impeller/entity/entity_unittests.cc
Expand Down
4 changes: 0 additions & 4 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -42128,8 +42128,6 @@ ORIGIN: ../../../flutter/impeller/entity/contents/tiled_texture_contents.cc + ..
ORIGIN: ../../../flutter/impeller/entity/contents/tiled_texture_contents.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/contents/vertices_contents.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/contents/vertices_contents.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/draw_order_resolver.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/draw_order_resolver.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/entity.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/entity.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/entity/entity_pass.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -45011,8 +45009,6 @@ FILE: ../../../flutter/impeller/entity/contents/tiled_texture_contents.cc
FILE: ../../../flutter/impeller/entity/contents/tiled_texture_contents.h
FILE: ../../../flutter/impeller/entity/contents/vertices_contents.cc
FILE: ../../../flutter/impeller/entity/contents/vertices_contents.h
FILE: ../../../flutter/impeller/entity/draw_order_resolver.cc
FILE: ../../../flutter/impeller/entity/draw_order_resolver.h
FILE: ../../../flutter/impeller/entity/entity.cc
FILE: ../../../flutter/impeller/entity/entity.h
FILE: ../../../flutter/impeller/entity/entity_pass.cc
Expand Down
3 changes: 0 additions & 3 deletions impeller/entity/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ impeller_component("entity") {
"contents/tiled_texture_contents.h",
"contents/vertices_contents.cc",
"contents/vertices_contents.h",
"draw_order_resolver.cc",
"draw_order_resolver.h",
"entity.cc",
"entity.h",
"entity_pass.cc",
Expand Down Expand Up @@ -250,7 +248,6 @@ impeller_component("entity_unittests") {
"contents/filters/matrix_filter_contents_unittests.cc",
"contents/host_buffer_unittests.cc",
"contents/tiled_texture_contents_unittests.cc",
"draw_order_resolver_unittests.cc",
"entity_pass_target_unittests.cc",
"entity_pass_unittests.cc",
"entity_playground.cc",
Expand Down
5 changes: 0 additions & 5 deletions impeller/entity/contents/color_source_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,6 @@ class ColorSourceContents : public Contents {
pass.SetVertexBuffer(std::move(geometry_result.vertex_buffer));
options.primitive_type = geometry_result.type;

// Enable depth writing for all opaque entities in order to allow
// reordering. Opaque entities are coerced to source blending by
// `EntityPass::AddEntity`.
options.depth_write_enabled = options.blend_mode == BlendMode::kSource;

// Take the pre-populated vertex shader uniform struct and set managed
// values.
frame_info.mvp = geometry_result.transform;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,16 +575,16 @@ Entity ApplyBlurStyle(FilterContents::BlurStyle blur_style,
const ContentContext& renderer, const Entity& entity,
RenderPass& pass) mutable {
bool result = true;
blur_entity.SetClipDepth(entity.GetClipDepth());
blur_entity.SetTransform(entity.GetTransform() *
blurred_transform);
result = result && blur_entity.Render(renderer, pass);
snapshot_entity.SetTransform(
entity.GetTransform() *
Matrix::MakeScale(1.f / source_space_scalar) *
snapshot_transform);
snapshot_entity.SetClipDepth(entity.GetClipDepth());
result = result && snapshot_entity.Render(renderer, pass);
blur_entity.SetClipDepth(entity.GetClipDepth());
blur_entity.SetTransform(entity.GetTransform() *
blurred_transform);
result = result && blur_entity.Render(renderer, pass);
return result;
}),
fml::MakeCopyable([blur_entity = blur_entity.Clone(),
Expand Down
3 changes: 0 additions & 3 deletions impeller/entity/contents/texture_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,6 @@ bool TextureContents::Render(const ContentContext& renderer,
}
pipeline_options.primitive_type = PrimitiveType::kTriangleStrip;

pipeline_options.depth_write_enabled =
stencil_enabled_ && pipeline_options.blend_mode == BlendMode::kSource;

pass.SetPipeline(strict_source_rect_enabled_
? renderer.GetTextureStrictSrcPipeline(pipeline_options)
: renderer.GetTexturePipeline(pipeline_options));
Expand Down
124 changes: 0 additions & 124 deletions impeller/entity/draw_order_resolver.cc

This file was deleted.

94 changes: 0 additions & 94 deletions impeller/entity/draw_order_resolver.h

This file was deleted.

Loading

0 comments on commit 6bf9f63

Please sign in to comment.