From af836fb0695af3ccfff2636180bc34c019a734af Mon Sep 17 00:00:00 2001 From: jose luis <2064537+sosan@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:25:45 +0100 Subject: [PATCH] fix(linux/x11): reintento silencioso de not_viewable y comentarios honestos (#212) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tres correcciones de la issue #212 (review de #205): 1. El `not_viewable` tras el `show()` ya no marca `placementFailed` de inmediato: se reintenta en silencio con un presupuesto acotado (`retryNotViewable`, `NOT_VIEWABLE_RETRIES=3` con la misma espera del backend) y solo si se agota se informa del fallo. El `show()` es asíncrono y con gestores con animaciones (GNOME/Mutter) el mapeo puede tardar más del presupuesto de lectura del backend: un `not_viewable` seguido de una lectura correcta es normal, no un error. Tests deterministas: lectura posterior con éxito → no fallo; presupuesto agotado → fallo; un `size_refused` no se reintenta. 2. `apply_size` deja de fingir que confirma el resize: `set_resizable` es `void` (no puede fallar) y `is_resizable()` relee la propiedad local que acabamos de escribir — un tautograma, no una confirmación del gestor. El doc y el call site ahora dicen que la confirmación real es el read-back posterior del compositor. El test en vivo observa tamaño y permiso tras la llamada, no el retorno. 3. `rooted_origin` documenta lo que GDK3 garantiza: el retorno de `gdk_window_get_origin` es "Not meaningful, ignore" — solo distingue la ventana destruida (0) de la viva (≠0), no valida la traducción de coordenadas. El call site ya no dice que "si falla, no hay lectura". Co-Authored-By: Claude Code --- src-tauri/src/widget_placement.rs | 44 +++++++++++++++++------ src-tauri/src/widget_placement_test.rs | 8 ++--- src/Widget.tsx | 15 ++++++-- src/lib/widget-placement.test.ts | 50 ++++++++++++++++++++++++++ src/lib/widget-placement.ts | 44 +++++++++++++++++++++++ 5 files changed, 142 insertions(+), 19 deletions(-) diff --git a/src-tauri/src/widget_placement.rs b/src-tauri/src/widget_placement.rs index c04c6d0..a318e87 100644 --- a/src-tauri/src/widget_placement.rs +++ b/src-tauri/src/widget_placement.rs @@ -21,7 +21,10 @@ //! visibilidad es del frontend: el backend nunca muestra la ventana. Los //! read-backs salen de GDK: la señal de visibilidad es //! `gdk_window_is_viewable()` y el origen sale de `gdk_window_get_origin` -//! (relativo a la raíz): si esa llamada falla, la lectura no es utilizable. +//! (relativo a la raíz). El valor de retorno de `get_origin` no confirma la +//! llamada (GDK3: "Not meaningful, ignore"); la geometría que se informa se +//! valida contra el read-back del compositor, que es la confirmación real +//! (revisión #212). use serde::{Deserialize, Serialize}; use std::future::Future; @@ -226,6 +229,15 @@ pub fn to_logical(physical: i32, scale: f64) -> i32 { /// (`gdk_window_get_geometry`) es **relativo al padre** —el marco de la /// decoración si el gestor reparenta la ventana— y devolverlo mezclaría dos /// espacios de coordenadas: la lectura no es utilizable. +/// +/// `ok` es el valor de retorno de `gdk_window_get_origin`, que GDK3 +/// documenta como *"Not meaningful, ignore"*: **no** confirma que la +/// traducción de coordenadas haya funcionado. Sólo distingue la ventana +/// destruida (0) de la ventana viva (≠ 0), y en ese caso el origen +/// reportado es el que el servidor X tenía en el último +/// `GdkEventConfigure` procesado —puede ir retrasado respecto al estado +/// real del servidor. La geometría que se informa se valida después +/// contra el read-back del compositor (revisión #212). pub fn rooted_origin(x: i32, y: i32, ok: bool) -> Option<(i32, i32)> { if ok { Some((x, y)) @@ -601,17 +613,20 @@ pub fn fix_size( /// arrastre de borde. La petición de tamaño del widget conserva el tamaño /// concedido cuando GTK recalcula la configuración. /// -/// Devuelve `true` si el permiso quedó cerrado: el ciclo no puede seguir como -/// si el resize se hubiera hecho si no se puede confirmar. +/// No hay confirmación posible aquí: `gtk_window_set_resizable` es `void` +/// (no puede fallar) y `is_resizable()` relee la propiedad local que acaba +/// de escribir — no es una confirmación del gestor (revisión #212). Lo que +/// de verdad confirma la geometría es el **read-back posterior del +/// compositor** (el tamaño y la posición que quedaron en pantalla), no esta +/// llamada. #[cfg(target_os = "linux")] pub fn apply_size( window: &W, width: i32, height: i32, -) -> bool { +) { fix_size(window, width, height); window.set_resizable(false); - !window.is_resizable() } /// Corre una llamada nativa en el hilo de UI y espera su resultado. @@ -682,10 +697,15 @@ impl Surface for UiSurface { } // La pista de tamaño, las pistas `min == max` y el resize, con el // permiso de resize abierto **sólo** para el resize: la misma - // llamada lo retira y lo confirma. - let closed = apply_size(window, w, h); - // La confirmación: el permiso tiene que quedar cerrado. - closed + // llamada lo retira. + // + // Esto NO confirma la geometría: `set_resizable` no puede fallar + // y `is_resizable()` solo relee la propiedad local (revisión + // #212). La confirmación real es el read-back posterior del + // compositor, que informa del tamaño y la posición que quedaron + // en pantalla. + apply_size(window, w, h); + true }) .await .unwrap_or(false) @@ -749,8 +769,10 @@ impl Surface for UiSurface { let (_, _, gw, gh) = gdk_window.geometry(); let size = (to_physical(gw, scale), to_physical(gh, scale)); // El origen: `gdk_window_get_origin` da las coordenadas relativas - // a la raíz (que es lo que se informa) y su valor de retorno dice - // si la llamada tuvo éxito. Si falla, **no** hay origen usable: + // a la raíz (que es lo que se informa). Su valor de retorno NO + // confirma que la llamada funcionó (GDK3: "Not meaningful, + // ignore"): sólo distingue la ventana destruida de la ventana + // viva. Si la ventana está destruida, **no** hay origen usable: // el respaldo de `gdk_window_get_geometry` es relativo al padre // (el marco de la decoración si el gestor reparenta la ventana) y // mezclaría dos espacios de coordenadas. diff --git a/src-tauri/src/widget_placement_test.rs b/src-tauri/src/widget_placement_test.rs index 6fc3fa7..a22d37a 100644 --- a/src-tauri/src/widget_placement_test.rs +++ b/src-tauri/src/widget_placement_test.rs @@ -871,11 +871,9 @@ fn the_native_sequence_grants_the_size_and_leaves_no_resize_permission() { // La secuencia de producción: la pista fija el tamaño y el permiso de // resize se abre **sólo** alrededor del resize programático (la misma - // llamada lo retira). - assert!( - crate::widget_placement::apply_size(&window, 61, 181), - "el permiso de resize queda cerrado" - ); + // llamada lo retira). `apply_size` no devuelve confirmación: lo que + // verifica el ciclo es el tamaño y el permiso observados después (#212). + crate::widget_placement::apply_size(&window, 61, 181); settle(600); assert_eq!(window.size(), (61, 181), "el tamaño se concede exacto"); assert!(!window.is_resizable(), "el permiso está cerrado"); diff --git a/src/Widget.tsx b/src/Widget.tsx index e03a0dc..e574f98 100644 --- a/src/Widget.tsx +++ b/src/Widget.tsx @@ -16,7 +16,7 @@ import { retryWithBackoff, STARTUP_RETRY, USAGE_RETRY } from './lib/startup'; import { createSessionMonitor } from './lib/session-monitor'; import { needsRefresh } from './lib/widget-refresh'; import { createWidgetRecovery, shouldAuditNativePosition } from './lib/widget-recovery'; -import { placementOutcome, reasonKey, samePlacement, type WidgetPlacementReason, type WidgetPlacementReport } from './lib/widget-placement'; +import { placementOutcome, reasonKey, retryNotViewable, samePlacement, type WidgetPlacementReason, type WidgetPlacementReport } from './lib/widget-placement'; import { createLatestQueue, physicalPanelBounds, samePhysical, DETAIL_EVENTS, DETAIL_WINDOW, isCurrentDetailInteraction, panelWidthFor, type DetailInteraction, type DetailState, type PhysicalBounds } from './lib/detail-panel'; import { createDetailPresence, presenceFromPoint, shouldSampleNativePresence } from './lib/detail-presence'; import { openDelayForOs, osFromUA } from './lib/os-detect'; @@ -526,6 +526,7 @@ export function Widget() { if (disposed || !current()) return; let limited = false; let reason: WidgetPlacementReason | null = null; + let lastReport: WidgetPlacementReport | null = null; // La comparación con la geometría cacheada usa la tolerancia del // backend **sólo** si esa geometría vino del read-back de X11: en el // resto de plataformas un cambio de 1 px sí es un cambio (#179). @@ -533,6 +534,7 @@ export function Widget() { // El backend es la autoridad: responde con la geometría que quedó en // pantalla, no con la solicitud encolada (#179). const apply = (report: WidgetPlacementReport) => { + lastReport = report; const outcome = placementOutcome(report, physical); // Sólo se cachea geometría fiable: una colocación cancelada (quedó a // medias) o una lectura fallida dejan la caché vacía. @@ -565,8 +567,15 @@ export function Widget() { } // El backend no puede leer una ventana que todavía no es visible: se // vuelve a pedir la colocación para confirmar la geometría (#179). - if (!same && reason === 'not_viewable' && !hiddenByUser.current) { - const report = await api.setWidgetBounds(physical); + // Un `not_viewable` no es un fallo del gestor sino "todavía no": se + // reintenta en silencio con un presupuesto acotado y sólo se marca el + // fallo si se agota (#212). + if (!same && reason === 'not_viewable' && lastReport && !hiddenByUser.current) { + const report = await retryNotViewable( + lastReport, + () => api.setWidgetBounds(physical), + (ms) => new Promise((resolve) => window.setTimeout(resolve, ms)), + ); if (disposed || !current()) return; apply(report); } diff --git a/src/lib/widget-placement.test.ts b/src/lib/widget-placement.test.ts index 1ac4199..c740825 100644 --- a/src/lib/widget-placement.test.ts +++ b/src/lib/widget-placement.test.ts @@ -7,8 +7,10 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { t } from '../i18n'; import { + NOT_VIEWABLE_RETRIES, placementOutcome, reasonKey, + retryNotViewable, samePhysicalWithin, samePlacement, type WidgetPlacementReason, @@ -100,6 +102,54 @@ describe('placementOutcome', () => { }); }); +describe('the not_viewable silent retry (#212)', () => { + const notViewable: WidgetPlacementReport = { + status: 'unverified', + bounds: null, + reason: 'not_viewable', + read_back: false, + }; + const applied = report(); + + it('retries in silence and reports success when a later read succeeds', async () => { + // La ventana aún no se había mapeado (informe `not_viewable`), pero la + // lectura posterior sí ve la geometría. El resultado final no es un + // fallo: el reintento silencioso lo absorbe (#212). + const request = vi.fn(async () => applied); + const report = await retryNotViewable(notViewable, request, async () => {}); + expect(report.status).toBe('applied'); + expect(report.reason ?? null).toBeNull(); + // Se terminó al ver la geometría: no se insistió. + expect(request).toHaveBeenCalledTimes(1); + }); + + it('does not retry when the window was visible (no not_viewable)', async () => { + const request = vi.fn<() => Promise>(); + const report = await retryNotViewable(applied, request, async () => {}); + expect(report.status).toBe('applied'); + expect(request).not.toHaveBeenCalled(); + }); + + it('bails out and keeps the failure when the budget is exhausted', async () => { + // El gestor nunca llega a mapear la ventana dentro del presupuesto: el + // último informe sigue siendo `not_viewable` y el widget marca el fallo. + const request = vi.fn(async () => notViewable); + const report = await retryNotViewable(notViewable, request, async () => {}); + expect(report.reason).toBe('not_viewable'); + expect(request).toHaveBeenCalledTimes(NOT_VIEWABLE_RETRIES); + }); + + it('stops retrying as soon as a non-not_viewable report arrives', async () => { + const request = vi.fn(async () => + report({ status: 'limited', bounds: { x: 0, y: 0, w: 320, h: 260 }, reason: 'size_refused' }), + ); + const result = await retryNotViewable(notViewable, request, async () => {}); + expect(result.reason).toBe('size_refused'); + // Un `size_refused` no es "todavía no": no hay que volver a intentar. + expect(request).toHaveBeenCalledTimes(1); + }); +}); + describe('the reason codes', () => { it('maps every backend code to its catalog key', () => { expect(reasonKey('size_refused')).toBe('widget.reasonSizeRefused'); diff --git a/src/lib/widget-placement.ts b/src/lib/widget-placement.ts index 260f059..cf80b1f 100644 --- a/src/lib/widget-placement.ts +++ b/src/lib/widget-placement.ts @@ -80,6 +80,21 @@ export const reasonKey = (reason?: WidgetPlacementReason | null): MessageKey | n */ export const PLACEMENT_TOLERANCE = 1; +/** + * Reintentos silenciosos cuando la ventana nativa todavía no es visible. + * + * `not_viewable` no es un fallo del gestor: es que la ventana aún no se ha + * mapeado. El `show()` es asíncrono y con animaciones (GNOME/Mutter) el + * gestor puede tardar más que el presupuesto de lectura del backend, así que + * un `not_viewable` seguido de una lectura correcta es normal, no un error. + * Se reintenta en silencio y sólo si el presupuesto se agota se informa del + * fallo (#212). + */ +export const NOT_VIEWABLE_RETRIES = 3; + +/** Espera entre reintentos: la misma que el backend usa entre lecturas. */ +export const NOT_VIEWABLE_RETRY_DELAY_MS = 60; + /** True cuando dos geometrías son la misma dentro de la tolerancia. */ export const samePhysicalWithin = ( a: PhysicalBounds | null, @@ -159,3 +174,32 @@ export const placementOutcome = ( readBack: report.read_back === true, }; }; + +/** + * Reintenta en silencio una colocación que el backend no pudo leer porque la + * ventana todavía no era visible, y devuelve el último informe. + * + * El primer `not_viewable` **no es un fallo**: el `show()` es asíncrono y el + * gestor puede tardar más que su presupuesto de lectura. Se vuelve a pedir la + * colocación con un límite acotado; si una lectura posterior tiene éxito, el + * informe final es el bueno y no se marca el fallo (#212). Sólo cuando se + * agota el presupuesto se devuelve el `not_viewable` de verdad, que el widget + * traduce como motivo de reintento manual. + * + * El reintento es acotado por diseño: un gestor que nunca mapea la ventana + * no puede dejar la tira reintentando sin fin. + */ +export const retryNotViewable = async ( + report: WidgetPlacementReport, + request: () => Promise, + wait: (ms: number) => Promise, + retries: number = NOT_VIEWABLE_RETRIES, +): Promise => { + let current = report; + for (let attempt = 0; attempt < retries; attempt += 1) { + if (current.reason !== 'not_viewable') return current; + await wait(NOT_VIEWABLE_RETRY_DELAY_MS); + current = await request(); + } + return current; +};