Skip to content
This repository was archived by the owner on Jan 30, 2025. It is now read-only.

Commit

Permalink
vm_tools: sommelier: add quirk condition always
Browse files Browse the repository at this point in the history 
This CL adds support for defining 'default' quirk configuration
that applies to all windows for condition `always=true`.

Priority is based on the appearence of SommelierRule in the file.
User-configurations always take priority over the system-defined.

BUG=b:328703154
TEST=Run unit test

Change-Id: Ia4a9a6040f9b6f49467fbe8993022391c8430794
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/5369414
Commit-Queue: Max Lee <[email protected]>
Tested-by: Max Lee <[email protected]>
Reviewed-by: Chloe Pelling <[email protected]>
  • Loading branch information
Max Lee authored and Chromeos LUCI committed Mar 14, 2024
1 parent ee8083e commit b042bd2
Show file tree
Hide file tree
Showing 4 changed files with 253 additions and 19 deletions.
5 changes: 5 additions & 0 deletions quirks/quirks.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ package quirks;
option java_multiple_files = true;

message Config {
// The order of SommelierRule determines the priority -
// i.e. last-defined SommelierRule has the highest prioirity.
repeated SommelierRule sommelier = 1;
}

Expand All @@ -31,7 +33,10 @@ message SommelierCondition {
// older versions of Sommelier applying rules containing unrecognized
// conditions.
oneof condition {
// Match by Steam game ID.
uint32 steam_game_id = 1;
// Match if true, useful for defining default configuration.
bool always = 2;
}
}

Expand Down
191 changes: 191 additions & 0 deletions quirks/sommelier-quirks-test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,195 @@ TEST_F(QuirksTest, SetsQuirkAppliedProperty) {
"FEATURE_X11_MOVE_WINDOWS");
}

TEST_F(QuirksTest, ConditionAlwaysAppliesToAllWindows) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;
sl_window* game456 = CreateToplevelWindow();
game456->steam_game_id = 456;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
EXPECT_TRUE(ctx.quirks.IsEnabled(game456, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysFalseDoesNotMatchToAnything) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;
sl_window* game456 = CreateToplevelWindow();
game456->steam_game_id = 456;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: false }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_FALSE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
EXPECT_FALSE(ctx.quirks.IsEnabled(game456, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysTrueButSteamIdFalse) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { steam_game_id: 123 }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_FALSE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysDisableButSteamIdEnable) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { steam_game_id: 123 }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysSteamIdBothEnable) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { steam_game_id: 123 }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysSteamIdBothDisable) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { steam_game_id: 123 }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_FALSE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysAfterSteamId) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { steam_game_id: 123 }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysSandwiched) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;
sl_window* game456 = CreateToplevelWindow();
game456->steam_game_id = 456;

ctx.quirks.Load(
"sommelier { \n"
" condition { steam_game_id: 123 }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { steam_game_id: 456 }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}");

EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
EXPECT_FALSE(ctx.quirks.IsEnabled(game456, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysMultiple) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_BLACK_SCREEN_FIX\n"
"}");

EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_BLACK_SCREEN_FIX));
}

TEST_F(QuirksTest, ConditionAlwaysMultipleLastDefinedHasPriority1) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { always: true }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}");
EXPECT_FALSE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
}

TEST_F(QuirksTest, ConditionAlwaysMultipleLastDefinedHasPriority2) {
sl_window* game123 = CreateToplevelWindow();
game123->steam_game_id = 123;

ctx.quirks.Load(
"sommelier { \n"
" condition { always: true }\n"
" disable: FEATURE_X11_MOVE_WINDOWS\n"
"}\n"
"sommelier { \n"
" condition { always: true }\n"
" enable: FEATURE_X11_MOVE_WINDOWS\n"
"}");
EXPECT_TRUE(ctx.quirks.IsEnabled(game123, quirks::FEATURE_X11_MOVE_WINDOWS));
}

} // namespace vm_tools::sommelier
63 changes: 49 additions & 14 deletions quirks/sommelier-quirks.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <google/protobuf/text_format.h>
#include <unistd.h>
#include <xcb/xproto.h>
#include <map>

