Skip to content

canvas: report malloc sizeof vello_cpu - #46958

Merged
sagudev merged 7 commits into
servo:mainfrom
janeoa:report-2dcanvas
Aug 6, 2026
Merged

sagudev merged 7 commits into
servo:mainfrom
janeoa:report-2dcanvas

Conversation

@janeoa

@janeoa janeoa commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

I've added the malloc_size_of as a dependancy, so that we impl MallocSizeOf for vello_cpu::Pixmap
Added a memory_report to the canvas.
As we have two backends (vello and vello_cpu), I did one common CanvasStoreSizesPerType to report them in the backend.rs.
Added mem_profiler_chan.clone() to the constellation.

Vello_cpu does have a nice

/// Returns a reference to the underlying data as premultiplied RGBA8.
///
/// The pixels are in row-major order.
pub fn data(&self) -> &[PremulRgba8] {
    &self.buf
}

to hook into. But for the Vello I have left a TODO instead and report 0.

image

Testing: I have no auto tests, but I did spawn some canvases and saw them being reported.
Fixes: #46785

janeoa added 2 commits August 3, 2026 12:00
Signed-off-by: Asset Malik <5373400+janeoa@users.noreply.github.com>
Signed-off-by: Asset Malik <5373400+janeoa@users.noreply.github.com>
@janeoa
janeoa requested a review from sagudev as a code owner August 3, 2026 11:11
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 3, 2026

@Narfinger Narfinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks ok to me but I am not that familiar with canvas and reporting.

@mrobinson mrobinson 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.

Looks good. I have a few nits:

Comment thread components/canvas/canvas_data.rs Outdated
Comment thread components/canvas/backend.rs Outdated
Comment thread components/canvas/backend.rs Outdated
Comment thread components/canvas/vello_backend.rs Outdated
Comment thread components/canvas/vello_backend.rs Outdated
Comment thread components/canvas/canvas_paint_thread.rs Outdated
Comment thread components/canvas/canvas_data.rs Outdated
Comment thread components/canvas/canvas_data.rs Outdated
Comment thread components/canvas/vello_backend.rs Outdated
@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Aug 4, 2026
Signed-off-by: Asset Malik <5373400+janeoa@users.noreply.github.com>
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Aug 4, 2026
@janeoa
janeoa requested a review from mrobinson August 4, 2026 09:02
@janeoa

janeoa commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Looks good. I have a few nits:

Thanks for the review! Applied the requested changes.

Comment thread components/malloc_size_of/lib.rs
Comment thread components/canvas/canvas_data.rs Outdated
janeoa added 2 commits August 4, 2026 16:51
Signed-off-by: Asset Malik <5373400+janeoa@users.noreply.github.com>
Signed-off-by: Asset Malik <5373400+janeoa@users.noreply.github.com>
@janeoa
janeoa requested a review from sagudev August 5, 2026 08:31
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Aug 5, 2026
@sagudev
sagudev enabled auto-merge August 5, 2026 09:13
@janeoa

janeoa commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@mrobinson, can you please take a second look at the PR? It seem to need your approval?

Comment thread components/canvas/canvas_data.rs Outdated
Comment thread components/canvas/canvas_data.rs Outdated
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 6, 2026
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
@jdm
jdm dismissed mrobinson’s stale review August 6, 2026 08:45

Addressed.

@sagudev
sagudev added this pull request to the merge queue Aug 6, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 6, 2026
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Aug 6, 2026
Merged via the queue into servo:main with commit fefcb83 Aug 6, 2026
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 6, 2026
@janeoa
janeoa deleted the report-2dcanvas branch August 6, 2026 12:46
RichardTjokroutomo pushed a commit to RichardTjokroutomo/servo that referenced this pull request Oct 6, 2026
I've added the `malloc_size_of` as a dependancy, so that we `impl
MallocSizeOf for vello_cpu::Pixmap`
Added a `memory_report` to the canvas. 
As we have two backends (vello and vello_cpu), I did one common
`CanvasStoreSizesPerType` to report them in the `backend.rs`.
Added `mem_profiler_chan.clone()` to the constellation.

Vello_cpu does have a nice 
```rust
/// Returns a reference to the underlying data as premultiplied RGBA8.
///
/// The pixels are in row-major order.
pub fn data(&self) -> &[PremulRgba8] {
    &self.buf
}
```
to hook into. But for the Vello I have left a `TODO` instead and report
0.

<img width="416" height="301" alt="image"
src="https://github.com/user-attachments/assets/05cca5af-1957-4a21-a67e-84b77a2677b1"
/>

Testing: I have no auto tests, but I did spawn some canvases and saw
them being reported.
Fixes: servo#46785

---------

Signed-off-by: Asset Malik <5373400+janeoa@users.noreply.github.com>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
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.

2dcanvas should be integrated into memory reporting

6 participants