Implement union and intersection for DlRegion

[knopp]

Fixes flutter/flutter#128412

Adds

• DlRegion DlRegion::MakeUnion(const Region &, const DlRegion &)
• DlRegion DlRegion::MakeIntersection(const Region &, const DlRegion &)
• bool DlRegion::intersects(const DlRegion &)
• bool DlRegion::intersects(const SkIRect &)

Instead of per span line vector all spans are stored in continuous buffer.

Complete benchmarks:

-----------------------------------------------------------------------------------------------
Benchmark                                                     Time             CPU   Iterations
-----------------------------------------------------------------------------------------------
BM_DlRegion_IntersectsSingleRect/Tiny                      2688 ns         2687 ns       258580
BM_SkRegion_IntersectsSingleRect/Tiny                     85889 ns        85877 ns         8092
BM_DlRegion_IntersectsSingleRect/Small                     4814 ns         4813 ns       142874
BM_SkRegion_IntersectsSingleRect/Small                   101102 ns       101102 ns         6833
BM_DlRegion_IntersectsSingleRect/Medium                    2329 ns         2329 ns       302911
BM_SkRegion_IntersectsSingleRect/Medium                   60436 ns        60183 ns        11156
BM_DlRegion_IntersectsSingleRect/Large                     1243 ns         1243 ns       565209
BM_SkRegion_IntersectsSingleRect/Large                     2813 ns         2813 ns       252187
BM_DlRegion_IntersectsRegion/Tiny                          38.9 ns         38.9 ns     17913855
BM_SkRegion_IntersectsRegion/Tiny                           203 ns          203 ns      3480855
BM_DlRegion_IntersectsRegion/Small                          306 ns          306 ns      2295413
BM_SkRegion_IntersectsRegion/Small                         1057 ns         1057 ns       660826
BM_DlRegion_IntersectsRegion/Medium                        8.83 ns         8.83 ns     79128233
BM_SkRegion_IntersectsRegion/Medium                        43.3 ns         43.3 ns     16076912
BM_DlRegion_IntersectsRegion/Large                         6.96 ns         6.96 ns    101646676
BM_SkRegion_IntersectsRegion/Large                         31.8 ns         31.8 ns     22121517
BM_DlRegion_IntersectsRegion/TinyAsymmetric                54.2 ns         54.2 ns     12890870
BM_SkRegion_IntersectsRegion/TinyAsymmetric                4575 ns         4574 ns       155368
BM_DlRegion_IntersectsRegion/SmallAsymmetric                190 ns          189 ns      3748547
BM_SkRegion_IntersectsRegion/SmallAsymmetric               6157 ns         6157 ns       114403
BM_DlRegion_IntersectsRegion/MediumAsymmetric              20.9 ns         20.9 ns     33523941
BM_SkRegion_IntersectsRegion/MediumAsymmetric              3247 ns         3247 ns       214694
BM_DlRegion_IntersectsRegion/LargeAsymmetric               8.97 ns         8.97 ns     76827676
BM_SkRegion_IntersectsRegion/LargeAsymmetric                154 ns          154 ns      4757924
BM_DlRegion_Operation/Union_Tiny                           26.3 us         26.3 us        24534
BM_SkRegion_Operation/Union_Tiny                           37.9 us         37.9 us        17973
BM_DlRegion_Operation/Union_Small                          64.4 us         64.4 us        10657
BM_SkRegion_Operation/Union_Small                           105 us          105 us         6278
BM_DlRegion_Operation/Union_Medium                         22.0 us         22.0 us        31631
BM_SkRegion_Operation/Union_Medium                         64.8 us         64.8 us        10744
BM_DlRegion_Operation/Union_Large                          1.00 us         1.00 us       697406
BM_SkRegion_Operation/Union_Large                          1.29 us         1.29 us       547089
BM_DlRegion_Operation/Union_TinyAsymmetric                 10.3 us         10.3 us        68647
BM_SkRegion_Operation/Union_TinyAsymmetric                 20.6 us         20.6 us        33282
BM_DlRegion_Operation/Union_SmallAsymmetric                14.0 us         14.0 us        49944
BM_SkRegion_Operation/Union_SmallAsymmetric                34.4 us         34.4 us        19618
BM_DlRegion_Operation/Union_MediumAsymmetric               5.24 us         5.24 us       134097
BM_SkRegion_Operation/Union_MediumAsymmetric               12.7 us         12.7 us        55069
BM_DlRegion_Operation/Union_LargeAsymmetric               0.376 us        0.376 us      1808589
BM_SkRegion_Operation/Union_LargeAsymmetric               0.533 us        0.532 us      1283674
BM_DlRegion_Operation/Intersection_Tiny                    8.13 us         8.13 us        87199
BM_SkRegion_Operation/Intersection_Tiny                    31.8 us         31.8 us        21864
BM_DlRegion_Operation/Intersection_Small                   55.9 us         55.9 us        11888
BM_SkRegion_Operation/Intersection_Small                   98.4 us         98.3 us         6963
BM_DlRegion_Operation/Intersection_Medium                  40.0 us         40.0 us        17667
BM_SkRegion_Operation/Intersection_Medium                  69.8 us         69.8 us         9910
BM_DlRegion_Operation/Intersection_Large                   1.06 us         1.06 us       650957
BM_SkRegion_Operation/Intersection_Large                   1.26 us         1.26 us       559624
BM_DlRegion_Operation/Intersection_TinyAsymmetric          2.62 us         2.62 us       264565
BM_SkRegion_Operation/Intersection_TinyAsymmetric          15.3 us         15.3 us        45528
BM_DlRegion_Operation/Intersection_SmallAsymmetric         7.15 us         7.15 us        93482
BM_SkRegion_Operation/Intersection_SmallAsymmetric         27.5 us         27.5 us        24450
BM_DlRegion_Operation/Intersection_MediumAsymmetric        2.95 us         2.95 us       235133
BM_SkRegion_Operation/Intersection_MediumAsymmetric        10.5 us         10.5 us        65925
BM_DlRegion_Operation/Intersection_LargeAsymmetric        0.165 us        0.165 us      4016433
BM_SkRegion_Operation/Intersection_LargeAsymmetric        0.409 us        0.409 us      1719716
BM_DlRegion_Operation/Intersection_SingleRect_Tiny        0.105 us        0.105 us      7403099
BM_SkRegion_Operation/Intersection_SingleRect_Tiny         10.8 us         10.8 us        64185
BM_DlRegion_Operation/Intersection_SingleRect_Small       0.410 us        0.410 us      1724524
BM_SkRegion_Operation/Intersection_SingleRect_Small        16.2 us         16.2 us        43707
BM_DlRegion_Operation/Intersection_SingleRect_Medium      0.458 us        0.458 us      1540049
BM_SkRegion_Operation/Intersection_SingleRect_Medium       7.54 us         7.54 us        93407
BM_DlRegion_Operation/Intersection_SingleRect_Large       0.175 us        0.175 us      3984926
BM_SkRegion_Operation/Intersection_SingleRect_Large       0.351 us        0.351 us      1931946
BM_DlRegion_FromRects/Tiny                                  154 us          154 us         4383
BM_SkRegion_FromRects/Tiny                                69429 us        69419 us           10
BM_DlRegion_FromRects/Small                                 369 us          369 us         1932
BM_SkRegion_FromRects/Small                              117584 us       117578 us            6
BM_DlRegion_FromRects/Medium                                475 us          475 us         1477
BM_SkRegion_FromRects/Medium                              21611 us        21610 us           33
BM_DlRegion_FromRects/Large                                1329 us         1329 us          533
BM_SkRegion_FromRects/Large                                1409 us         1409 us          501
BM_DlRegion_GetRects/Tiny                                  39.2 us         39.2 us        18030
BM_SkRegion_GetRects/Tiny                                  84.2 us         84.2 us         9971
BM_DlRegion_GetRects/Small                                 88.9 us         88.9 us         7873
BM_SkRegion_GetRects/Small                                  212 us          212 us         3598
BM_DlRegion_GetRects/Medium                               0.845 us        0.813 us       881224
BM_SkRegion_GetRects/Medium                                3.10 us         3.09 us       223483
BM_DlRegion_GetRects/Large                                0.120 us        0.120 us      5954761
BM_SkRegion_GetRects/Large                                0.337 us        0.336 us      2068656

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.

