Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Display list R-Tree culling #38429

Merged
merged 14 commits into from
Dec 22, 2022
Merged

Display list R-Tree culling #38429

merged 14 commits into from
Dec 22, 2022

Conversation

flar
Copy link
Contributor

@flar flar commented Dec 20, 2022 •
edited
Loading

Fixes: flutter/flutter#92740

This PR uses an R-Tree (when computed by the Builder) to cull the dispatching of a DisplayList similar to what is done in SkPicture objects when constructed with a bbox factory.

Some additional changes:

  • DlRTree no longer depends on the Skia bbox hierarchy base class
  • DlRTree now uses a more streamlined approach to managing its data now that it is not relying on a generic superclass
  • basic tests of R-Tree culling added, and some unit tests. Is something more sophisticated warranted?
  • the code that tags rectangles with the rendering op index is currently executed post-Push which means the rectangles need to be tagged with op_index_ - 1. I eventually want to move the bounds calculations to before the Push calls and then potentially use them to proactively cull operations that will never be visible due to clipping.

The basic principle here is to add the "index" of every rendering op to the DisplayList R-Tree. At rendering time, if the DL is a super-set of the current clip bounds then an R-Tree search is done on those bounds and only the indicated rendering ops are executed. Also executed are all attribute ops, any saveLayer/restore enclosing a chosen rendering op, and any transform/clip operations that will be followed by a matched rendering op before their next restore.

Previous PR prototypes (now obsolete):

@flar flar added the Work in progress (WIP) Not ready (yet) for review! label Dec 20, 2022
@flar flar force-pushed the display-list-rtree-culling-3 branch from 217c86b to 4fe453d Compare December 21, 2022 04:42
@flar flar removed the Work in progress (WIP) Not ready (yet) for review! label Dec 22, 2022
@flar flar marked this pull request as ready for review December 22, 2022 02:21
This was referenced Dec 22, 2022
Closed
Closed
Closed
@flar
Copy link
Contributor Author

flar commented Dec 22, 2022

@endless7 does this solution answer your concerns about the correlation of ops to bounds in the R-Tree?

ColdPaleLight
ColdPaleLight approved these changes Dec 22, 2022
View reviewed changes
Copy link
Member

@ColdPaleLight ColdPaleLight left a comment

Choose a reason for hiding this comment

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

Fantastic! LGTM!