#include "quirks/quirks.pb.h"
#include "quirks/sommelier-quirks.h"
Expand Down Expand Up @@ -49,9 +50,28 @@ void Quirks::LoadFromFile(std::string path) {
}

bool Quirks::IsEnabled(struct sl_window* window, int feature) {
bool is_enabled =
enabled_features_.find(std::make_pair(window->steam_game_id, feature)) !=
enabled_features_.end();
bool is_enabled = false;

// Check if feature is enabled while minimizing map looks ups as much as
// possible. |feature_to_steam_id_[feature][steam_id]| takes priority over
// |always_features_[feature]|. This allows rule ordering to take priority
// as we delete |feature_to_steam_id_[feature]| when
// |always_features_[feature]| is popoulated.
bool steam_id_override = false;
auto steam_id_iter = feature_to_steam_id_.find(feature);
if (steam_id_iter != feature_to_steam_id_.end()) {
auto enabled_iter = steam_id_iter->second.find(window->steam_game_id);
if (enabled_iter != steam_id_iter->second.end()) {
steam_id_override = true;
is_enabled = enabled_iter->second;
}
}
if (!steam_id_override) {
auto always_iter = always_features_.find(feature);
if (always_iter != always_features_.end()) {
is_enabled = always_iter->second;
}
}

// Log enabled quirks once per quirk, per window.
if (is_enabled && sl_window_should_log_quirk(window, feature)) {
Expand Down Expand Up @@ -80,22 +100,37 @@ bool Quirks::IsEnabled(struct sl_window* window, int feature) {
}

void Quirks::Update() {
enabled_features_.clear();
feature_to_steam_id_.clear();
always_features_.clear();

for (quirks::SommelierRule rule : active_config_.sommelier()) {
// For now, only support a single instance of a single
// steam_game_id condition.
if (rule.condition_size() != 1 ||
!rule.condition()[0].has_steam_game_id()) {
// For now, only support a single instance of a single condition.
if (rule.condition_size() != 1) {
continue;
}
uint32_t id = rule.condition()[0].steam_game_id();
if (rule.condition()[0].has_steam_game_id()) {
uint32_t id = rule.condition()[0].steam_game_id();

for (int feature : rule.enable()) {
enabled_features_.insert(std::make_pair(id, feature));
}
for (int feature : rule.disable()) {
enabled_features_.erase(std::make_pair(id, feature));
for (int feature : rule.enable()) {
feature_to_steam_id_[feature][id] = true;
}
for (int feature : rule.disable()) {
feature_to_steam_id_[feature][id] = false;
}
} else if (rule.condition()[0].has_always() &&
rule.condition()[0].always()) {
for (int feature : rule.enable()) {
always_features_[feature] = true;
// Clear Steam ID definitions defined so far, so that this always rule
// takes priority since it is defined later.
feature_to_steam_id_.erase(feature);
}
for (int feature : rule.disable()) {
always_features_[feature] = false;
// Clear Steam ID definitions defined so far, so that this always rule
// takes priority since it is defined later.
feature_to_steam_id_.erase(feature);
}
}
}
}
13 changes: 8 additions & 5 deletions quirks/sommelier-quirks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#ifndef VM_TOOLS_SOMMELIER_QUIRKS_SOMMELIER_QUIRKS_H_
#define VM_TOOLS_SOMMELIER_QUIRKS_SOMMELIER_QUIRKS_H_

#include <set>
#include <map>
#include <string>
#include <utility>

Expand Down Expand Up @@ -35,10 +35,13 @@ class Quirks {
quirks::Config active_config_;

// The active config in a more easily queryable form.
//
// Each pair is built from a Steam Game ID and a Feature enum, indicating
// that the Feature is enabled for windows with that STEAM_GAME property.
std::set<std::pair<uint32_t, int>> enabled_features_;
// This is a map of Feature ID (see quirks.proto) to Steam game ID to
// enabled boolean. The boolean represents if the Feature is enabled for
// the windows with that STEAM_GAME property - note that false explicitly
// disables the property.
std::map<int, std::map<uint32_t, bool>> feature_to_steam_id_;

std::map<int, bool> always_features_;
};

#endif // VM_TOOLS_SOMMELIER_QUIRKS_SOMMELIER_QUIRKS_H_

0 comments on commit b042bd2

Please sign in to comment.