Skip to content

SVG import: tspan line breaks follow the baseline gap (none on the same baseline, bounded far gaps) - #8

Merged
echelon merged 1 commit into
storytold:mainfrom
storybored379:pr/svg-import-tspan-line-gap
Oct 4, 2026
Merged

echelon merged 1 commit into
storytold:mainfrom
storybored379:pr/svg-import-tspan-line-gap

Conversation

@storybored379

Copy link
Copy Markdown
Contributor

The SVG text fallback turns the gap between an absolutely positioned <tspan y> and the previous line into
that many line breaks. That has two problems:

  • At least one break was added even for a tspan on the current baseline. Styled or kerned runs
    written with x and y on every tspan, as Illustrator writes them
    (<tspan x="0" y="0">Hello </tspan><tspan x="55.6" y="0" fill="red">world</tspan>), came back as one line per run:
    "Hello \nworld".
  • The count had no upper bound. A 150-byte SVG with <tspan y="1e15"> asks for about 7·10¹³ '\n' characters.
    Opening it (File ▸ Open, MCP open_file or vectorcraft-cli convert) eats all memory and aborts with
    "memory allocation of … bytes failed".

What changed

  • crates/svg/src/import.rs: a tspan less than half a line from the current baseline adds no break. The gap is
    clamped to 10 000 line breaks, and a non-finite gap counts as one. Other gaps are imported exactly as before.

How it was found and tested

  • Both were found with synthetic SVG edge cases through vectorcraft-cli convert. The y="1e15" file aborted after 2 s under a
    4 GB ulimit -v; without a limit it keeps allocating.
  • New test import_tspan_far_below_is_bounded (crates/svg/tests/roundtrip.rs) uses a tspan 2 000 000 units down. That is
    harmless on main (138 888 line breaks) but shows the missing bound. It also checks that an ordinary
    two-line gap still gives one blank line. On current main without the change it fails with "138888 line
    breaks". It passes with the change.
  • New test import_tspan_on_the_same_baseline_stays_on_the_line covers runs on one baseline plus a following
    line. On main it fails ("Hello \nworld\nnext") and passes with the change.
  • Full gates: cargo fmt --all -- --check (no diff in touched files; current main already has one in xtask/src/version.rs), cargo clippy --workspace --all-targets -- -D warnings (Rust 1.99, with
    chunks_exact_to_as_chunks/question_mark allowed because they already fire on main), cargo test --workspace,
    cargo xtask layers, cargo xtask assets, cargo xtask wasm.
  • Files: crates/svg/src/import.rs, crates/svg/tests/roundtrip.rs
  • Clean-room: no third-party source consulted.

Found and fixed with Claude Code (AI-assisted).

🤖 Generated with Claude Code

…me baseline, bounded far gaps)

Text import turns the gap between an absolutely positioned <tspan y> and the previous line into
that many line breaks, with two problems:

- At least one break was added even when the tspan sits on the current baseline. Styled or
  kerned runs written with x and y on every tspan (as Illustrator writes them) were split into one
  line per run: "Hello <tspan y=same>world</tspan>" came back as "Hello \nworld".
- The count had no upper bound. A tspan with y="1e15" (or any y far below its text) asked for
  ~10^14 '\n' characters, and opening the file aborted the app, the MCP server or
  `vectorcraft-cli` with "memory allocation failed" after eating all memory.

A tspan less than half a line from the current baseline now adds no break, the gap is capped at
10 000 line breaks, and a non-finite gap counts as one; other gaps are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@echelon
echelon merged commit 4be1e00 into storytold:main Oct 4, 2026
@echelon

echelon commented Oct 4, 2026

Copy link
Copy Markdown
Member

Thanks @storybored379, merged! Fixing both the same-baseline runs (common in real-world SVGs) and the unbounded allocation from a huge y in one small change is excellent. I confirmed both new tests fail on the old code.

If you're not already there, come say hi in our Discord, we'd love to have you: https://discord.gg/artcraft

apexmiguel9-hub pushed a commit to apexmiguel9-hub/vectorcraft-android that referenced this pull request Oct 6, 2026
…lar shaders

