From 6603a4e814faeeef8f19a05ee40b546f805ea43c Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Sat, 23 Oct 2021 03:39:10 +0000 Subject: [PATCH] Bug 1734854: Make NativeLayerRootCA avoid changing sublayers when it has no effect. r=gfx-reviewers,jrmuizel When isolating video layers, changes to sublayers that we are already ignoring should not cause us to change our sublayers. This patch checks to see if the changes to sublayers can all be safely ignored. Differential Revision: https://phabricator.services.mozilla.com/D129330 --- gfx/layers/NativeLayerCA.h | 1 + gfx/layers/NativeLayerCA.mm | 94 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 73 insertions(+), 22 deletions(-) diff --git a/gfx/layers/NativeLayerCA.h b/gfx/layers/NativeLayerCA.h index db2e899fdf28..2579ec0baed3 100644 --- a/gfx/layers/NativeLayerCA.h +++ b/gfx/layers/NativeLayerCA.h @@ -137,6 +137,7 @@ class NativeLayerRootCA : public NativeLayerRoot { WhichRepresentation aRepresentation, const nsTArray>& aSublayers); CALayer* mRootCALayer = nullptr; // strong + bool mIsIsolatingVideo = false; bool mMutatedLayerStructure = false; bool mMutatedMouseMovedRecently = false; }; diff --git a/gfx/layers/NativeLayerCA.mm b/gfx/layers/NativeLayerCA.mm index 017305bde1a9..a4914d116739 100644 --- a/gfx/layers/NativeLayerCA.mm +++ b/gfx/layers/NativeLayerCA.mm @@ -339,35 +339,85 @@ void NativeLayerRootCA::Representation::Commit(WhichRepresentation aRepresentati } if (mMutatedLayerStructure || mustRebuild) { - NSMutableArray* sublayers = [NSMutableArray arrayWithCapacity:aSublayers.Length()]; - for (auto layer : aSublayers) { - [sublayers addObject:layer->UnderlyingCALayer(aRepresentation)]; + // Bug 1731821 should eliminate this most of this logic and allow us to unconditionally + // accept aSublayers. + + // We're going to check for an opportunity to isolate the topmost video layer. We'll avoid + // modifying mRootCALayer.sublayers unless we absolutely must, which will avoid flickering + // in the CATranscation. We check aSublayers with 3 possible outcomes. + // 1) We can't isolate video, so accept the provided sublayers. + // 2) We can isolate video, and we weren't isolating that video before, so create our own + // sublayers with the proper structure. + // 3) We can isolate video, and we were already doing that. The sublayers underneath the + // video might have changed in some way that doesn't prevent isolation, so ignore them. + // Leave our sublayers unchanged. + + // Define a block we'll use to accept the provided sublayers if we must. In the different + // cases, we'll call this at different times. + void (^acceptProvidedSublayers)() = ^() { + NSMutableArray* sublayers = [NSMutableArray arrayWithCapacity:aSublayers.Length()]; + for (auto layer : aSublayers) { + [sublayers addObject:layer->UnderlyingCALayer(aRepresentation)]; + } + mRootCALayer.sublayers = sublayers; + }; + + // See if the top layer is already rooted in our mRootCALayer. If it is, then we can check + // for isolation without first disrupting our sublayers. + bool topLayerIsRooted = + aSublayers.Length() && + (aSublayers.LastElement()->UnderlyingCALayer(aRepresentation).superlayer == mRootCALayer); + + if (!topLayerIsRooted) { + // We have to accept the provided sublayers. We may still isolate, but it's because the + // new topmost layer was not already isolated. This is an acceptable time to potentially + // flicker as the sublayers are changed. + acceptProvidedSublayers(); } - mRootCALayer.sublayers = sublayers; - // Now that we've set these layer relationships, we check to see if we should break - // them and isolate a single video layer. It's important to do this *after* the - // sublayers have been set, because we need the relationships there to do the - // bounds checking of layer spaces against each other. - // Bug 1731821 should eliminate this entire if block. + // Now that we've confirmed these layer relationships, we check to see if we should break + // them and isolate a single video layer. It's important that the topmost layer is a + // child of mRootCALayer for this logic to work. + MOZ_DIAGNOSTIC_ASSERT( + !aSublayers.Length() || + (aSublayers.LastElement()->UnderlyingCALayer(aRepresentation).superlayer == + mRootCALayer), + "The topmost layer must be a child of mRootCALayer."); + + bool didIsolate = false; if (mightIsolate && aWindowIsFullscreen && !aMouseMovedRecently) { CALayer* isolatedLayer = FindVideoLayerToIsolate(aRepresentation, aSublayers); if (isolatedLayer) { - // Create a full coverage black layer behind the isolated layer. - CGFloat rootWidth = mRootCALayer.bounds.size.width; - CGFloat rootHeight = mRootCALayer.bounds.size.height; - - // Reaching the low-power mode requires that there is a single black layer - // covering the entire window behind the video layer. Create that layer. - CALayer* blackLayer = [CALayer layer]; - blackLayer.position = NSZeroPoint; - blackLayer.anchorPoint = NSZeroPoint; - blackLayer.bounds = CGRectMake(0, 0, rootWidth, rootHeight); - blackLayer.backgroundColor = [[NSColor blackColor] CGColor]; - - mRootCALayer.sublayers = @[ blackLayer, isolatedLayer ]; + // No matter what happens next, we did choose to isolate. + didIsolate = true; + + // We only need to change our sublayers if we weren't already isolating, or + // if the isolatedLayer does not match our current top layer. + if (!mIsIsolatingVideo || isolatedLayer != mRootCALayer.sublayers.lastObject) { + // Create a full coverage black layer behind the isolated layer. + CGFloat rootWidth = mRootCALayer.bounds.size.width; + CGFloat rootHeight = mRootCALayer.bounds.size.height; + + // Reaching the low-power mode requires that there is a single black layer + // covering the entire window behind the video layer. Create that layer. + CALayer* blackLayer = [CALayer layer]; + blackLayer.position = NSZeroPoint; + blackLayer.anchorPoint = NSZeroPoint; + blackLayer.bounds = CGRectMake(0, 0, rootWidth, rootHeight); + blackLayer.backgroundColor = [[NSColor blackColor] CGColor]; + + mRootCALayer.sublayers = @[ blackLayer, isolatedLayer ]; + } } } + + // If we didn't accept the sublayers earlier, and we decided we couldn't isolate, + // accept them now. + if (topLayerIsRooted && !didIsolate) { + acceptProvidedSublayers(); + } + + mIsIsolatingVideo = didIsolate; } mMutatedLayerStructure = false; -- 2.11.4.GIT