Skip to content

Commit

Permalink
Reverts "[Impeller] Implement draw order optimization. (#54067)" (#54136
Browse files Browse the repository at this point in the history
)

Reverts: #54067
Initiated by: jonahwilliams
Reason for reverting: disappearing fabs and background color in framework golden tests https://flutter-gold.skia.org/search?issue=152354&crs=github&patchsets=2&corpus=flutter
Original PR Author: bdero

Reviewed By: {jonahwilliams}

This change reverts the following previous change:
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 26, 2024
1 parent 09a2c56 commit 342a425
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 495 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 @@ -42130,8 +42130,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 @@ -45017,8 +45015,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 @@ -555,16 +555,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 342a425

Please sign in to comment.