[flar]

Just a thought I wanted to pass along. A cursory inspection looks like this might be easy to do with the code as written.

If you look at 2 regions and consider that their interaction can be broken down into 4 types of overlap:

• Neither: not in either region
• AnotB: In A, but not in B
• BnotA: In B, but not in A
• Both: In both regions

Then you can write a general Op that takes 3 flags (AnotB, BnotA, Both) or a bitmask and can perform any number of operations for the same code cost and very very minimal performance overhead:

Op name	AnotB	BnotA	Both
Union	Yes	Yes	Yes
Intersect	No	No	Yes
Subtract	Yes	No	No
Xor	Yes	Yes	No

So you get 4 ops for the price of 1 (not that we have any immediate use for the other 3).

(And, in terms of API, the flags would be an internal implementation detail - the API should take an enum, or different method names or something descriptive like that.)

[knopp]

Just a thought I wanted to pass along. A cursory inspection looks like this might be easy to do with the code as written.

If you look at 2 regions and consider that their interaction can be broken down into 4 types of overlap:

• Neither: not in either region
• AnotB: In A, but not in B
• BnotA: In B, but not in A
• Both: In both regions

Then you can write a general Op that takes 3 flags (AnotB, BnotA, Both) or a bitmask and can perform any number of operations for the same code cost and very very minimal performance overhead:

Op name AnotB BnotA Both
Union Yes Yes Yes
Intersect No No Yes
Subtract Yes No No
Xor Yes Yes No
So you get 4 ops for the price of 1 (not that we have any immediate use for the other 3).

(And, in terms of API, the flags would be an internal implementation detail - the API should take an enum, or different method names or something descriptive like that.)

That seems similar to what SkRegion does to a degree, but I think it comes at a cost (union of regions is well optimized in SkRegion but is still slower). I can imagine doing this for span lines, but when it comes to processing actual spans in the line inside DlRegion::mergeLines (should probably be called unionLines) I don't know how if I could do this without additional overhead. We're at the point where replacing vector with custom buffer had significant performance impact.

engine/display_list/geometry/dl_region.cc

Lines 96 to 200 in 5f7083c

	size_t DlRegion::mergeLines(std::vector<Span>& res,
	const SpanBuffer& a_buffer,
	SpanChunkHandle a_handle,
	const SpanBuffer& b_buffer,
	SpanChunkHandle b_handle) {
	const Span *begin1, *end1;
	a_buffer.getSpans(a_handle, begin1, end1);
	const Span *begin2, *end2;
	b_buffer.getSpans(b_handle, begin2, end2);
	size_t min_size = (end1 - begin1) + (end2 - begin2);
	if (res.size() < min_size) {
	res.resize(min_size);
	}
	Span* end = res.data();
	while (true) {
	if (begin1->right < begin2->left - 1) {
	*end = *begin1;
	++begin1;
	++end;
	if (begin1 == end1) {
	break;
	}
	} else if (begin2->right < begin1->left) {
	*end = *begin2;
	++begin2;
	++end;
	if (begin2 == end2) {
	break;
	}
	} else {
	break;
	}
	}
	Span currentSpan{0, 0};
	while (begin1 != end1 && begin2 != end2) {
	if (currentSpan.left == currentSpan.right) {
	if (begin1->right < begin2->left - 1) {
	*end = *begin1;
	++begin1;
	++end;
	} else if (begin2->right < begin1->left) {
	*end = *begin2;
	++begin2;
	++end;
	} else if (begin1->left == begin2->left) {
	currentSpan.left = begin1->left;
	currentSpan.right = std::max(begin1->right, begin2->right);
	++begin1;
	++begin2;
	} else if (begin1->left < begin2->left) {
	currentSpan.left = begin1->left;
	currentSpan.right = begin1->right;
	++begin1;
	} else {
	currentSpan.left = begin2->left;
	currentSpan.right = begin2->right;
	++begin2;
	}
	} else if (currentSpan.right >= begin1->left) {
	currentSpan.right = std::max(currentSpan.right, begin1->right);
	++begin1;
	} else if (currentSpan.right >= begin2->left) {
	currentSpan.right = std::max(currentSpan.right, begin2->right);
	++begin2;
	} else {
	*end = currentSpan;
	++end;
	currentSpan.left = currentSpan.right = 0;
	}
	}
	if (currentSpan.left != currentSpan.right) {
	while (begin1 != end1 && currentSpan.right >= begin1->left) {
	currentSpan.right = std::max(currentSpan.right, begin1->right);
	++begin1;
	}
	while (begin2 != end2 && currentSpan.right >= begin2->left) {
	currentSpan.right = std::max(currentSpan.right, begin2->right);
	++begin2;
	}
	*end = currentSpan;
	++end;
	}
	FML_DCHECK(begin1 == end1 || begin2 == end2);
	while (begin1 != end1) {
	*end = *begin1;
	++begin1;
	++end;
	}
	while (begin2 != end2) {
	*end = *begin2;
	++begin2;
	++end;
	}
	return end - res.data();
	}

[flar]

I'm not done reviewing all of the code, but reached a stopping point so I thought I would submit the review so far...

[flar]

A few suggestions but also a potential bug in modifying an input region hidden in the union code.

[chinmaygarde]

Looks ready for another look cc @flar

[flar]

Everything still looks good and the PR is still approved, just adding some clarification notes on my previous optional comments that might have been unclear.

[flar]

As before, functionally it is already good to go, but some observations that might help with maintenance or performance to consider...

[flar]

LGTM