From 1aff1347e3d8a319e028c7ce160b0b02a81aa1f6 Mon Sep 17 00:00:00 2001 From: Geoffrey Coulaud Date: Mon, 31 Jul 2023 18:44:18 +0200 Subject: [PATCH] Managers refactor (#164) The main reason for this is the compositing of both local and online covers with the same logic. It was a problem raised in #146 with some covers getting stretched. Changes: - Renamed and simplified managers methods - Created a generic `cover manager` - Added more retryable errors to `steam api manager` - Removed `local cover manager` and `online cover manager` - Reduced dependency on `PIL` --- src/main.py | 6 +- src/store/managers/async_manager.py | 2 +- src/store/managers/cover_manager.py | 197 +++++++++++++++++++++ src/store/managers/display_manager.py | 2 +- src/store/managers/file_manager.py | 2 +- src/store/managers/local_cover_manager.py | 74 -------- src/store/managers/manager.py | 8 +- src/store/managers/online_cover_manager.py | 126 ------------- src/store/managers/sgdb_manager.py | 7 +- src/store/managers/steam_api_manager.py | 5 +- src/store/store.py | 2 +- src/utils/steamgriddb.py | 4 +- 12 files changed, 215 insertions(+), 220 deletions(-) create mode 100644 src/store/managers/cover_manager.py delete mode 100644 src/store/managers/local_cover_manager.py delete mode 100644 src/store/managers/online_cover_manager.py diff --git a/src/main.py b/src/main.py index 949029d..1bad1ac 100644 --- a/src/main.py +++ b/src/main.py @@ -45,8 +45,7 @@ from src.logging.setup import log_system_info, setup_logging from src.preferences import PreferencesWindow from src.store.managers.display_manager import DisplayManager from src.store.managers.file_manager import FileManager -from src.store.managers.local_cover_manager import LocalCoverManager -from src.store.managers.online_cover_manager import OnlineCoverManager +from src.store.managers.cover_manager import CoverManager from src.store.managers.sgdb_manager import SGDBManager from src.store.managers.steam_api_manager import SteamAPIManager from src.store.store import Store @@ -97,9 +96,8 @@ class CartridgesApplication(Adw.Application): self.load_games_from_disk() # Add rest of the managers for game imports - shared.store.add_manager(LocalCoverManager()) + shared.store.add_manager(CoverManager()) shared.store.add_manager(SteamAPIManager()) - shared.store.add_manager(OnlineCoverManager()) shared.store.add_manager(SGDBManager()) shared.store.toggle_manager_in_pipelines(FileManager, True) diff --git a/src/store/managers/async_manager.py b/src/store/managers/async_manager.py index 153ce82..c7f2ff8 100644 --- a/src/store/managers/async_manager.py +++ b/src/store/managers/async_manager.py @@ -56,7 +56,7 @@ class AsyncManager(Manager): def _task_thread_func(self, _task, _source_object, data, _cancellable): """Task thread entry point""" game, additional_data, *_rest = data - self.execute_resilient_manager_logic(game, additional_data) + self.run(game, additional_data) def _task_callback(self, _source_object, _result, data): """Method run after the task is done""" diff --git a/src/store/managers/cover_manager.py b/src/store/managers/cover_manager.py new file mode 100644 index 0000000..4496e80 --- /dev/null +++ b/src/store/managers/cover_manager.py @@ -0,0 +1,197 @@ +# local_cover_manager.py +# +# Copyright 2023 Geoffrey Coulaud +# Copyright 2023 kramo +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# SPDX-License-Identifier: GPL-3.0-or-later + +from pathlib import Path +from typing import NamedTuple + +import requests +from gi.repository import Gio, GdkPixbuf +from requests.exceptions import HTTPError, SSLError + +from src import shared +from src.game import Game +from src.store.managers.manager import Manager +from src.store.managers.steam_api_manager import SteamAPIManager +from src.utils.save_cover import resize_cover, save_cover + + +class ImageSize(NamedTuple): + width: float = 0 + height: float = 0 + + @property + def aspect_ratio(self) -> float: + return self.width / self.height + + def __str__(self): + return f"{self.width}x{self.height}" + + def __mul__(self, scale: float | int) -> "ImageSize": + return ImageSize( + self.width * scale, + self.height * scale, + ) + + def __truediv__(self, divisor: float | int) -> "ImageSize": + return self * (1 / divisor) + + def __add__(self, other_size: "ImageSize") -> "ImageSize": + return ImageSize( + self.width + other_size.width, + self.height + other_size.height, + ) + + def __sub__(self, other_size: "ImageSize") -> "ImageSize": + return self + (other_size * -1) + + def element_wise_div(self, other_size: "ImageSize") -> "ImageSize": + """Divide every element of self by the equivalent in the other size""" + return ImageSize( + self.width / other_size.width, + self.height / other_size.height, + ) + + def element_wise_mul(self, other_size: "ImageSize") -> "ImageSize": + """Multiply every element of self by the equivalent in the other size""" + return ImageSize( + self.width * other_size.width, + self.height * other_size.height, + ) + + def invert(self) -> "ImageSize": + """Invert the element of self""" + return ImageSize(1, 1).element_wise_div(self) + + +class CoverManager(Manager): + """ + Manager in charge of adding the cover image of the game + + Order of priority is: + 1. local cover + 2. icon cover + 3. online cover + """ + + run_after = (SteamAPIManager,) + retryable_on = (HTTPError, SSLError, ConnectionError) + + def download_image(self, url: str) -> Path: + image_file = Gio.File.new_tmp()[0] + path = Path(image_file.get_path()) + with requests.get(url, timeout=5) as cover: + cover.raise_for_status() + path.write_bytes(cover.content) + return path + + def is_stretchable(self, source_size: ImageSize, cover_size: ImageSize) -> bool: + is_taller = source_size.aspect_ratio < cover_size.aspect_ratio + if is_taller: + return True + max_stretch = 0.12 + resized_height = (1 / source_size.aspect_ratio) * cover_size.width + stretch = 1 - (resized_height / cover_size.height) + return stretch <= max_stretch + + def save_composited_cover( + self, + game: Game, + image_path: Path, + scale: float = 1, + blur_size: ImageSize = ImageSize(2, 2), + ) -> None: + """ + Save the image composited with a background blur. + If the image is stretchable, just stretch it. + + :param game: The game to save the cover for + :param path: Path where the source image is located + :param scale: + Scale of the smalled image side + compared to the corresponding side in the cover + :param blur_size: Size of the downscaled image used for the blur + """ + + # Load source image + source = GdkPixbuf.Pixbuf.new_from_file(str(image_path)) + source_size = ImageSize(source.get_width(), source.get_height()) + cover_size = ImageSize._make(shared.image_size) + + # Stretch if possible + if scale == 1 and self.is_stretchable(source_size, cover_size): + save_cover(game.game_id, resize_cover(pixbuf=source)) + return + + # Create the blurred cover background + # fmt: off + cover = ( + source + .scale_simple(*blur_size, GdkPixbuf.InterpType.BILINEAR) + .scale_simple(*cover_size, GdkPixbuf.InterpType.BILINEAR) + ) + # fmt: on + + # Scale to fit, apply scaling, then center + uniform_scale = scale * min(cover_size.element_wise_div(source_size)) + source_in_cover_size = source_size * uniform_scale + source_in_cover_position = (cover_size - source_in_cover_size) / 2 + + # Center the scaled source image in the cover + source.composite( + cover, + *source_in_cover_position, + *source_in_cover_size, + *source_in_cover_position, + uniform_scale, + uniform_scale, + GdkPixbuf.InterpType.BILINEAR, + 255, + ) + save_cover(game.game_id, resize_cover(pixbuf=cover)) + + def main(self, game: Game, additional_data: dict) -> None: + if game.blacklisted: + return + for key in ( + "local_image_path", + "local_icon_path", + "online_cover_url", + ): + # Get an image path + if not (value := additional_data.get(key)): + continue + if key == "online_cover_url": + image_path = self.download_image(value) + else: + image_path = Path(value) + if not image_path.is_file(): + continue + + # Icon cover + if key == "local_icon_path": + self.save_composited_cover( + game, + image_path, + scale=0.7, + blur_size=ImageSize(1, 2), + ) + return + + self.save_composited_cover(game, image_path) diff --git a/src/store/managers/display_manager.py b/src/store/managers/display_manager.py index 7d3a8f0..a5005a4 100644 --- a/src/store/managers/display_manager.py +++ b/src/store/managers/display_manager.py @@ -30,7 +30,7 @@ class DisplayManager(Manager): run_after = (SteamAPIManager, SGDBManager) signals = {"update-ready"} - def manager_logic(self, game: Game, _additional_data: dict) -> None: + def main(self, game: Game, _additional_data: dict) -> None: if game.get_parent(): game.get_parent().get_parent().remove(game) if game.get_parent(): diff --git a/src/store/managers/file_manager.py b/src/store/managers/file_manager.py index 4caa3b4..8eee69b 100644 --- a/src/store/managers/file_manager.py +++ b/src/store/managers/file_manager.py @@ -31,7 +31,7 @@ class FileManager(AsyncManager): run_after = (SteamAPIManager,) signals = {"save-ready"} - def manager_logic(self, game: Game, additional_data: dict) -> None: + def main(self, game: Game, additional_data: dict) -> None: if additional_data.get("skip_save"): # Skip saving when loading games from disk return diff --git a/src/store/managers/local_cover_manager.py b/src/store/managers/local_cover_manager.py deleted file mode 100644 index 0b95f30..0000000 --- a/src/store/managers/local_cover_manager.py +++ /dev/null @@ -1,74 +0,0 @@ -# local_cover_manager.py -# -# Copyright 2023 Geoffrey Coulaud -# Copyright 2023 kramo -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# -# SPDX-License-Identifier: GPL-3.0-or-later - -import logging - -from gi.repository import GdkPixbuf - -from src import shared -from src.game import Game -from src.store.managers.manager import Manager -from src.store.managers.steam_api_manager import SteamAPIManager -from src.utils.save_cover import resize_cover, save_cover - - -class LocalCoverManager(Manager): - """Manager in charge of adding the local cover image of the game""" - - run_after = (SteamAPIManager,) - - def manager_logic(self, game: Game, additional_data: dict) -> None: - if image_path := additional_data.get("local_image_path"): - if not image_path.is_file(): - logging.error("Local image path is not a file: %s", image_path) - return - save_cover(game.game_id, resize_cover(image_path)) - elif icon_path := additional_data.get("local_icon_path"): - cover_width, cover_height = shared.image_size - - dest_width = cover_width * 0.7 - dest_height = cover_width * 0.7 - - dest_x = cover_width * 0.15 - dest_y = (cover_height - dest_height) / 2 - - image = GdkPixbuf.Pixbuf.new_from_file(str(icon_path)).scale_simple( - dest_width, dest_height, GdkPixbuf.InterpType.BILINEAR - ) - - cover = image.scale_simple( - 1, 2, GdkPixbuf.InterpType.BILINEAR - ).scale_simple(cover_width, cover_height, GdkPixbuf.InterpType.BILINEAR) - - image.composite( - cover, - dest_x, - dest_y, - dest_width, - dest_height, - dest_x, - dest_y, - 1, - 1, - GdkPixbuf.InterpType.BILINEAR, - 255, - ) - - save_cover(game.game_id, resize_cover(pixbuf=cover)) diff --git a/src/store/managers/manager.py b/src/store/managers/manager.py index b1aadf6..4ef50d0 100644 --- a/src/store/managers/manager.py +++ b/src/store/managers/manager.py @@ -50,7 +50,7 @@ class Manager(ErrorProducer): return type(self).__name__ @abstractmethod - def manager_logic(self, game: Game, additional_data: dict) -> None: + def main(self, game: Game, additional_data: dict) -> None: """ Manager specific logic triggered by the run method * Implemented by final child classes @@ -59,7 +59,7 @@ class Manager(ErrorProducer): * May raise other exceptions that will be reported """ - def execute_resilient_manager_logic(self, game: Game, additional_data: dict): + def run(self, game: Game, additional_data: dict): """Handle errors (retry, ignore or raise) that occur in the manager logic""" # Keep track of the number of tries @@ -106,7 +106,7 @@ class Manager(ErrorProducer): def try_manager_logic(): try: - self.manager_logic(game, additional_data) + self.main(game, additional_data) except Exception as error: # pylint: disable=broad-exception-caught handle_error(error) @@ -116,5 +116,5 @@ class Manager(ErrorProducer): self, game: Game, additional_data: dict, callback: Callable[["Manager"], Any] ) -> None: """Pass the game through the manager""" - self.execute_resilient_manager_logic(game, additional_data) + self.run(game, additional_data) callback(self) diff --git a/src/store/managers/online_cover_manager.py b/src/store/managers/online_cover_manager.py deleted file mode 100644 index bc6ea1e..0000000 --- a/src/store/managers/online_cover_manager.py +++ /dev/null @@ -1,126 +0,0 @@ -# online_cover_manager.py -# -# Copyright 2023 Geoffrey Coulaud -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# -# SPDX-License-Identifier: GPL-3.0-or-later - -import logging -from pathlib import Path - -import requests -from gi.repository import Gio, GdkPixbuf -from requests.exceptions import HTTPError, SSLError -from PIL import Image - -from src import shared -from src.game import Game -from src.store.managers.local_cover_manager import LocalCoverManager -from src.store.managers.manager import Manager -from src.utils.save_cover import resize_cover, save_cover - - -class OnlineCoverManager(Manager): - """Manager that downloads game covers from URLs""" - - run_after = (LocalCoverManager,) - retryable_on = (HTTPError, SSLError, ConnectionError) - - def save_composited_cover( - self, - game: Game, - image_file: Gio.File, - original_width: int, - original_height: int, - target_width: int, - target_height: int, - ) -> None: - """Save the image composited with a background blur to fit the cover size""" - - logging.debug( - "Compositing image for %s (%s) %dx%d -> %dx%d", - game.name, - game.game_id, - original_width, - original_height, - target_width, - target_height, - ) - - # Load game image - image = GdkPixbuf.Pixbuf.new_from_stream(image_file.read()) - - # Create background blur of the size of the cover - cover = image.scale_simple(2, 2, GdkPixbuf.InterpType.BILINEAR).scale_simple( - target_width, target_height, GdkPixbuf.InterpType.BILINEAR - ) - - # Center the image above the blurred background - scale = min(target_width / original_width, target_height / original_height) - left_padding = (target_width - original_width * scale) / 2 - top_padding = (target_height - original_height * scale) / 2 - image.composite( - cover, - # Top left of overwritten area on the destination - left_padding, - top_padding, - # Size of the overwritten area on the destination - original_width * scale, - original_height * scale, - # Offset - left_padding, - top_padding, - # Scale to apply to the resized image - scale, - scale, - # Compositing stuff - GdkPixbuf.InterpType.BILINEAR, - 255, - ) - - # Resize and save the cover - save_cover(game.game_id, resize_cover(pixbuf=cover)) - - def manager_logic(self, game: Game, additional_data: dict) -> None: - # Ensure that we have a cover to download - cover_url = additional_data.get("online_cover_url") - if not cover_url: - return - - # Download cover - image_file = Gio.File.new_tmp()[0] - image_path = Path(image_file.get_path()) - with requests.get(cover_url, timeout=5) as cover: - cover.raise_for_status() - image_path.write_bytes(cover.content) - - # Get image size - cover_width, cover_height = shared.image_size - with Image.open(image_path) as pil_image: - width, height = pil_image.size - - # Composite if the image is shorter and the stretch amount is too high - aspect_ratio = width / height - target_aspect_ratio = cover_width / cover_height - is_taller = aspect_ratio < target_aspect_ratio - resized_height = height / width * cover_width - stretch = 1 - (resized_height / cover_height) - max_stretch = 0.12 - if is_taller or stretch <= max_stretch: - save_cover(game.game_id, resize_cover(image_path)) - else: - self.save_composited_cover( - game, image_file, width, height, cover_width, cover_height - ) diff --git a/src/store/managers/sgdb_manager.py b/src/store/managers/sgdb_manager.py index 142495f..5c002cb 100644 --- a/src/store/managers/sgdb_manager.py +++ b/src/store/managers/sgdb_manager.py @@ -24,19 +24,18 @@ from requests.exceptions import HTTPError, SSLError from src.errors.friendly_error import FriendlyError from src.game import Game from src.store.managers.async_manager import AsyncManager -from src.store.managers.local_cover_manager import LocalCoverManager -from src.store.managers.online_cover_manager import OnlineCoverManager from src.store.managers.steam_api_manager import SteamAPIManager +from src.store.managers.cover_manager import CoverManager from src.utils.steamgriddb import SGDBAuthError, SGDBHelper class SGDBManager(AsyncManager): """Manager in charge of downloading a game's cover from steamgriddb""" - run_after = (SteamAPIManager, LocalCoverManager, OnlineCoverManager) + run_after = (SteamAPIManager, CoverManager) retryable_on = (HTTPError, SSLError, ConnectionError, JSONDecodeError) - def manager_logic(self, game: Game, _additional_data: dict) -> None: + def main(self, game: Game, _additional_data: dict) -> None: try: sgdb = SGDBHelper() sgdb.conditionaly_update_cover(game) diff --git a/src/store/managers/steam_api_manager.py b/src/store/managers/steam_api_manager.py index 19b38af..1a62ddd 100644 --- a/src/store/managers/steam_api_manager.py +++ b/src/store/managers/steam_api_manager.py @@ -18,6 +18,7 @@ # SPDX-License-Identifier: GPL-3.0-or-later from requests.exceptions import HTTPError, SSLError +from urllib3.exceptions import ConnectionError as Urllib3ConnectionError from src.game import Game from src.store.managers.async_manager import AsyncManager @@ -32,7 +33,7 @@ from src.utils.steam import ( class SteamAPIManager(AsyncManager): """Manager in charge of completing a game's data from the Steam API""" - retryable_on = (HTTPError, SSLError, ConnectionError) + retryable_on = (HTTPError, SSLError, Urllib3ConnectionError) steam_api_helper: SteamAPIHelper = None steam_rate_limiter: SteamRateLimiter = None @@ -42,7 +43,7 @@ class SteamAPIManager(AsyncManager): self.steam_rate_limiter = SteamRateLimiter() self.steam_api_helper = SteamAPIHelper(self.steam_rate_limiter) - def manager_logic(self, game: Game, additional_data: dict) -> None: + def main(self, game: Game, additional_data: dict) -> None: # Skip non-steam games appid = additional_data.get("steam_appid", None) if appid is None: diff --git a/src/store/store.py b/src/store/store.py index 9da9ef0..231bbdc 100644 --- a/src/store/store.py +++ b/src/store/store.py @@ -130,7 +130,7 @@ class Store: # Connect signals for manager in self.managers.values(): for signal in manager.signals: - game.connect(signal, manager.execute_resilient_manager_logic) + game.connect(signal, manager.run) # Add the game to the store if not game.source in self.source_games: diff --git a/src/utils/steamgriddb.py b/src/utils/steamgriddb.py index 9cd9c3c..57dda3e 100644 --- a/src/utils/steamgriddb.py +++ b/src/utils/steamgriddb.py @@ -103,11 +103,11 @@ class SGDBHelper: image_trunk = shared.covers_dir / game.game_id still = image_trunk.with_suffix(".tiff") - uri_kwargs = image_trunk.with_suffix(".gif") + animated = image_trunk.with_suffix(".gif") prefer_sgdb = shared.schema.get_boolean("sgdb-prefer") # Do nothing if file present and not prefer SGDB - if not prefer_sgdb and (still.is_file() or uri_kwargs.is_file()): + if not prefer_sgdb and (still.is_file() or animated.is_file()): return # Get ID for the game