In r293943 some shouldApplyContainment methods were optimised, however they are not flexible enough to deal with the content-visibility feature, which in certain cases can influence containment. So to be flexible use lambdas that return a bool instead of relying on effectiveContainment().
Created attachment 459757 [details] Patch
Comment on attachment 459757 [details] Patch These functions are hot because they fall into the "containingBlock" code path. Does this affect performance?
(In reply to Simon Fraser (smfr) from comment #2) > Comment on attachment 459757 [details] > Patch > > These functions are hot because they fall into the "containingBlock" code > path. Does this affect performance? Are you thinking of canContainFixedPositionObjects/canContainAbsolutelyPositionedObjects or more? If the lambda's are nicely inlined the performance should be the same, but I never checked efficiency for that (I guess I could try godbold.org tomorrow). An older patch used bool parameters for shouldApplyLayoutOrPaintContainment/shouldApplySizeOrStyleContainment, to me it does not look as nice as lambdas, but it pretty much guarantees same perf as we have now, and it works to support content-visibility later on, so I do not mind using that instead.
Created attachment 459779 [details] Patch
(In reply to Rob Buis from comment #3) > (In reply to Simon Fraser (smfr) from comment #2) > > Comment on attachment 459757 [details] > > Patch > > > > These functions are hot because they fall into the "containingBlock" code > > path. Does this affect performance? > > Are you thinking of > canContainFixedPositionObjects/canContainAbsolutelyPositionedObjects or more? > If the lambda's are nicely inlined the performance should be the same, but I > never checked efficiency for that (I guess I could try godbold.org > tomorrow). An older patch used bool parameters for > shouldApplyLayoutOrPaintContainment/shouldApplySizeOrStyleContainment, to me > it does not look as nice as lambdas, but it pretty much guarantees same perf > as we have now, and it works to support content-visibility later on, so I do > not mind using that instead. From checking godbolt.org output the lambdas do seem to generate quite some extra instructions, so I changed the patch to use boolean parameters instead.
<rdar://problem/94218126>
Pull request: https://github.com/WebKit/WebKit/pull/1373
Pull request: https://github.com/WebKit/WebKit/pull/1374
Pull request: https://github.com/WebKit/WebKit/pull/2857
Pull request: https://github.com/WebKit/WebKit/pull/3182
Committed 253324@main (6c79952a8e99): <https://commits.webkit.org/253324@main> Reviewed commits have been landed. Closing PR #3182 and removing active labels.