display_list/display_list.cc Show resolved Hide resolved
display_list/display_list.cc Show resolved Hide resolved
@flar flar added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 22, 2022
@auto-submit auto-submit bot merged commit 147ac7d into flutter:main Dec 22, 2022
@flar flar mentioned this pull request Dec 22, 2022
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 22, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 22, 2022
@engine-flutter-autoroll
147ac7d02 Display list R-Tree culling (flutter/engine#38429)
0f30d41
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 22, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 22, 2022
@engine-flutter-autoroll
147ac7d02 Display list R-Tree culling (flutter/engine#38429)
96d31f0
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 23, 2022
@engine-flutter-autoroll
Roll Flutter Engine from 234ab4c1e9c4 to ca0c843bf75f (11 revisions) (#…
daa2ecf 
…117563)

* 23582159a [web] Render in custom target (flutter/engine#37738)

* 766f8fd91 Roll Fuchsia Mac SDK from UsYNZnnfR_s0OGQoX... to Xu_G6EQQ2UG48e5qI... (flutter/engine#38457)

* 6f829b7e3 Roll Dart SDK from fc0a3217b39a to cb6245d8f8d3 (1 revision) (flutter/engine#38458)

* 74d9ba455 Roll Skia from f1610a251e3a to 67904a365fdc (1 revision) (flutter/engine#38459)

* 19de6262b Roll Skia from 67904a365fdc to c93fa176c9ca (6 revisions) (flutter/engine#38460)

* 147ac7d02 Display list R-Tree culling (flutter/engine#38429)

* 3549fb1ed Roll Skia from c93fa176c9ca to 33807a735c32 (3 revisions) (flutter/engine#38464)

* d76ccb8f4 Roll Dart SDK from cb6245d8f8d3 to 47b0d07e6be9 (3 revisions) (flutter/engine#38465)

* 2c9e1d98d Roll Skia from 33807a735c32 to 89742d768c97 (3 revisions) (flutter/engine#38467)

* 3b115f033 Revert "[web] Render in custom target (#37738)" (flutter/engine#38469)

* ca0c843bf Roll Fuchsia Mac SDK from Xu_G6EQQ2UG48e5qI... to W0GUdjHi4gI48optN... (flutter/engine#38468)
@flar
Copy link
Contributor Author

flar commented Dec 23, 2022

This PR shows a net gain in the builder benchmarks for the kRtree and kBoundsAndRtree sub-measurements:

https://flutter-engine-perf.skia.org/e/?begin=1671661938&end=1671754421&keys=X789f7ff76f30f8ccc672464f335fe09b&num_commits=50&request_type=1&xbaroffset=31974

XilaiZhang added a commit that referenced this pull request Dec 23, 2022
@XilaiZhang
Revert "Display list R-Tree culling (#38429)"
0564f08 
This reverts commit 147ac7d.
@XilaiZhang XilaiZhang mentioned this pull request Dec 23, 2022
Closed
@endless7
Copy link
Contributor

@endless7 does this solution answer your concerns about the correlation of ops to bounds in the R-Tree?

I got it. Thanks!!

loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Jan 3, 2023
@flar @loic-sharma
Display list R-Tree culling (flutter#38429)
7db59cb 
* collect DL indices in RTree for clip culling

* fix bounds in unit test and minor opt in Dispatch

* normalize inline matrix objects and minor fixes to unit test

* remove over-eager DCHECK and improve R-Tree comments

* formatting

* include vector for Windows

* method rename and distribute child nodes more evenly

* add R-Tree specific unit tests and debug checks

* add comments about geometry to R-Tree unit tests and adjust spacing

* licenses

* licenses attempt 2

* fix potential overflow with uint32_t

* aggressively const DisplayList fields and methods

* add implementation comments per review feedback
loic-sharma pushed a commit to fluttergithubbot/flutter that referenced this pull request Jan 6, 2023
@engine-flutter-autoroll @loic-sharma
Roll Flutter Engine from 234ab4c1e9c4 to ca0c843bf75f (11 revisions) (f…
54cbcc9 
…lutter#117563)

* 23582159a [web] Render in custom target (flutter/engine#37738)

* 766f8fd91 Roll Fuchsia Mac SDK from UsYNZnnfR_s0OGQoX... to Xu_G6EQQ2UG48e5qI... (flutter/engine#38457)

* 6f829b7e3 Roll Dart SDK from fc0a3217b39a to cb6245d8f8d3 (1 revision) (flutter/engine#38458)

* 74d9ba455 Roll Skia from f1610a251e3a to 67904a365fdc (1 revision) (flutter/engine#38459)

* 19de6262b Roll Skia from 67904a365fdc to c93fa176c9ca (6 revisions) (flutter/engine#38460)

* 147ac7d02 Display list R-Tree culling (flutter/engine#38429)

* 3549fb1ed Roll Skia from c93fa176c9ca to 33807a735c32 (3 revisions) (flutter/engine#38464)

* d76ccb8f4 Roll Dart SDK from cb6245d8f8d3 to 47b0d07e6be9 (3 revisions) (flutter/engine#38465)

* 2c9e1d98d Roll Skia from 33807a735c32 to 89742d768c97 (3 revisions) (flutter/engine#38467)

* 3b115f033 Revert "[web] Render in custom target (flutter#37738)" (flutter/engine#38469)

* ca0c843bf Roll Fuchsia Mac SDK from Xu_G6EQQ2UG48e5qI... to W0GUdjHi4gI48optN... (flutter/engine#38468)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
@engine-flutter-autoroll @gspencergoog
Roll Flutter Engine from 234ab4c1e9c4 to ca0c843bf75f (11 revisions) (f…
a8254b7 
…lutter#117563)

* 23582159a [web] Render in custom target (flutter/engine#37738)

* 766f8fd91 Roll Fuchsia Mac SDK from UsYNZnnfR_s0OGQoX... to Xu_G6EQQ2UG48e5qI... (flutter/engine#38457)

* 6f829b7e3 Roll Dart SDK from fc0a3217b39a to cb6245d8f8d3 (1 revision) (flutter/engine#38458)

* 74d9ba455 Roll Skia from f1610a251e3a to 67904a365fdc (1 revision) (flutter/engine#38459)

* 19de6262b Roll Skia from 67904a365fdc to c93fa176c9ca (6 revisions) (flutter/engine#38460)

* 147ac7d02 Display list R-Tree culling (flutter/engine#38429)

* 3549fb1ed Roll Skia from c93fa176c9ca to 33807a735c32 (3 revisions) (flutter/engine#38464)

* d76ccb8f4 Roll Dart SDK from cb6245d8f8d3 to 47b0d07e6be9 (3 revisions) (flutter/engine#38465)

* 2c9e1d98d Roll Skia from 33807a735c32 to 89742d768c97 (3 revisions) (flutter/engine#38467)

* 3b115f033 Revert "[web] Render in custom target (flutter#37738)" (flutter/engine#38469)

* ca0c843bf Roll Fuchsia Mac SDK from Xu_G6EQQ2UG48e5qI... to W0GUdjHi4gI48optN... (flutter/engine#38468)
This was referenced Jun 1, 2023
Closed
Closed
@flar flar mentioned this pull request Aug 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ColdPaleLight ColdPaleLight ColdPaleLight approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
Display List
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DisplayList has no proactive culling as was available in SkPicture
3 participants
@flar @endless7 @ColdPaleLight