Repository navigation
Conversation
| }; | ||
| window.next_frame.scene.insert_primitive(PolychromeSprite { | ||
| order: 0, | ||
| pad: PAINT_EPOCH.load(Ordering::Relaxed).max(1), |
There was a problem hiding this comment.
[P2] Include mutable texture versions in retained tile invalidation
The epoch stored in pad is omitted by fast/layers/tiles.rs's polychrome-sprite hash, so updating and repainting the same texture at unchanged bounds leaves cached scroll tiles showing the old frame. Include a changing texture version in tile invalidation (or exclude mutable sprites from those tiles), and cover update → repaint → composite with a regression.
Reviewed by ChatGPT on behalf of @huacnlee.
There was a problem hiding this comment.
Addressed in d08573c. The tile hash now covers PolychromeSprite::pad, and pad carries a per-texture content version (bumped on every update_dynamic_texture) instead of a global epoch — so only the tiles showing the refreshed texture are re-rendered, and an unrelated texture's update leaves every other tile cached. A regression test (repainting_an_updated_texture_dirties_its_tiles) covers update → repaint → tile-hash change, and asserts a never-updated texture still keeps its tiles.
| "dynamic texture update is outside the texture" | ||
| ); | ||
|
|
||
| texture.replace_region( |
There was a problem hiding this comment.
[P1] Synchronize uploads with in-flight GPU readers
replace_region overwrites the same texture that a previously committed frame may still be sampling; the map mutex does not synchronize GPU work, and Metal's CPU replacement API provides no such synchronization. Use uploads ordered on the rendering queue or recycle textures only after their GPU readers complete, otherwise streaming updates can produce torn or mixed frames.
Reviewed by ChatGPT on behalf of @huacnlee.
There was a problem hiding this comment.
Addressed in d08573c. update_texture no longer touches the texture on the CPU: it stages the packed rows in a shared (or managed, on discrete GPUs, with did_modify_range) buffer, and each frame's command buffer copies the staged updates into their textures with a blit encoder before it encodes any pass. Encoders run in order on a command buffer, and command buffers run in commit order on the queue, so no frame samples a region an in-flight predecessor is still overwriting. The flush hook runs on all four command-buffer creation points (the main frame, the fast paths frame, rounded-clip groups, and scroll-layer tile rasterization), since each samples textures. The initial create still uses replace_region, which is safe because a texture's id is never reused and no committed frame can reference it yet. The texture usage now also includes ShaderWrite for the blit destination.
… them for tiles Addresses review comments on longbridge#86: - The Metal backend no longer overwrites a texture with replace_region while a committed frame may still be sampling it. update_texture stages the rows in a shared or managed buffer, and each frame's command buffer copies the staged updates into their textures with a blit before it encodes any pass: encoders run in order on the buffer, and command buffers run in order on the queue, so no frame samples a region an in-flight predecessor overwrites. The initial create still uses replace_region, which is safe because a texture's id is never reused and no frame can reference it yet. - The scroll-layer tile hash now covers PolychromeSprite::pad. Atlas sprites keep the padding at zero; a dynamic texture's sprite carries a per-texture content version (bumped on every update), so tiles showing refreshed contents are re-rendered while an unrelated texture's update leaves other tiles cached. A regression test covers update -> repaint -> tile hash.
Windows that embed content that changes every frame — a remote desktop surface, a video stream, a live chart — have to round-trip each frame through RenderImage and the sprite atlas: every upload reallocates or disturbs atlas tiles, the image cache churns, and glyphs sharing the atlas pay for it. This adds dynamic textures: BGRA8 textures the caller owns and paints directly, outside the atlas allocator. - Window::create_dynamic_texture allocates one; the payload may be strided, and is validated and repacked in fast::dynamic_textures before the backend sees it. - Window::update_dynamic_texture uploads a sub-region in place and bumps a paint epoch; the sprite's pad carries it, so a retained frame holding the texture is told apart from the refreshed one without touching the scene types. - Window::paint_dynamic_texture inserts the sprite; its atlas texture id comes from a reserved high range, below the layer tile range, so the id spaces stay disjoint without extending upstream's enums. - Window::remove_dynamic_texture releases it. The GPU side is a small platform contract, gpui::DynamicTexturePlatform, installed by the renderer. Metal implements it with standalone MTLTextures and replaceRegion uploads, and resolves a dynamic texture id in draw_polychrome_sprites and draw_monochrome_sprites before falling back to the atlas. Platforms without a backend report an error from create_dynamic_texture instead of failing later; a Windows (D3D11) backend can follow the same shape. Upstream files only hook: one init line in the renderer constructor, one lookup-or-fall-through in each sprite draw path, a pub(crate) bump on Window::snap_bounds, and the public names exported from gpui.rs (the gpui.rs allowlist hunk grows by four names for it). Tests cover the validation and dispatch against a mock platform — payload and stride mismatch, out-of-bounds and negative regions, unknown and removed ids, a failing backend leaving no stale state, and the atlas id mapping — and the module's own state is checked to stay out of the atlas's id space. cargo check, clippy and script/check-upstream are clean; the new unit tests pass.
… them for tiles Addresses review comments on longbridge#86: - The Metal backend no longer overwrites a texture with replace_region while a committed frame may still be sampling it. update_texture stages the rows in a shared or managed buffer, and each frame's command buffer copies the staged updates into their textures with a blit before it encodes any pass: encoders run in order on the buffer, and command buffers run in order on the queue, so no frame samples a region an in-flight predecessor overwrites. The initial create still uses replace_region, which is safe because a texture's id is never reused and no frame can reference it yet. - The scroll-layer tile hash now covers PolychromeSprite::pad. Atlas sprites keep the padding at zero; a dynamic texture's sprite carries a per-texture content version (bumped on every update), so tiles showing refreshed contents are re-rendered while an unrelated texture's update leaves other tiles cached. A regression test covers update -> repaint -> tile hash.
d08573c to
657d1d5
Compare
Motivation
Content that changes every frame — video playback, remote desktop streams, live previews — cannot go through the sprite atlas today. Rebuilding an
Imageper frame reallocates atlas tiles, which invalidates the glyphs and icons sharing those textures and shows up as texture churn and frame spikes. The scene'sPolychromeSpritealready has apadfield, but there is no public path to allocate a mutable GPU texture and paint it.This adds a
fast::dynamic_texturesmodule: standalone BGRA8 textures that live outside the sprite atlas, updated in place through the platform backend, so refreshing a frame never disturbs cached atlas content.Implementation
Following the repo's layering rules, all logic lives in
fast/modules:crates/gpui/src/fast/dynamic_textures.rs— the core API:DynamicTexturePlatform(create/update/remove a texture by id), free functions, and fourimpl Windowmethods (create_dynamic_texture,update_dynamic_texture,paint_dynamic_texture,remove_dynamic_texture), following the existing upstream precedent ofimpl Windowblocks in fast modules. The core validates every region against the created size (in i64 arithmetic, since caller bounds are untrusted), repacksbytes_per_row-strided input into tight rows, and tracks live textures so stale ids are refused. Texture ids map to reserved atlas indices (DYNAMIC_INDEX_BASE = 0xE000_0000, belowLAYER_TILE_TEXTURE_BASE), keeping the three id spaces disjoint without touchingAtlasTextureKind. Each texture carries a per-texture content version, bumped on every update; painted sprites carry it inPolychromeSprite::pad, which the scroll-layer tile hash covers, so tiles showing refreshed contents re-render while unrelated textures' updates leave other tiles cached (a regression test covers update → repaint → tile-hash change).crates/gpui_apple/src/fast/dynamic_textures.rs— the Metal backend: each texture is its ownMTLTexture(shared storage on unified memory, managed otherwise). Updates never overwrite the texture on the CPU:update_texturestages the rows in a buffer, and each frame's command buffer copies the staged updates into their textures with a blit encoder before it encodes any pass. Encoders run in order on the command buffer and command buffers run in commit order on the queue, so no frame samples a region an in-flight predecessor is still overwriting. The initial create still usesreplace_region, which is safe because a texture's id is never reused and no committed frame can reference it yet.snap_boundsbecomespub(crate); four export names join the alphabetized fast:: group ingpui.rs(allowlist hunk 12 → 16, commented).create_dynamic_textureinstead of failing later.Verification
script/check-upstream: 0 violations (540 files compared against import commit 1089f40)cargo check -p gpui -p gpui_apple -p gpui_macos: clean (the twounused variablewarnings infast/layers/reuse.rspredate this change)cargo test -p gpui fast::: 347 passed (the 10fast::dynamic_texturestests cover payload/stride validation, region bounds, row repacking, id round-trip, failing-backend error propagation, and the tile-invalidation regression)cargo clippy -p gpui -p gpui_apple -p gpui_macos: no new warningsrustfmt --edition 2024 --checkon the changed files: clean