MEDIDO, con la cadena de 35 frames del tombstone. Se lee de abajo arriba:

    storytold#15 egui_wgpu::Painter::set_window
    storytold#14 egui_wgpu::renderer::Renderer::new
    storytold#13 wgpu_core::...::CoreDevice::<dispatch>::DeviceInterface::create_render_pipeline
    storytold#12 wgpu_core::device::global::Global::device_create_render_pipeline
    storytold#11 wgpu_core::device::global::Global::device_create_general_render_pipeline
    storytold#10 wgpu_core::device::resource::Device::create_render_pipeline
    storytold#9 wgpu_core::device::resource::Device::create_render_pipeline_inner
    storytold#8 wgpu_hal::dynamic::device::vulkan::DynDevice::create_render_pipeline
    storytold#7 wgpu_hal::vulkan::device::Device::create_render_pipeline
    storytold#6 vulkan.mtk.so    (FragmentShaderCompileState::CompileUF()+1272)
    storytold#4 vulkan.mtk.so
    storytold#3 libufwriter.so   (BILParseStream+164)
    #00 libc.so          (abort+156)

**Los seis frames interiores no son de Rust, son del driver.** `egui_wgpu` pide su
primer pipeline, `wgpu` lo pasa a Vulkan, y `vulkan.mtk.so` aborta mientras compila
el fragment shader, dentro de `libufwriter.so` — el escritor de binarios del
compilador propietario, porque `BIL` es su IR.

## Y no es que a Vulkan le falte nada

Es lo que mas confunde, y conviene dejarlo escrito con numeros. Del perfil
`vp_gpuinfo` del Moto G56 5G (Android 16) y del informe de la base de datos de
Sascha Willems:

| | |
|---|---|
| `apiVersion` | **1.3.1023** (la base de datos lo da como 1.3.303) |
| extensiones | **129**, 61 de ellas `VK_KHR` |
| `VK_ANDROID_external_memory_android_hardware_buffer` | si |
| `driverID` | `VK_DRIVER_ID_IMAGINATION_PROPRIETARY`, PowerVR B-Series 25.1 |
| subgroup | 128, todas las operaciones |
| todas las versiones core 1.0-1.3 | soportadas |

O sea: **no falta ninguna feature, es un driver que conforms pero aborta.** Ni
siquiera hay mensaje: `PVR_ASSERT` viene compilado en release y es un `abort()` a
peloso, asi que el logcat no dice que construccion del SPIR-V le molesta. No hay
forma de averiguarlo desde aqui.

## Por que OpenGL no cuesta nada

Porque VectorCraft **rasteriza en CPU** con `vello_cpu`. La GPU solo presenta
pixeles que ya estan pintados. Toda la parte de Vulkan que de verdad importa —
compute shaders, para rasterizar en GPU— es justo la que este proyecto no usa, y lo
hizo a proposito para no depender del driver. Aqui el coste de GL es cero.

## Como queda

Una funcion, `wgpu_solo_opengl()`, que parte de
`WgpuSetupCreateNew::without_display_handle()` y cambia `instance_descriptor.backends`
a `Backends::GL`. Se escribe asi, modificando el campo, y no con sintaxis de
actualizacion de structs, porque no depende de que campos existan los que no se
nombran.

`eframe` reexporta `egui_wgpu` y `egui_wgpu::wgpu` (su `lib.rs:162`), asi que no
hacen falta dependencias nuevas.

**Queda anotado como una linea si el driver deja de abortar**, y el propio egui-wgpu
tiene ademas el interruptor `WGPU_BACKEND=opengl` para probarlo sin recompilar.
apexmiguel9-hub pushed a commit to apexmiguel9-hub/vectorcraft-android that referenced this pull request Oct 6, 2026
MEDIDO, y el motivo de fondo es que **abrir una segunda Activity da ANR en este port**.
No es SAF: es de abrir cualquier Activity. Traza del propio movil, textual:

    "main" prio=5 tid=1 Native
      native: storytold#1 <std::sys::sync::condvar::futex::Condvar>::wait+112
      native: storytold#2 <WaitableNativeActivityState>::set_activity_state+160
      native: storytold#3 try_with_waitable_activity_ref::<glue::on_pause::{closure#0}…>+240
      native: storytold#5 android_activity::activity_impl::glue::on_pause+20
      native: storytold#8 android.app.NativeActivity.onPause+16

    Reason: Input dispatching timed out (…MainActivity is not responding.
      Waited 10001ms for FocusEvent(hasFocus=false))
    mLastPausedActivity: com.android.documentsui/…picker.PickActivity

