#3349

Icon: Add independently referenced SVG constants alongside asset sources

Opensuxiaoshaoopened 28d ago

Motivation

GPUI Kit currently supports loading the full icon catalog through AllAssets, or selecting extra icons explicitly with icon_assets! and composing that source with the default Assets.

For icons chosen directly in Rust code, the latter requires maintaining two places: the use site and the asset macro's selection list. Removing the last UI use does not remove its entry from the asset source. The generated runtime load(path) still references those bytes, so an obsolete selection can remain in the final binary.

Could we add a third, additive option: independently referenced SVG constants that convert directly into Icon, with resource inclusion following reachable Rust references?

Proposed usage

API sketch, subject to maintainer feedback:

use gpui_kit::component::{Icon, SvgIcon, button::Button};

let icon = Icon::new(SvgIcon::SEARCH);
let button = Button::new("search").icon(SvgIcon::SEARCH);

let disclosure = if expanded {
    SvgIcon::CHEVRON_DOWN
} else {
    SvgIcon::CHEVRON_RIGHT
};
let icon = Icon::new(disclosure);

SvgIcon would be a small Clone + Copy value with private &'static [u8] storage, a const fn new, a byte accessor, and generated associated constants. Each constant references only its own SVG. From<SvgIcon> for Icon would initially delegate to the existing Icon::data; direct element use and entity construction can follow the existing icon conventions.

Applications would not need another dependency, a selection macro, or an asset-source registration for these byte-backed icons. The normal default Assets registration remains necessary for components that use their built-in path-based icons.

Coexistence and implementation scope

OptionSelection/loading model
IconName with AllAssetsComplete catalog through runtime asset paths
IconName with icon_assets!Explicit asset selection through runtime paths
Proposed SvgIcon constantsEmbedded bytes selected by reachable Rust references

Both existing IconName types, default Assets, AllAssets, icon_assets!, custom IconNamed implementations, and path/data APIs would remain available with their existing behavior. The new type would not replace either enum or implement the path-based IconNamed trait.

The SVG files and catalog generation would stay in gpui-kit-assets; SvgIcon and its conversion into the styled Icon would live in gpui-component. Internal generated declarations can connect the two without making Assets depend on Component, duplicating SVG files, or adding a crate. Keeping the new type local to Component also avoids an upstream-trait overlap with the existing blanket From<T: IconNamed> for Icon implementation.

The new entry point should not introduce an ALL array or runtime name-to-bytes table that references the entire catalog. With normal optimized compilation/linking, unreferenced constants need not retain their SVG bytes in the executable. This concerns the final binary, not the downloaded crate or intermediate build artifacts. A caller that explicitly retains a collection of icons will naturally retain those resources.

Current cost: two copies of the SVG bytes

This can work with the current APIs, but it is not a zero-copy implementation:

  1. SvgIcon converts into Icon through Icon::data(&[u8]), which allocates an Arc<[u8]> and copies the bytes.
  2. When Icon builds its GPUI Svg, it passes the bytes to Svg::data(&[u8]), which allocates another Arc<[u8]> and copies them again. That method also hashes the contents and formats a synthetic cache-path string.

These operations happen when constructing the respective values; they do not imply that SVG parsing and rasterization repeat on every paint. GPUI's sprite atlas caches rendered results. Cloning an existing Icon shares its current Arc, although subsequently building a Svg still goes through the second copying API.

The path-based asset flow does not pass through these two data setters, so these copies are a real distinction. No application-level slowdown or size improvement is claimed without measurement. The initial benefit is simpler usage and eliminating a separately maintained selection list.

Current Component source: Icon::data and the forwarding to Svg::data.

Follow-up when GPUI supports reusable SVG data

Related upstream work, currently open:

The PR proposes SvgData::from_static, from_shared, and copy_from_slice, together with Svg::data(impl Into<SvgData>). Prepared data carries reusable cache identity, and static input borrows its original bytes.

If that API is merged and becomes available in GPUI Kit's GPUI dependency, adopting it here should be a small, localized follow-up:

  • Store SvgData in the private IconSource::Data variant.
  • Convert SvgIcon through SvgData::from_static instead of copying its bytes.
  • Forward the stored SvgData directly into Svg::data, cloning the handle only where necessary, rather than converting it back to a slice.
  • Use as_bytes() for native-menu consumers that need borrowed SVG bytes.

The proposed public calls, such as Icon::new(SvgIcon::SEARCH) and Button::icon(SvgIcon::SEARCH), would remain unchanged. Existing Icon::data(&[u8]) can keep its exact signature and copying ownership semantics by constructing SvgData::copy_from_slice internally; broadening that public signature is not required for this optimization. Thus the later migration can remove both raw-byte copies for the new static constants without a breaking public API change or another application migration.

Parsing, rasterization, and texture storage still have their normal costs; this only removes redundant byte ownership and cache-key preparation work.

Suggested validation

  • Compile examples using existing enums/custom IconNamed implementations alongside the new constants and component icon slots.
  • Check byte-backed rendering and native-menu integration.
  • Compare optimized binaries using no extra icon, one icon, and two icons, then remove a reference and verify the unreferenced SVG is absent. Avoid relying only on total binary size.
  • Keep native and WASM behavior explicit: this new option embeds selected bytes on both; existing CDN/path loading remains available.

Related existing byte-input API: #2961.

Activity

1
suxiaoshao28d ago

Would this additive SvgIcon approach fit GPUI Kit's intended API and crate boundaries? In particular, would you be comfortable offering it alongside the existing full-catalog and macro-selected asset sources?

If the direction looks good, may I start an implementation PR using the current Icon::data API, accepting the two copies described above for now? The proposed GPUI SvgData work can then be adopted in a small follow-up without changing the new public icon calls or breaking existing inputs. I would appreciate your feedback on the type name and placement before starting.