Skip to content

vello_cpu: Cache the resolved image in Fine - #5

Open
nicoburns wants to merge 1 commit into
mainfrom
devin/1790445884-fine-image-resolve-cache
Open

nicoburns wants to merge 1 commit into
mainfrom
devin/1790445884-fine-image-resolve-cache

Conversation

@nicoburns

Copy link
Copy Markdown
Member

Summary

Fixes the negative multithreaded scaling of cached glyph rendering in vello_cpu.

Before this change, Fine did the following for every EncodedPaint::Image command:

  1. called resources.image_resolver.resolve(id)
  2. cloned the Arc<Pixmap>
  3. dropped the clone at the end of the command

All cached glyphs sample the same glyph-atlas page, so every worker thread hammered one shared refcount. In a perf profile at 4 threads:

  • ImageRegistry::resolve was 16.6% of samples, 98% of it on the atomic increment.
  • paint_fill was 24%, 90% of it just after the atomic decrement.
  • The painters also suffered, because the pixmap's width, height and data pointer sit on the refcount's cache line.

Now Fine keeps the most recently resolved image:

resolved_image: Option<(ImageId, Arc<Pixmap>)>,
// per command:
if !matches!(&self.resolved_image, Some((cached, _)) if cached == id) {
    self.resolved_image = Some((*id, resolver.resolve(*id)?));
}
let pixmap: &Pixmap = &self.resolved_image.as_ref().unwrap().1;

This means one resolve per thread per image rather than one per command. ImageSource::Pixmap is borrowed instead of cloned.

Part 2 of 3 of the CPU glyph-cache speedups. It textually conflicts with the alpha-mask fast-path PR in fine/mod.rs; whichever merges second needs a trivial rebase.

Measurements

Throwaway probe: 1600×1200 target, ~9.3k glyphs, 14px, atlas_cache(true), mean of 60 warm frames. Render time, main → this branch:

threads main this branch
1 4.50 ms 3.27 ms
4 6.32 ms 1.89 ms
8 6.60 ms 1.22 ms

With the cache on, total frame time at 8 threads goes from 11.2 ms to 4.1 ms. Uncached rendering is unaffected.

cargo test -p vello_cpu -p vello_common passes.

Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/55344a6e161d48b4a3a8688fabe11d4c
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/55344a6e161d48b4a3a8688fabe11d4c?variant=devin-insiders
Requested by: @nicoburns

@staging-devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

This branch has not been deployed

No deployments
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.

1 participant