Skip to content

Commit 567ec62

Browse files
author
Rachel Lee
committed
Fix NoVote/NoPreference on LayerHistory
Previous CL ag/30285441 was supposed to allow game default override if the layervote is simply "NoPreference" category. However there was a bug with the CL. This CL fixes this and also removes the early skip of NoVote, in order for explicit NoVote layers to not affect frame rate scoring even if surface has drawing. Bug: 378455432 Test: atest libsurfaceflinger_unittest Test: manual test game with overlay Test: manual test video with overlay Test: manual test on both MRR and ARR Flag: EXEMPT bugfix Change-Id: Iba36cc89597f544bdc3311424f6e5d04439d7dc7
1 parent fa8a786 commit 567ec62

2 files changed

Lines changed: 216 additions & 4 deletions

File tree

‎services/surfaceflinger/Scheduler/LayerHistory.cpp‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,11 +308,14 @@ void LayerHistory::partitionLayers(nsecs_t now, bool isVrrDevice) {
308308
const auto setFrameRateVoteType =
309309
info->isVisible() ? voteType : LayerVoteType::NoVote;
310310

311-
const bool hasSetFrameRateOpinion = frameRate.isValid() && !frameRate.isNoVote();
311+
const bool hasSetFrameRateOpinion =
312+
frameRate.isValuelessType() || frameRate.vote.rate.isValid();
312313
const bool hasCategoryOpinion =
313314
frameRate.category != FrameRateCategory::NoPreference &&
314315
frameRate.category != FrameRateCategory::Default;
315-
const bool hasFrameRateOpinion = hasSetFrameRateOpinion || hasCategoryOpinion;
316+
const bool hasFrameRateOpinionAboveGameDefault =
317+
hasSetFrameRateOpinion || hasCategoryOpinion;
318+
const bool hasFrameRateOpinionArr = frameRate.isValid() && !frameRate.isNoVote();
316319

317320
if (gameModeFrameRateOverride.isValid()) {
318321
info->setLayerVote({gameFrameRateOverrideVoteType, gameModeFrameRateOverride});
@@ -321,7 +324,8 @@ void LayerHistory::partitionLayers(nsecs_t now, bool isVrrDevice) {
321324
trace(*info, gameFrameRateOverrideVoteType,
322325
gameModeFrameRateOverride.getIntValue());
323326
}
324-
} else if (hasFrameRateOpinion && frameRate.isVoteValidForMrr(isVrrDevice)) {
327+
} else if (hasFrameRateOpinionAboveGameDefault &&
328+
frameRate.isVoteValidForMrr(isVrrDevice)) {
325329
info->setLayerVote({setFrameRateVoteType,
326330
isValuelessVote ? 0_Hz : frameRate.vote.rate,
327331
frameRate.vote.seamlessness, frameRate.category});
@@ -337,8 +341,18 @@ void LayerHistory::partitionLayers(nsecs_t now, bool isVrrDevice) {
337341
trace(*info, gameFrameRateOverrideVoteType,
338342
gameDefaultFrameRateOverride.getIntValue());
339343
}
344+
} else if (hasFrameRateOpinionArr && frameRate.isVoteValidForMrr(isVrrDevice)) {
345+
// This allows NoPreference votes on ARR devices after considering the
346+
// gameDefaultFrameRateOverride (above).
347+
info->setLayerVote({setFrameRateVoteType,
348+
isValuelessVote ? 0_Hz : frameRate.vote.rate,
349+
frameRate.vote.seamlessness, frameRate.category});
350+
if (CC_UNLIKELY(mTraceEnabled)) {
351+
trace(*info, gameFrameRateOverrideVoteType,
352+
frameRate.vote.rate.getIntValue());
353+
}
340354
} else {
341-
if (hasFrameRateOpinion && !frameRate.isVoteValidForMrr(isVrrDevice)) {
355+
if (hasFrameRateOpinionArr && !frameRate.isVoteValidForMrr(isVrrDevice)) {
342356
SFTRACE_FORMAT_INSTANT("Reset layer to ignore explicit vote on MRR %s: %s "
343357
"%s %s",
344358
info->getName().c_str(),

‎services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,204 @@ TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreference) {
715715
EXPECT_EQ(0, frequentLayerCount(time));
716716
}
717717

718+
// Tests MRR NoPreference-only vote, no game default override. Expects vote reset.
719+
TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreference_mrr) {
720+
SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
721+
SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
722+
SET_FLAG_FOR_TEST(flags::vrr_config, true);
723+
724+
const LayerHistory::LayerVoteType defaultVote = LayerHistory::LayerVoteType::Min;
725+
726+
auto layer = createLegacyAndFrontedEndLayer(1);
727+
setDefaultLayerVote(layer.get(), defaultVote);
728+
showLayer(1);
729+
setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
730+
ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
731+
setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
732+
733+
EXPECT_EQ(1u, layerCount());
734+
EXPECT_EQ(0u, activeLayerCount());
735+
736+
nsecs_t time = systemTime();
737+
for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
738+
setBufferWithPresentTime(layer, time);
739+
time += HI_FPS_PERIOD;
740+
}
741+
742+
EXPECT_EQ(1u, summarizeLayerHistory(time).size());
743+
EXPECT_EQ(1u, activeLayerCount());
744+
EXPECT_EQ(1, frequentLayerCount(time));
745+
EXPECT_EQ(defaultVote, summarizeLayerHistory(time)[0].vote);
746+
EXPECT_EQ(0_Hz, summarizeLayerHistory(time)[0].desiredRefreshRate);
747+
EXPECT_EQ(FrameRateCategory::Default, summarizeLayerHistory(time)[0].frameRateCategory);
748+
}
749+
750+
// Tests VRR NoPreference-only vote, no game default override. Expects NoPreference, *not* vote
751+
// reset.
752+
TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreference_vrr) {
753+
SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
754+
SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
755+
SET_FLAG_FOR_TEST(flags::vrr_config, true);
756+
mSelector->setActiveMode(kVrrModeId, HI_FPS);
757+
758+
const LayerHistory::LayerVoteType defaultVote = LayerHistory::LayerVoteType::Min;
759+
760+
auto layer = createLegacyAndFrontedEndLayer(1);
761+
setDefaultLayerVote(layer.get(), defaultVote);
762+
showLayer(1);
763+
setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
764+
ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
765+
setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
766+
767+
EXPECT_EQ(1u, layerCount());
768+
EXPECT_EQ(0u, activeLayerCount());
769+
770+
nsecs_t time = systemTime();
771+
for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
772+
setBufferWithPresentTime(layer, time);
773+
time += HI_FPS_PERIOD;
774+
}
775+
776+
EXPECT_EQ(1u, summarizeLayerHistory(time).size());
777+
EXPECT_EQ(1u, activeLayerCount());
778+
EXPECT_EQ(1, frequentLayerCount(time));
779+
EXPECT_EQ(LayerHistory::LayerVoteType::ExplicitCategory, summarizeLayerHistory(time)[0].vote);
780+
EXPECT_EQ(0_Hz, summarizeLayerHistory(time)[0].desiredRefreshRate);
781+
EXPECT_EQ(FrameRateCategory::NoPreference, summarizeLayerHistory(time)[0].frameRateCategory);
782+
}
783+
784+
TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreferenceWithGameDefault_vrr) {
785+
SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
786+
SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
787+
SET_FLAG_FOR_TEST(flags::vrr_config, true);
788+
mSelector->setActiveMode(kVrrModeId, HI_FPS);
789+
790+
const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
791+
const uid_t uid = 456;
792+
793+
history().updateGameDefaultFrameRateOverride(
794+
FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
795+
796+
auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
797+
showLayer(1);
798+
setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
799+
ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
800+
setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
801+
802+
EXPECT_EQ(1u, layerCount());
803+
EXPECT_EQ(0u, activeLayerCount());
804+
805+
nsecs_t time = systemTime();
806+
for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
807+
setBufferWithPresentTime(layer, time);
808+
time += HI_FPS_PERIOD;
809+
}
810+
811+
EXPECT_EQ(1u, summarizeLayerHistory(time).size());
812+
EXPECT_EQ(1u, activeLayerCount());
813+
EXPECT_EQ(1, frequentLayerCount(time));
814+
EXPECT_EQ(LayerHistory::LayerVoteType::ExplicitDefault, summarizeLayerHistory(time)[0].vote);
815+
EXPECT_EQ(gameDefaultFrameRate, summarizeLayerHistory(time)[0].desiredRefreshRate);
816+
EXPECT_EQ(FrameRateCategory::Default, summarizeLayerHistory(time)[0].frameRateCategory);
817+
}
818+
819+
TEST_F(LayerHistoryIntegrationTest, oneLayerCategoryNoPreferenceWithGameDefault_mrr) {
820+
SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
821+
SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
822+
SET_FLAG_FOR_TEST(flags::vrr_config, true);
823+
824+
const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
825+
const uid_t uid = 456;
826+
827+
history().updateGameDefaultFrameRateOverride(
828+
FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
829+
830+
auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
831+
showLayer(1);
832+
setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT,
833+
ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
834+
setFrameRateCategory(1, ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE);
835+
836+
EXPECT_EQ(1u, layerCount());
837+
EXPECT_EQ(0u, activeLayerCount());
838+
839+
nsecs_t time = systemTime();
840+
for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
841+
setBufferWithPresentTime(layer, time);
842+
time += HI_FPS_PERIOD;
843+
}
844+
845+
EXPECT_EQ(1u, summarizeLayerHistory(time).size());
846+
EXPECT_EQ(1u, activeLayerCount());
847+
EXPECT_EQ(1, frequentLayerCount(time));
848+
EXPECT_EQ(LayerHistory::LayerVoteType::ExplicitDefault, summarizeLayerHistory(time)[0].vote);
849+
EXPECT_EQ(gameDefaultFrameRate, summarizeLayerHistory(time)[0].desiredRefreshRate);
850+
EXPECT_EQ(FrameRateCategory::Default, summarizeLayerHistory(time)[0].frameRateCategory);
851+
}
852+
853+
TEST_F(LayerHistoryIntegrationTest, oneLayerNoVoteWithGameDefault_vrr) {
854+
SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
855+
SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
856+
SET_FLAG_FOR_TEST(flags::vrr_config, true);
857+
mSelector->setActiveMode(kVrrModeId, HI_FPS);
858+
859+
const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
860+
const uid_t uid = 456;
861+
862+
history().updateGameDefaultFrameRateOverride(
863+
FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
864+
865+
auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
866+
showLayer(1);
867+
setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_NO_VOTE,
868+
ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
869+
870+
EXPECT_EQ(1u, layerCount());
871+
EXPECT_EQ(0u, activeLayerCount());
872+
873+
nsecs_t time = systemTime();
874+
for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
875+
setBufferWithPresentTime(layer, time);
876+
time += HI_FPS_PERIOD;
877+
}
878+
879+
// Expect NoVote to be skipped in summarize.
880+
EXPECT_EQ(0u, summarizeLayerHistory(time).size());
881+
EXPECT_EQ(1u, activeLayerCount());
882+
EXPECT_EQ(1, frequentLayerCount(time));
883+
}
884+
885+
TEST_F(LayerHistoryIntegrationTest, oneLayerNoVoteWithGameDefault_mrr) {
886+
SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, false);
887+
SET_FLAG_FOR_TEST(flags::game_default_frame_rate, true);
888+
SET_FLAG_FOR_TEST(flags::vrr_config, true);
889+
890+
const Fps gameDefaultFrameRate = Fps::fromValue(30.0f);
891+
const uid_t uid = 456;
892+
893+
history().updateGameDefaultFrameRateOverride(
894+
FrameRateOverride({uid, gameDefaultFrameRate.getValue()}));
895+
896+
auto layer = createLegacyAndFrontedEndLayerWithUid(1, gui::Uid(uid));
897+
showLayer(1);
898+
setFrameRate(1, (0_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_NO_VOTE,
899+
ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS);
900+
901+
EXPECT_EQ(1u, layerCount());
902+
EXPECT_EQ(0u, activeLayerCount());
903+
904+
nsecs_t time = systemTime();
905+
for (size_t i = 0; i < PRESENT_TIME_HISTORY_SIZE; i++) {
906+
setBufferWithPresentTime(layer, time);
907+
time += HI_FPS_PERIOD;
908+
}
909+
910+
// Expect NoVote to be skipped in summarize.
911+
EXPECT_EQ(0u, summarizeLayerHistory(time).size());
912+
EXPECT_EQ(1u, activeLayerCount());
913+
EXPECT_EQ(1, frequentLayerCount(time));
914+
}
915+
718916
TEST_F(LayerHistoryIntegrationTest, oneLayerExplicitVoteWithCategory) {
719917
SET_FLAG_FOR_TEST(flags::frame_rate_category_mrr, true);
720918

0 commit comments

Comments
 (0)