Rewrite desktop on maplibre-native-ffi - #834
Merged
Merged
Conversation
Step 1 of DESKTOP_FFI_REWRITE.md. Removes the desktop MapLibre Native stack wholesale so the FFI-backed implementation is designed against a clean slate rather than inheriting JNI assumptions. Deletes both bindings modules, the maplibre-native and SimpleJNI submodules, the C++/CMake build, SimpleJNI's KSP wiring, the desktopRenderer/DesktopVariant native-variant build logic, and the JNI build jobs in CI and release automation. Desktop keeps only host-independent code; GestureOptions and RenderOptions lose their MapControls/MapDebugOptions coupling. Desktop does not compile after this commit, which the plan explicitly allows: the only error is the missing ComposableMapView actual. Because AGP's KMP lint and Dokka analyze every target, lint-android and build-docs transitively compile desktop too, so they join test-desktop and build-desktop-app in dropping out of all-good until the new implementation lands. The jobs still run so the branch shows when desktop returns. In flake.nix, vulkan-loader moves from packages into runtimeLibraries: the Linux FFI runtime dlopens libvulkan.so.1, which needs LD_LIBRARY_PATH rather than just build inputs.
The Nix flake existed only to provide a C++ toolchain for building MapLibre Native on NixOS. Consuming the published FFI removes that need, and the graphics libraries the runtime loads come from the host system, so delete the flake rather than trimming it down. Pin shellcheck in mise so actionlint can check `run:` blocks; it was resolving to an unversioned shim and failing the pre-commit hook. Also restore the full all-good gate list. CI does not need to be green while this branch is in progress, so there is no reason to carry a temporary exclusion list and remember to undo it.
Step 2 of DESKTOP_FFI_REWRITE.md. Splits the single jvmTarget property into androidJvmTarget (still 11) and desktopJvmTarget (25), and raises the shared toolchain to 25. The FFI binding ships Java 24 bytecode and uses FFM, so desktop cannot go lower; Android bytecode is unchanged, verified as class file major 55. buildSrc pins 25 separately because it cannot read the root gradle.properties. Adds the Central Portal snapshot repository scoped to org.maplibre.nativeffi, catalog entries for the binding and both runtime backends, and the FFI binding as a desktopMain dependency of the library. The application, not the library, selects the native runtime. DesktopHostPlatform replaces the deleted Configuration/DesktopVariant logic as the single host detector: it maps the host to a natives classifier and the Metal-on-macOS / Vulkan-elsewhere backend split, and errors rather than guessing on an unpublished platform. Adds --enable-native-access=ALL-UNNAMED to the demo application (covering both `run` and packaged distributions) and to desktop test tasks, and documents it for consumers running an unpackaged JVM. checkDesktopFfiRuntime asserts the desktop runtime classpath carries exactly one binding and exactly one native runtime matching this host, so a misconfigured runtime fails at build time rather than at map creation.
Match maplibre-native-ffi, which pins zulu-25.34.17.0. Temurin does not publish a Windows arm64 build, which the desktop machine matrix needs.
Step 3 of DESKTOP_FFI_REWRITE.md, adding the public extension point in org.maplibre.compose.desktop that keeps map behavior independent of the Compose host. DesktopMapHostFactory is the replaceable piece, selected through LocalDesktopMapHostFactory. A host owns the GPU objects on both sides of the handoff: it allocates the target MapLibre renders into, synchronizes producer and consumer, and draws the finished target into Compose. DesktopMapRenderer is the other side, implemented by the map session. Render targets are borrowed, backend-neutral handle wrappers rather than generated FFM types, so the SPI does not leak org.maplibre.nativeffi.render. DesktopMapExtent carries logical and physical size derived together, since letting them drift under fractional scaling produces a stretched map. Backend negotiation is a pure function over the FFI runtime's backends and the factory's, deliberately free of FFI and Compose types so it can be tested without a graphics stack. Its diagnostic names the runtime backends, the host backends, the OS and architecture, and the likely missing dependency. The default factory is a placeholder reporting that no host exists yet; the Skiko host arrives with the native bridges. Tests land with the fake host in the next commit, because desktopTest cannot compile until the ComposableMapView actual exists.
Read out of the FFI bindings, the C headers, and the working Compose example before writing DesktopMapSession. Most of this is observable only by reading native source or by debugging a failure, so it belongs in the plan rather than in commit archaeology. The findings that change the design: - Borrowed-texture render sessions cannot be resized at all. A size or scale change means close, replace the texture, and attach a new session. This is what DesktopRenderTarget.generation exists to signal. - renderUpdate() reports "nothing to render" by throwing InvalidStateException, the same type as a detached or closed session, distinguished only by its diagnostic text. Treating it as an error fails every map on its first frame. - RenderTargetExtent is logical while the texture is physical, and nothing validates the relationship; a mismatch renders garbage rather than throwing. - The map's pixelRatio is fixed at creation, so moving a window between displays of different density requires recreating the map, not re-attaching. - Closing a map purges its queued events, so teardown must never await one. - Style load failures arrive only as events, never as exceptions. Also records four confirmed gaps needing local fallbacks: no visible-region API, no meters-per-pixel API, no maximum-FPS control, and no animation-completion signal.
Step 4, reordered ahead of the bridge port: desktopTest cannot compile until desktopMain does, so the fake host and its tests were unreachable while the ComposableMapView actual was missing. Desktop compiles again for the first time since the legacy deletion, and the first desktop tests run. DesktopMapSession implements both MapAdapter and DesktopMapRenderer, owning the runtime, map, and render session on one dedicated thread. Nothing native is created eagerly: the runtime binds to its creating thread, so everything is built lazily inside render(), which the host already calls on the owner thread. NativeOwnerThread is re-entrant because every camera mutator requests a frame while already on the owner thread, which would otherwise deadlock against a single-threaded executor. Behavior that follows directly from the native semantics recorded in the plan: - A target generation or extent change closes the render session and attaches a new one, because borrowed-texture sessions cannot be resized. There is no map.resize to call; attaching sets the map size from the descriptor extent. - renderUpdate's "no map render update is available" is matched by diagnostic and treated as a skipped frame with the pending bit intact, rather than as an application error that would fail every map on its first frame. - A display scale change recreates the map, since pixelRatio is fixed at creation and resizing cannot change it. - Teardown closes render session, then map, then runtime, each in its own finally, since a handle with live children refuses to close and a failed close leaves it live. - Unknown event types are logged rather than failing, because the event type is a value class over Int and an FFI upgrade can introduce new ones. Four gaps get local fallbacks with TODO(maplibre-native-ffi) at the boundary: visible region projects the four viewport corners, meters-per-dp reimplements mbgl's formula, maximum FPS throttles renderUpdate, and camera animation completion uses a generation stamp against superseding transitions. Rendered feature queries and the style object remain unimplemented and are marked for step 6.
Collects the rough edges found while building the desktop integration so they can be fixed upstream rather than left as permanent workarounds here. Each entry records what MapLibre Compose does today, so the workaround can be removed when the fix lands. Entries are marked verified or reported. Verified means confirmed directly against the snapshot — a compiler error, a grep of the native source, or a run. Reported means it came from reading the bindings and headers; a few adjacent claims from that same reading pass turned out to be wrong, so nothing should be filed upstream unchecked. The most consequential entries so far: borrowed-texture sessions cannot be resized and have no re-attach, renderUpdate reports an idle map by throwing, pixelRatio is fixed at map creation, and a failed runtime close leaves its thread permanently unable to host another runtime.
The Skiko host bridges MapLibre's render target into Compose's GPU context, which needs direct Vulkan and OpenGL access. LWJGL provides it, matching the maplibre-native-ffi Compose example this port follows. The BOM is dropped in favour of pinning each module to the catalog version. `platform()` is deprecated for removal inside KMP source-set dependency blocks, as is `variantOf`, so both the BOM and the natives classifiers are spelled out directly. LWJGL names its x64 classifier `natives-linux` where MapLibre Native FFI uses `natives-linux-x64`, so DesktopHostPlatform now derives both rather than letting a caller assume they are interchangeable. checkDesktopFfiRuntime now also asserts the LWJGL natives resolve for this host; a missing classifier jar would otherwise surface as an UnsatisfiedLinkError deep in bridge setup rather than as a dependency problem.
Step 5 for Linux. The demo now renders MapLibre's demotiles style: MapLibre draws into a VkImage whose memory is exported as a file descriptor, Compose imports that descriptor into its GL context as a texture, and Skia composites it. Ported from the maplibre-native-ffi Compose example, which is the reference for this path. Two bugs in the previous commit had to be fixed to get there, both of which produced a blank map with no diagnostic: - DesktopMapSession created its own owner thread, but the host calls render() on the host's renderer thread. MapLibre binds its runtime to the creating thread, so the runtime was bound somewhere the host never called from and every frame failed. The session no longer owns a thread; it dispatches through the host's withRendererAccess, which is what the example does. - DesktopMapSurface set a Failed state without logging it, so the wrong-thread failure was completely silent. Failed and Unavailable states now always reach the log. Also fixed: the surface never closed the renderer, leaking the native chain. The renderer is now closed before the host, since its handles can only be released from the host's still-live renderer thread. Compose 1.10.3 spells the Skia canvas accessor `nativeCanvas`; `skiaCanvas` is the 1.11 name the example uses. All Skiko reflection is confined to SkikoReflection so a Compose upgrade touches one file. Windows and macOS report unsupported for now rather than crashing mid-frame; their bridges are next.
Compose exposes no supported way to reach its graphics context, so the default host reads it reflectively. That makes a Compose upgrade able to break map rendering with no compile error and no test failure — it would first appear as a blank map at runtime, on whichever platform owns the moved member. SkikoReflectionContractTest fails at build time instead. It starts no map, creates no GPU resources, and loads no native library, so it runs headlessly on any platform regardless of which backend that platform uses. It also answers the question left open in step 3: every reflected member exists at Compose 1.10.3 / skiko 0.9.37.4, not just every reflected class.
The desktop style implementation uses MapLibre's generic JSON style API, so every layer, source, filter, and expression crosses this boundary. Centralizing it is what stops layer setters and query code from growing their own subtly different encodings. Two details the tests pin down because getting them wrong is silent: - Integers stay integral. MapLibre distinguishes 5 from 5.0 for some properties, so a zoom stop or index must not widen to a double in transit. - Object key order is preserved. Layer JSON is assembled key by key and MapLibre reads `type` before the properties that depend on it. An unsigned value past Long.MAX_VALUE is reinterpreted rather than read back negative, since the C ABI carries uint64_t in a Long's bit pattern.
Desktop is built on MapLibre's generic JSON style API, so every filter and
styled property is encoded here. Ported from the Android encoder rather than
invented: the encodings belong to the style spec, not to a platform, so a
desktop map given the same expression as an Android map has to render
identically.
One deliberate divergence. Android and iOS wrap a map literal as
`{"literal": {...}}`; desktop emits `["literal", {...}]`. Those platforms hand
the result to their SDK's expression parser, while desktop writes raw style
JSON, where `literal` is an operator and the object form would parse as an
ordinary object with a `literal` key. Marked with a TODO to confirm against a
running map and reconcile if Android and iOS turn out to be wrong rather than
merely different.
The colour test pins that Compose quantizes alpha to 8 bits, so a nominal 0.5
encodes as 0.5019608. The encoder reports what Color actually holds, matching
Android.
Completes the default host's backend matrix: Linux bridges Vulkan to OpenGL, Windows bridges Vulkan to Direct3D 12, and macOS runs Metal on both sides. Only the Linux path has rendered on real hardware. These two are ported from the maplibre-native-ffi Compose example and carry TODO(maplibre-compose) markers at each point the port could not be verified, the sharpest being a hard-coded byte offset into Skiko's private DirectXDevice struct. They are validated on the machine matrix in step 9. The two platforms allocate in opposite directions, which is worth knowing before debugging either. On Linux MapLibre's Vulkan image is exported as a file descriptor that OpenGL imports, and the import consumes the descriptor. On Windows it is Compose's ID3D12Device that allocates, and Vulkan imports the resulting NT handle — which the import duplicates rather than consumes, so our copy is closed on every path. Note on history: MacosMetalHost, MacosObjectiveC, SkikoMetalPresenter, and SkikoDirect3DPresenter were swept into the preceding commit by a `git add -A` while these were being written, so they carry that commit's unrelated message. Left in place rather than rewriting pushed history; this commit is where they are explained.
Step 6's bulk. Desktop styling is built on MapLibre's generic JSON style API: sources and layers are live descriptors that accumulate their definition before being added to a style and write through to MapLibre afterwards, so a layer can be configured at any point in a composition and is never briefly visible in a half-configured state. DesktopStyle binds them to a map and reconstructs base-style sources and layers as UnknownSource/UnknownLayer. Those reconstructions bind without re-adding: they are views onto what MapLibre already owns, and attaching them would duplicate a source or reorder the layer stack. All 19 families are implemented against the style spec, verified against the vendored mbgl converters rather than guessed. Notable calls: raster-dem Custom encoding is emitted as "mapbox" because mbgl rejects anything else and would drop the whole source; ComputedSource has no style-spec representation at all and stands in as an empty GeoJSON source so layers naming it still resolve. Cluster and source-feature queries need RenderSessionHandle, which a source cannot reach, and are marked accordingly. KNOWN REGRESSION, documented in DESKTOP_FFI_REWRITE.md: the desktop map no longer renders. It rendered at 6a5088d. The runtime and map are created with no error logged, but the frame loop stops after the first frame with both the renderer thread and the AWT event thread parked idle — a lost wake-up, not a deadlock or a native failure. Two real bugs found while chasing it are fixed here and were not sufficient alone: the pump now keeps requesting frames until MapLibre reports MAP_IDLE, since MapLibre only advances while pumped and pumping only happens inside a frame; and requestFrame now defers its Compose state write to a later event-loop turn, because a write made from inside the draw pass that called it is discarded. Committed rather than dropped so the investigation starts from a known state. Also adds a first-frame log, since a blank map is the failure mode with the least to go on, and pins the Windows and macOS Skiko members the bridges reflect into after a reviewer found them unasserted.
The desktop map renders at e094615. The failure I recorded there happened only after the machine had slept, and the same commit renders after a fresh boot, so the cause was losing the GPU contexts underneath the Vulkan-to-OpenGL sharing rather than the style work. Recorded as a suspend/resume finding instead, because it is one: the frame loop had nothing to wake it afterwards, which is the first concrete evidence that surface loss needs real handling rather than the current assumption that the host simply reports a new target generation. The two bugs found while chasing it were genuine and stay fixed: the pump now keeps requesting frames until MapLibre reports MAP_IDLE, and requestFrame defers its Compose state write out of the draw pass that called it. Also records the style load failure the demo logs — `http: invalid authority`, the built-in loader refusing a non-HTTP URI — as the first concrete case for the step 7 resource adapter.
MapLibre's network stack rejects anything that is not HTTP with `invalid authority`, and Compose hands out `jar:file:` and `file:` URIs for packaged resources. Any style, sprite, glyph, or tile referenced through Res.getUri therefore failed to load: the demo logged 16 such failures at startup, and zero after this change. The provider intercepts only non-network schemes and passes HTTP and HTTPS through untouched, so those keep MapLibre's own caching, retry, and revalidation rather than being reimplemented here. It is installed during runtime creation, before the map exists, because MapLibre refuses to replace a resource provider once the runtime owns maps and offers no way to clear one. Taking a request means owning the handle, so completion and close happen on whatever worker thread MapLibre called from. Also restores the first-frame log, which had been silently dropped by an earlier edit. That mattered: its absence was twice read as evidence that nothing rendered, when it only ever meant the line was gone. The demo now reports "Rendered the first map frame with VULKAN".
The cache previously landed in the working directory, which meant it was invisible to the user, was not shared between runs launched from different places, and got committed often enough that the repository carried a .gitignore entry for it. That entry is removed here along with the behavior that needed it. It now follows each platform's own convention — XDG on Linux, Library/Caches on macOS, LOCALAPPDATA on Windows — so it lands where a user or an uninstaller would look. DesktopRuntimeOptions makes the path and the ambient size limit configurable through LocalDesktopRuntimeOptions; the options are read when the runtime is created, so changing them recreates the map. The cache directory is created before the runtime, because MapLibre opens the database on creation and fails when the directory is missing, which on a fresh machine it always is. Verified by deleting the cache and running the demo: it renders its first frame with no errors and writes ~/.cache/maplibre-compose/maplibre-cache.db.
This decides the shape of desktop offline support, and the plan called for measuring it rather than assuming. MapLibre binds a runtime to its creating thread and permits one per thread, so a map owns a runtime on its own thread. An offline manager usable without a map needs a second runtime on a second thread, and both would open the same cache file. They can. Two runtimes on two threads opened the same database and both pumped without error, so the offline manager can own a runtime of its own and desktop does not need a process-level runtime service with every map serialized onto one owner thread — which would have been a far larger change. The test stays in the suite so a future FFI snapshot that changes this fails loudly rather than corrupting a user's cache. Desktop tests now pull a native runtime for this host, since anything reaching the FFI needs one exactly as an application does. That also unblocks the FFI integration tests the plan asks for in step 8.
jpackage runs jlink against whichever JDK it is given, so without pinning it the installed application takes whatever JDK Gradle happened to run on. That can be older than the Java 24 the MapLibre Native FFI binding requires — a mismatch that builds cleanly, installs cleanly, and fails when the user opens a map. checkDesktopDistribution asserts what actually ends up in the image: the bundled Java version, an FFI native runtime, natives matching this host, and LWJGL natives. Every one of those failures is silent at build time and fatal at launch, so CI now runs it before packaging rather than trusting an exit code. Verified locally: the distributable bundles Java 25.0.3 alongside the vulkan/natives-linux-x64 runtime and the LWJGL natives. Note that `packageDeb` cannot run on this machine — jpackage needs dpkg-deb and fakeroot, which Fedora does not ship by default — so the installer format itself is only exercised in CI.
Getting started now carries real instructions rather than a placeholder: the library is backend-independent and the application picks a native runtime, the same shape as choosing an Android ABI, plus the snapshot repository, the Java 25 requirement, and the native-access argument. The status table was set to all-unsupported for desktop when the legacy implementation was deleted. It now reflects what works: HTTP and Compose resource URIs, gestures, clicks, camera, projection, visible region, layers, expressions, sources, and images. Feature queries, Material 3 controls, and offline stay unsupported, which is accurate — queries need a RenderSessionHandle a source cannot reach today, and offline is still outstanding. Contributor docs no longer claim desktop does not compile.
The built-in network file source rejects an unrecognized URI with `invalid authority`, naming neither the URI nor the reason. On Compose Desktop that is the common case rather than an exotic one, since packaged resources are `jar:file:` URIs, and the message gives a consumer nothing to act on.
Input was never attached. DesktopMapSession had moveBy, scaleBy, and the rest, but no Compose pointer or key modifier ever called them, so the desktop map could not be panned, zoomed, rotated, or pitched at all. Rotation and pitch now follow the maplibre-native-ffi Compose example: one jumpTo moving bearing and pitch together, at 0.5 degrees per logical pixel, with pitch clamped to 0..60. The first attempt drove the FFI's two-point rotateBy, which derives an angle between two pointer positions for a two-finger gesture — from a mouse drag it rotates about the wrong centre and fights the separate pitch change. Scroll zoom is 2^(-scroll * 0.25) rather than a flat multiplier, since zoom is logarithmic. A new drag cancels any transition still running. Separately, this restores the pump keep-alive that an earlier edit dropped, and it explains the style switcher applying only every other change. MapLibre advances only while pumped, and pumping happens only inside a frame, so a frame that renders nothing ended the loop: a style switch loaded, failed its first renderUpdate because parsing had not finished, and then nothing woke the pump. The next switch's frame was what completed the previous one. The loop now keeps requesting frames until MapLibre reports MAP_IDLE. Desktop also drops its own OrnamentOptions and takes the maplibreNativeMain one, now that desktop depends on that source set. Ornaments are unimplemented on desktop: MapLibre Native's core has none, and the Material 3 controls are the direction for this, so there is nothing to forward.
Offline: DesktopOfflineManager runs its own runtime on its own thread, which SharedCacheDatabaseTest established is safe alongside a map's. Operations are correlated by handle id against OFFLINE_OPERATION_COMPLETED, and every exit path — cancellation, disposal, runtime failure — completes or cancels its continuation rather than awaiting an event that closing the runtime would discard. Four bugs found by review and fixed before landing: - The manager was reference counted and disposed with the composition, so navigating away from an offline screen closed the runtime and silently killed in-flight downloads; mbgl keeps download state in memory only, so the pack came back reporting paused. It is now a process singleton per options value, matching Android and iOS. - The runtime leaked if anything after create threw, because teardown lived in a try the failure path never entered. - setMaximumAmbientCacheSize could never succeed with the default options, since the guard compared against a null limit. It now warns instead of throwing, which is what cross-platform code calling it at startup expects. - A TODO claimed an FFI gap that does not exist: mbgl does deliver a status event on a real pause. The status read-back is kept for the unchanged-state case, with the reason corrected, so this is not filed upstream in error. Desktop teardown: the surface tore the surface down before closing the renderer, which left the renderer without the host session it reaches its owner thread through. Every remaining native call then ran on the disposing thread and was rejected as wrong-thread, leaking the map and runtime. Closing first fixes it; the demo now exits with no errors. A failed layer attach no longer kills the Compose thread, and names the layer and its source. Native reports only "layer source does not exist", which says neither. Demo: desktop now reports every gated feature as supported. LayerStyling works, and InteropBlending falls out of compositing MapLibre's texture into the Compose scene. Adds the render options and offline demos, with a desktop render-options screen exposing MapLibre's debug overlays individually. OfflineManagerDemo moves to a new maplibreNativeShared source set, mirroring the library, and material3's offline controls extend to desktop.
Two separate bugs, both of which broke every demo that adds its own layers. Compose adds a layer to the style before the effect that adds its source runs. The applier inserts nodes and calls onEndChanges, which is where LayerManager reaches MapLibre, and only afterwards dispatches remember-observers, where SourceReferenceEffect lives. MapLibre's mobile SDKs tolerate a layer naming a source that does not exist yet; the C API rejects it outright with "layer source does not exist". A layer now attaches its own source first, and Source.attach is idempotent so the effect's later add is a no-op. The underlying ordering is fragile on every platform and would be better fixed in the shared layer — noted in the plan — but desktop is the only one strict enough to notice. Then symbol layers failed with "layer doesn't support this property". An unset optional property compiles to a null literal, and the layer JSON carried "symbol-sort-key": null and "icon-overlap": null. The style spec has no null: MapLibre rejects the entire layer rather than treating the property as absent. Null-valued properties are now omitted from the layer object, while still being pushed to an already-attached layer, where null is how a property is reset. Found by putting the layer JSON in the failure message, which named the offending properties directly; the native error says only that some property is unsupported. The demo now renders its layer-based demos with no errors.
The map handle is created lazily on the first frame, but MaplibreMap applies the initial camera position, zoom range, pitch range, and bounding box as soon as the adapter is handed to it — which happens earlier. Those calls reached a null map and were dropped, so every map opened at MapLibre's default position instead of the one the caller asked for. Setup calls made before the map exists are now recorded and replayed in order as soon as it is created. Also turns two silent no-ops into diagnostics. A layer added while its style is unloaded was dropped without a word, which is indistinguishable from a layer that was added and did not draw; it now says so. Adds a headless test for the offline manager, since it owns a runtime, a thread, and a database and none of that needs a window — a crash on opening the offline screen should be reproducible without clicking. It passes, so the reported crash is in the UI layer above it.
Implements queryRenderedFeatures for both the point and box overloads against the render session, converting MapLibre's queried features to the GeoJSON ones the common API returns. Source id, source layer id, and feature state ride along as properties, because a common Feature has nowhere else to carry them. Rendered feature state belongs to the render session, so none of this can be tested without a real GPU. FakeDesktopMapHost stops at the graphics boundary — it hands out invented handles, so MapLibre never attaches a session and nothing below render() runs. HeadlessVulkanMapHost closes the gap: a genuine Vulkan device and VkImage with no window, and no external memory extensions, so it also works on a software implementation. Two vehicles use it. HeadlessMapFixture drives a real session frame by frame with no Compose at all; HeadlessVulkanMapHostFactory provided through LocalDesktopMapHostFactory runs MaplibreMap itself under runComposeUiTest, so the surface, session, sources, and layers all take their real paths. That found the offline demo crash on its first run. An unset filter compiles to a null literal, which mbgl reads as "match everything", but the null was being substituted with a scalar true — which mbgl rejects outright as "filter value must be a non empty array", taking the whole layer with it. Every layer with a default filter was affected; it only happened to show up on the offline screen first.
Both were reported as broken in the demo app. Neither reproduces here: a layer that leaves and re-enters the composition comes back, and the camera a map is composed with reaches the map even though it is applied before the map exists. The layer case shared a cause with the filter bug fixed in the previous commit, which took the whole layer with it on the first add.
Two unrelated bugs, both reported from the demo app. A style mutation never asked for a frame. MapLibre only advances while its runtime is pumped, pumping only happens inside a frame, and once the map goes idle nothing asks for another — so a layer added after that point was genuinely in the style and simply never drawn, until an unrelated pan or resize woke the loop. Every source, layer, and image write crosses SessionStyleBinding.withMap, so the wake goes there rather than at the dozen call sites where the next one added would forget it. Most mutations make MapLibre notify us on its own; addSource, removeSource, and removeImage notify nothing, which is what requestRepaint covers. cancelTransitions and setRenderSettings had the same gap. Clicking a cluster zoomed to the whole world because getClusterExpansionZoom was a stub returning 0.0 and the demo animated to it. The stub's TODO was wrong about the blocker: queryFeatureExtension exists, just on the render session rather than the map, because it answers from what a render pass built. StyleBinding grows a withRenderSession accessor and the three cluster methods are implemented over it. The subtle part is that cluster_id has to be re-typed unsigned. MapLibre looks it up with an exact variant check, so an id encoded as a signed integer does not fail — the lookup misses and the query returns an empty result with an OK status. Same for the leaves limit, where a wrongly typed value is ignored and MapLibre substitutes its own default of ten, which still returns features and still looks correct. The test asserts limit = 2 returns exactly two leaves, which is the only thing that tells those apart.
The exact-type check is mbgl's contract, and every platform binding adapts to it: Android casts cluster_id back by hand in two places, and iOS avoids the id half only by never taking the feature out of C++, while still casting limit and offset itself. maplibre-native-ffi is the analogous layer, so that is where this belongs rather than upstream. Recommends taking a cluster id instead of a whole feature, since mbgl discards everything else in it and an unsigned parameter makes the mistake unrepresentable. Our coercion is now marked for removal; only snapshots of the FFI exist, so there is no released version to keep working against.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4da89c3d17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 667736a839
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c244778eb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A task that only writes owner-thread state queued nothing for native to wake the loop's park with, so a gesture end posted to a map at rest never reached onEventsDrained and the camera move never ended.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8214299482
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 700cb4439e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
@greptile-apps review |
This was referenced Aug 8, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces the desktop JNI integration with the
org.maplibre.nativeffiKotlin Multiplatform bindings, bringing desktop to near-parity with mobile and laying the foundation for Android and iOS to share the same maplibre-native integration layer in the future.The line count may seem unexpectedly positive, since we're getting rid of our C++ integration. But of the +11k, about 8k is new test coverage we never had on the old stack, and the code we're getting rid of was missing a lot of desktop functionality that's now implemented.
Bigger reductions will happen when the android and ios (and web?) sides drop their bespoke implementations and build on the same
mlnFfiSharedsource set, and then when the owned facade types are replaced with exposing mln-ffi types directly.Test plan
200 new desktop tests, including headless tests with a fake map host and real rendering tests.
Also a new demo app figure that boots the desktop app under compose-glfw for better native fidelity, and proves the custom host (no reflection) path
How it works
MapLibre renders into a texture that Skia draws as part of the Compose scene, so the map composites like any other composable—overlays, alpha, and transforms apply to it. This requires a bridge between the two graphics APIs (MapLibre supports OpenGL, Vulkan, Metal, while Compose uses OpenGL, D3D, and Metal) and reflection into Compose internals to grab the gpu context we need. An alternative
glfw-fixture/is a second demo-app on a non-AWT Compose host, which shows how this can be done without reflection as long as a custom compose host exposes its graphics context.Caveats
--enable-native-access=ALL-UNNAMED. The binding is Java 24 bytecode and uses FFM.DesktopOrientationProviderremains a stub.Relevant issues
Partially addresses:
Not yet addressed, but a prerequisite for:
Checklist
To your knowledge, are you making any breaking changes?
Yes. The
org.maplibre.compose:maplibre-native-bindings-jniartifact and its OS/architecture/renderer capabilities are no longer published, the desktop target requires Java 25, and consumers must pass--enable-native-access=ALL-UNNAMED.AI assistance