fix(compositor): rendre le repli CPU Linux forçable, testé et garanti - #223
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughLinux compositor support now includes explicit hardware and CPU backend selection, automatic CPU fallback, adapter diagnostics, Linux validation, CI coverage, and Vulkan runtime packaging. GIF export uses automatic GPU creation. ChangesLinux compositor support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant create_auto
participant create_backend
participant VulkanAdapter
create_auto->>create_backend: request hardware or CPU backend
create_backend->>VulkanAdapter: create adapter with fallback settings
VulkanAdapter-->>create_backend: adapter details and device type
create_backend-->>create_auto: GPU or diagnostic error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 235-242: Update the “Resolve the toolchain paths” step to derive
libclang from the installed libclang-dev package, using dpkg -L (or an
equivalent deterministic highest-version selection) instead of find with head
-1. Preserve the existing empty-path validation and LIBCLANG_PATH export, while
ensuring the selected library matches the freshly installed package.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a1a40529-548d-4a57-a59b-0476492201e9
📒 Files selected for processing (7)
.github/workflows/ci.ymlcrates/compositor-view-napi/src/lib.rscrates/compositor/src/d3d_linux.rscrates/compositor/tests/cpu_backend_linux.rscrates/compositor/tests/export_timing.rscrates/compositor/tests/output_geometry_golden.rselectron-builder.json5
…Linux `find | head -1` prenait le premier résultat dans l'ordre de parcours du système de fichiers, qui n'est pas trié. L'image ubuntu-latest embarque plusieurs LLVM : si l'un d'eux a un paquet -dev préinstallé, on pouvait sélectionner une version autre que celle qu'apt vient d'installer. Les headers built-in de clang étant liés à la version de libclang, le symptôme aurait été `stddef.h file not found` — une erreur qui ne désigne pas sa cause. `sort -V | tail -1` prend la plus récente, de façon déterministe. Remonté par CodeRabbit sur #223.
Le repli logiciel existait déjà sur Linux, mais par accident : `create_backend` ignorait son paramètre et wgpu rendait lavapipe de lui-même quand c'était le seul ICD. Ça marche — et c'est précisément le problème, parce que rien ne pouvait l'exercer, rien ne le signalait et rien ne le garantissait. - `create_backend` honore enfin son paramètre. `Backend::Cpu` passe par `force_fallback_adapter` ; `Backend::Hardware` rejette explicitement un adaptateur logiciel, ce qui rend `create` réellement matériel strict (un golden mesuré sur llvmpipe passait jusqu'ici pour une mesure GPU). `create_auto` gagne le repli explicite Hardware -> Cpu, comme côté Windows. - `OPENSCREEN_COMPOSITOR_BACKEND=hardware|cpu` force le choix. `VK_DRIVER_FILES` ferait la même chose au niveau du loader Vulkan, mais s'applique au processus entier : sous Electron il prive aussi Chromium de son GPU, qui rastérise alors toute son UI sur CPU et sature la machine. Le chemin CPU n'était donc pas testable sans casser la session. Même motif que `OPENSCREEN_EXPORT_ENCODER`. - L'adaptateur retenu est journalisé. Windows loggue son repli, Linux ne loggait rien : un hôte tombé sur lavapipe rendait à quelques fps sans que rien ne permette de l'établir à distance. - `diagnose()` sépare « aucun ICD Vulkan installé » du reste et nomme le paquet à installer. C'est la seule panne de cette famille que l'utilisateur peut réparer lui-même, et elle s'affichait en « Aperçu indisponible sur cette machine ». - `classify` s'appuie sur `DeviceType::Cpu` — ce que l'ICD déclare — plutôt que sur une sous-chaîne du nom ; le nom reste en filet. - Le .deb et le pacman déclarent Mesa (`mesa-vulkan-drivers` / `vulkan-swrast`). Aucune dépendance n'était déclarée, donc rien ne garantissait qu'un ICD existe. - Nouveau job CI `Rust test (Linux compositor)`. Le Rust Linux n'était compilé nulle part : la CI couvrait macOS et Windows, et les 2154 lignes du moteur wgpu ne passaient que par le poste des contributeurs. Le job installe lavapipe, donc il exerce pour de vrai le backend CPU sur un runner sans GPU. Deux corrections que ce job a révélées, et sans lesquelles il ne peut pas exister : - `export_timing.rs` et `output_geometry_golden.rs` ne compilaient pas sous Linux (`probe_frame_count` / `readback_resized` n'existent que côté Windows et macOS). Les fichiers de `tests/` étant compilés quelle que soit la plateforme, le crate entier était incompilable en `--tests` sur Linux, en silence. Gardés en `cfg(not(target_os = "linux"))` plutôt qu'en `cfg(windows)`, pour ne pas les retirer du job macOS qui les compile aujourd'hui. - L'export GIF construisait son device avec `Gpu::create` (matériel strict) alors que son propre commentaire dit suivre l'export MP4, qui prend `create_auto`. Un hôte sans GPU exportait donc un MP4 mais pas un GIF — sur le seul chemin où le backend CPU existe précisément pour que l'export aboutisse. Le rendre strict sur Linux sans ce correctif aurait cassé le GIF sur les machines lavapipe qui fonctionnent aujourd'hui.
…Linux `find | head -1` prenait le premier résultat dans l'ordre de parcours du système de fichiers, qui n'est pas trié. L'image ubuntu-latest embarque plusieurs LLVM : si l'un d'eux a un paquet -dev préinstallé, on pouvait sélectionner une version autre que celle qu'apt vient d'installer. Les headers built-in de clang étant liés à la version de libclang, le symptôme aurait été `stddef.h file not found` — une erreur qui ne désigne pas sa cause. `sort -V | tail -1` prend la plus récente, de façon déterministe. Remonté par CodeRabbit sur #223.
aeaa754 to
7ceb892
Compare
…Linux `find | head -1` prenait le premier résultat dans l'ordre de parcours du système de fichiers, qui n'est pas trié. L'image ubuntu-latest embarque plusieurs LLVM : si l'un d'eux a un paquet -dev préinstallé, on pouvait sélectionner une version autre que celle qu'apt vient d'installer. Les headers built-in de clang étant liés à la version de libclang, le symptôme aurait été `stddef.h file not found` — une erreur qui ne désigne pas sa cause. `sort -V | tail -1` prend la plus récente, de façon déterministe. Remonté par CodeRabbit sur #223.
Summary
Le repli logiciel existait déjà sur Linux — mais par accident.
create_backendignorait son paramètre, et wgpu rendait lavapipe de lui-même quand c'était le seul ICD. Ça marche, et c'est exactement le problème : rien ne pouvait l'exercer, rien ne le signalait, rien ne le garantissait.Trois conséquences, toutes fermées ici.
1. Le chemin CPU n'était pas testable
create_backend(_backend)ignorait son argument : impossible de demander lavapipe sur une machine qui a un GPU. Le seul levier restant étaitVK_DRIVER_FILES, qui agit sur le processus entier — sous Electron il prive aussi Chromium de son GPU, qui rastérise alors toute son UI sur CPU et sature la machine. Vérifié à mes dépens pendant cette PR : le test est inexploitable et emporte les autres applications Electron du poste.Backend::Cpupasse désormais parforce_fallback_adapter,Backend::Hardwarerejette explicitement un adaptateur logiciel.OPENSCREEN_COMPOSITOR_BACKEND=hardware|cpuforce le choix sans toucher au loader Vulkan — seul notre compositeur bascule. Même motif queOPENSCREEN_EXPORT_ENCODER.createdevient réellement matériel strict. Un golden mesuré sur llvmpipe passait jusqu'ici pour une mesure GPU.2. Rien ne le signalait
Windows loggue son repli ; Linux ne loggait rien, et
diagnose()n'était queformat!("{err:#}"). Un hôte tombé sur lavapipe rendait à quelques fps sans qu'aucun log ne permette de l'établir à distance.diagnose()sépare « aucun ICD Vulkan installé » du reste et nomme le paquet à installer. C'est la seule panne de cette famille que l'utilisateur peut réparer lui-même, et elle s'affichait en « Aperçu indisponible sur cette machine », sans piste.classifys'appuie surDeviceType::Cpu— ce que l'ICD déclare — plutôt que sur une sous-chaîne du nom, qui reste en filet.3. Rien ne le garantissait
electron-builder.json5ne déclarait aucune dépendance. Sansmesa-vulkan-drivers, aucun adaptateur :probe()répond"none", que le TS traite comme « pas d'addon » et qui n'affiche donc aucune notice..deb→mesa-vulkan-drivers,pacman→vulkan-swrast.dependsremplace la liste par défaut d'electron-builder au lieu de s'y ajouter, donc les valeurs par défaut sont reprises verbatim, la nôtre en dernier.diagnose()nomme le paquet.Le job CI qui manquait
Le Rust Linux n'était compilé nulle part :
ci.ymlcouvrait macOS (test) et Windows (check), et les 2154 lignes du moteur wgpu ne passaient que par le poste des contributeurs. Le nouveau job installe lavapipe, donc il exerce pour de vrai le backend CPU sur un runner sans GPU —OPENSCREEN_REQUIRE_CPU_BACKEND=1le fait échouer plutôt que sauter s'il ne l'obtient pas.Il a immédiatement trouvé deux choses, sans lesquelles il ne peut pas exister :
export_timing.rsetoutput_geometry_golden.rsne compilaient pas sous Linux — ils appellentprobe_frame_count/readback_resized, qui n'existent que côté Windows et macOS. Les fichiers detests/étant compilés quelle que soit la plateforme, le crate entier était incompilable en--testssur Linux, en silence. Gardés encfg(not(target_os = "linux"))et noncfg(windows), pour ne pas les retirer du job macOS qui les compile aujourd'hui.Gpu::create(matériel strict) alors que son propre commentaire dit suivre l'export MP4, qui prendcreate_auto. Un hôte sans GPU exportait donc un MP4 mais pas un GIF — sur le seul chemin où le backend CPU existe précisément pour que l'export aboutisse. Bug pré-existant, y compris sur Windows ; le corriger était aussi nécessaire pour quecreatedevienne strict sans casser les machines lavapipe qui fonctionnent aujourd'hui.Ce que cette PR ne fait pas
Elle ne construit pas un backend CPU : elle rend vérifiable et garanti celui qui existait. Elle ne touche ni au décodage ni à l'encodage Linux, tous deux logiciels par construction (pas de VAAPI) — c'est un chantier distinct.
Related issue
n/a — issu de l'audit de l'état de #162, dont les 4 commits sont sur
maindepuis le port multi-plateforme.Type of change
Release impact
Desktop impact
Testing
Sur Ubuntu 24.04, AMD Radeon 610M (RADV RAPHAEL_MENDOCINO) + lavapipe installé :
Le forçage vérifié sur une machine qui a un GPU, sans toucher au Vulkan de la session :
cargo build -p compositor-view-napi --releasepasse également.Non vérifié localement : le packaging. Les listes
dependssont reprises deapp-builder-lib(FpmTarget.getDefaultDepends, electron-builder 26.8.1) mais aucun.debni.pacmann'a été construit ici — à confirmer sur le premier build de release.Summary by CodeRabbit
New Features
Bug Fixes
Tests