Skip to content

Port vello_editor example to vello_cpu + rename to just editor - #669

Merged
nicoburns merged 7 commits into
linebender:mainfrom
nicoburns:vello-cpu-editor
Jul 30, 2026
Merged

Port vello_editor example to vello_cpu + rename to just editor#669
nicoburns merged 7 commits into
linebender:mainfrom
nicoburns:vello-cpu-editor

Conversation

@nicoburns

Copy link
Copy Markdown
Collaborator

Extracted from #662.

Ports the vello_editor example to use vello_cpu instead of vello. This actually makes things quite a bit simpler as we avoid having to deal with all of the WGPU setup code.

Comment on lines +450 to +454
builder.fill_glyphs(glyph_run.positioned_glyphs().map(|glyph| Glyph {
id: glyph.id,
x: glyph.x,
y: glyph.y,
}));

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching from glyph_run.glyphs() (+manually computing positions) to glyph_run.positioned_glyphs() was a drive-by change from the LLM, but a good one so I've kept it.

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not ran this yet. On a first pass, the set_brush function feels like a massive code smell; it isn't obvious to me why the 'just always pass a color around' and/or pass a Vello CPU paint around were so unviable that this was the best option.

Comment on lines +285 to +290
if let (Some(width), Some(height)) =
(NonZeroU32::new(size.width), NonZeroU32::new(size.height))
{
render_state.surface.resize(width, height).unwrap();
}
self.handle_resize(to_nonzero_u16(size.width), to_nonzero_u16(size.height));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this still works when the window is minimised (so the surface doesn't get resized but the render context does...)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... we probably just need to make sure it doesn't render in that case.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing locally on macOS, minimising seems to work fine.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a huge surprise; the thing which needs to be checked is how it handles on Windows. I believe that minimising on macOS doesn't change the window size.

It's possible that this has changed on recent winits?
I no longer have a windows machine, so can't test this easily.

Comment thread examples/editor/src/text.rs Outdated
Comment on lines +430 to +432
renderer.set_stroke(Stroke::new(width.into()));
set_brush(renderer, &style.brush);
renderer.stroke_path(&line.to_path(0.0));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are arguments for using ink skipping underline here, but that's non-blocking.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to wait for a vello_cpu for this, then it will be trivial :)

@nicoburns

Copy link
Copy Markdown
Collaborator Author

it isn't obvious to me why the 'just always pass a color around' and/or pass a Vello CPU paint around were so unviable that this was the best option.

I had exactly the same thought. And the answer is that parley::Brush requires Default (which Color doesn't implement) and PartialEq (which PaintType doesn't implement). So it was either this or a newtype until that's fixed.

@DJMcNab

DJMcNab commented Jul 6, 2026

Copy link
Copy Markdown
Member

I think using a newtype is probably the best option, as it's a similar amount of code, but more accurately reflects the state of the feature.

@nicoburns

Copy link
Copy Markdown
Collaborator Author

Updated to a newtype around peniko::Color for the Brush

@nicoburns
nicoburns requested a review from DJMcNab July 6, 2026 14:44

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's okay for this to land without testing minimising on Windows. Please do just add a comment that we've not tested it and it might not work before landing.

@nicoburns

Copy link
Copy Markdown
Collaborator Author

Video evidence that this does not crash when minimised on Windows is attached. This was generated automatically by an AI running in a VM:

rec-2234d1ca-9fcb-44fb-a798-4452018740d7-edited.mp4

@nicoburns
nicoburns added this pull request to the merge queue Jul 30, 2026
Merged via the queue into linebender:main with commit 45583a3 Jul 30, 2026
24 checks passed
@nicoburns
nicoburns deleted the vello-cpu-editor branch July 30, 2026 03:16
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