`set_activity_state` (`android-activity-0.6.1/src/native_activity/glue.rs:511`) escribe
el comando en un pipe y **espera en un condvar** a que `activity_state` cambie, y solo el
hilo del bucle puede cambiarlo. Ese hilo es el de `android_main`, que es donde corre el
update de egui.

O sea, el ciclo cerrado: el update espera al selector -> el selector pausa la Activity ->
la pausa espera al update -> el selector nunca devuelve. **Mi suposicion anterior de que
"el despertar viene de `onActivityResult`, en otro hilo, y no hay bloqueo" era
incorrecta**: ignoraba ese handshake.

Un browser dibujado en egui no abre ninguna Activity, asi que el ANR desaparece de raiz.

## El navegador: `egui_file`, y no hay que escribirlo

MEDIDO, leyendo el crate:

* `grep -rniE 'android|jni|SAF' src/` **no devuelve ni una linea**. Es egui puro.
* `src/lib.rs:231` — `fs: Box::new(Fs {})`, y `src/fs.rs` es `std::fs` a pelo
  (`std::fs::create_dir`, `read_dir`, `rename`). **No hay que escribir ningun `Vfs`.**
* Pide `egui ^0.36`, que es exactamente la del proyecto. Una dependencia (`dyn-clone`),
  MIT, 4,1 M de descargas, actualizado 2026-08-17.
* Inmediate mode: `show(ctx)` cada frame y `state()` / `path()` / `selection()` para el
  resultado. **Ninguna espera.** Ademas trae `multi_select`, asi que Place tambien sale.

## El permiso

`MANAGE_EXTERNAL_STORAGE` si hace falta, y se declara. MEDIDO, y no es un detalle de la
web: sin el, el navegador se queda en lo nuestro y no ve el telefono.

    $ adb shell run-as ai.storyteller.vectorcraft ls /sdcard
    ls: /sdcard: Permission denied

Y el otro lado, con un build de test de **ArmorPaint sin ningun** permiso de
almacenamiento: su browser muestra la **rejilla vacia** en `/Download`. Los nombres de las
carpetas de primer nivel si se ven; su contenido, no. Que es justo la linea que separa
las dos apps.

Decidido lo que **no** hace falta: bajar el `minSdk`. Lo que decide es el `targetSdk`, no
la version del telefono, asi que el permiso hace falta igual en Android 11 que en 16. En
los Android 7 a 10 que cubre `minSdk 24` no se pide y no hace falta.

Es un aviso con dos botones, no un dialogo modal: se puede saltar y la app sigue
sirviendo para lo que es suyo.

## Lo que gana el motor sin tocarlo

Con rutas de verdad en vez de `content://`:

* **Los ficheros linked vuelven a funcionar.** El motor usa `std::fs` para ellos
  (`cmd/links.rs:627`, `cmd/swatchlib.rs:136,190`, `cmd/flatten.rs:417`,
  `cmd/pdfcmds.rs:176`, `cmd/printpresets.rs:211`), asi que con SAF un `content://` les
  hacia `stat` a una cadena rara.
* **`note_recent` guarda rutas de verdad**, asi que los recientes se abren.

## Lo que queda fuera, y por que

**Guardar y exportar no funcionan todavia.** No es un descuido: `pick_save`, `pick_open`,
`pick_folder` y `pick_open_multi` son sincronos (`FnMut(…) -> Option<String>`), y un
browser en immediate mode no puede devolver una ruta en la misma llamada en la que el
usuario la elige. Ademas `save` escribe en esa misma llamada (`io.rs:251-263`).

Open y Place si, con `open_async`+`inbox` y `place_async`+`place_inbox`, que **ya existen
upstream** (`lib.rs:159,168`) y se drenan cada frame (`lib.rs:676`). Cero cambios en
`crates/**`.

Para guardar hace falta una costura aditiva, `save_async`, con el mismo patron que
`open_async`. Es el paso siguiente, y medido antes de escribirlo.

## Estructura

* `browser.rs` — el navegador, los `Services` y el frame del browser.
* `permiso.rs` — lo unico que queda de JNI: pedir y leer el permiso.
* `saf.rs` — **borrado**. Con ella caen el base64, `onActivityResult` y el `Condvar`.
* `MainActivity.java` — de 552 a 118 lineas. Solo el permiso y `nativeListo`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants