[Impeller] Avoid heap-allocating std::function captures per draw by artemkulyk · Pull Request #192679 · flutter/flutter

artemkulyk · 2026-09-12T05:30:18Z

UberSDF and circle contents build per-draw callbacks that capture the sampler binding and geometry result by value. Those captures exceed the std::function small-buffer optimization and heap-allocate on every draw.

DrawGeometry invokes the callbacks before returning, so capturing by reference is valid and removes those allocations.

Fixes #192673
Related to #164607

test-exempt: capture-by-ref only; DrawGeometry remains synchronous

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Made with Cursor

UberSDF and circle contents build callbacks per draw that capture the sampler binding and geometry result by value; these captures exceed the small-buffer optimization and heap-allocate. DrawGeometry invokes the callbacks synchronously within the call, so capture by reference is safe and removes two allocations per draw. test-exempt: capture-by-ref only; DrawGeometry remains synchronous. Measured on Apple M1 (Metal, release), interleaved medians of primitive_rendering_benchmarks: BM_DrawOval/AAFilled/ImpellerMetalSDF/16 -5.9% CPU, BM_DrawRect/AAFilled/ImpellerMetalSDF/16 -1.5% CPU.

b-luk · 2026-09-14T18:05:40Z

@artemkulyk Is this PR still in progress, or is it ready for review?

artemkulyk · 2026-09-14T18:08:27Z

@b-luk , It is ready for review, but I cannot open it, because I have two other PRs open.

b-luk · 2026-09-14T19:28:57Z

/gemini review

gemini-code-assist

Code Review

This pull request modifies CircleContents and UberSDFContents in Impeller to capture local variables by reference instead of moving them by value into lambda callbacks. Comments have been added to explain that capturing by reference is safe because the callbacks are executed before DrawGeometry returns. Additionally, the optional header was included in uber_sdf_contents.h. I have no feedback to provide.

b-luk · 2026-09-14T19:40:59Z

+      // DrawGeometry invokes this before returning, so capturing locals by
+      // reference is safe.


I don't think we need an explicit comment about this. Other pipeline Contents files do not have this comment, so leaving this out would be consistent with the rest of the files.

Removed. Keeping it consistent with the other Contents.

b-luk · 2026-09-14T19:41:02Z

+      // geometry_result by reference is safe.
+      [&geometry_result](const ContentContext& renderer, const Entity& entity,
+                         RenderPass& pass,
+                         const Geometry* geometry) { return geometry_result; });


Can we omit create_geom_callback completely? It has a default of DefaultCreateGeometryCallback:

flutter/engine/src/flutter/impeller/entity/contents/color_source_contents.h

Lines 117 to 118 in a638a66

const CreateGeometryCallback& create_geom_callback =

DefaultCreateGeometryCallback) {

implemented at

flutter/engine/src/flutter/impeller/entity/contents/color_source_contents.h

Line 245 in a638a66

return geom->GetPositionBuffer(renderer, entity, pass);

which is what both CircleContents and UberSDFContents both are passing in.

/*force_stencil=*/false is also the default and could be omitted.

Most other pipeline Contents' Render functions already omit these and use the defaults, e.g. SolidColorContents

flutter/engine/src/flutter/impeller/entity/contents/solid_color_contents.cc

Lines 68 to 74 in a638a66

return ColorSourceContents::DrawGeometry<VS>(

renderer, entity, pass, pipeline_callback, frame_info,

[&frag_info, &data_host_buffer](RenderPass& pass) {

FS::BindFragInfo(pass, data_host_buffer.EmplaceUniform(frag_info));

pass.SetCommandLabel("Solid Fill");

return true;

});

so removing these from CircleContents and UberSDFContents makes them more similar to the other ones.

Done in both. Default makes the same call, both geometries are kNormal, so no behavior change.

…ntents CircleContents and UberSDFContents were the only contents passing a custom create_geom_callback, and it only returned a precomputed GetPositionBuffer result. The default callback performs the same call, and neither CircleGeometry nor UberSDFGeometry overrides GetResultMode (both are kNormal). force_stencil was false as well, so the stencil-then-cover path never engaged. This also removes the now-unused geometry_result precompute. test-exempt: refactor with no behavior change; existing golden tests cover both pipelines.

artemkulyk · 2026-09-14T23:28:48Z

Applied in 24489a0. Thanks!

artemkulyk · 2026-09-15T10:45:52Z

/gemini review

gemini-code-assist

Code Review

This pull request simplifies the rendering pipeline setup in CircleContents and UberSDFContents by removing the manual retrieval of the geometry position buffer and the associated create_geom_callback argument. Additionally, the lambda capture for sampler_binding in UberSDFContents has been simplified to capture by reference, and an include for was added to its header file. There are no review comments, and I have no feedback to provide.

b-luk · 2026-09-15T18:53:50Z

@b-luk , It is ready for review, but I cannot open it, because I have two other PRs open.

This PR seems fine to me. You can switch one of your other open PRs to draft so that you can mark this one as ready.

b-luk

LGTM

I don't know if this will have any meaningful impact on performance. But the simplified code is a benefit.

artemkulyk · 2026-09-15T19:33:08Z

Thanks! Earlier isolated A/B runs on M1 Metal showed about 2% lower CPU time for SDF rectangles; oval and circle results were within noise. I haven't remeasured after the callback cleanup, so I wouldn't claim a broad performance improvement.

gemini-code-assist

Code Review

This pull request simplifies the rendering implementation in CircleContents and UberSDFContents by removing redundant geometry position buffer retrievals and custom geometry callbacks. It also refactors lambda captures in UberSDFContents and adds the header to uber_sdf_contents.h. There are no review comments, and I have no feedback to provide.

auto-submit · 2026-09-16T23:47:19Z

autosubmit label was removed for flutter/flutter/192679, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

b-luk · 2026-09-17T00:07:31Z

@gaaclarke for second review

gaaclarke

lgtm, getting the position buffer and passing it in should be the same as passing in that geometry

github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Sep 12, 2026

gemini-code-assist Bot reviewed Sep 14, 2026

View reviewed changes

b-luk requested changes Sep 14, 2026

View reviewed changes

b-luk added the CICD Run CI/CD label Sep 14, 2026

Merge branch 'master' into pr/sdf-contents-allocations

8265d68

flutter-dashboard Bot removed the CICD Run CI/CD label Sep 15, 2026

gemini-code-assist Bot reviewed Sep 15, 2026

View reviewed changes

b-luk added the CICD Run CI/CD label Sep 15, 2026

b-luk approved these changes Sep 15, 2026

View reviewed changes

artemkulyk marked this pull request as ready for review September 16, 2026 06:20

gemini-code-assist Bot reviewed Sep 16, 2026

View reviewed changes

Merge branch 'master' into pr/sdf-contents-allocations

c17d8f7

flutter-dashboard Bot removed the CICD Run CI/CD label Sep 16, 2026

b-luk added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Sep 16, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 16, 2026

b-luk requested a review from gaaclarke September 17, 2026 00:07

gaaclarke approved these changes Sep 17, 2026

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 17, 2026

auto-submit Bot added this pull request to the merge queue Sep 17, 2026

Merged via the queue into flutter:master with commit 642d396 Sep 17, 2026
36 of 38 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 17, 2026

justinmc mentioned this pull request Sep 25, 2026

Flutter Notable Commits - 2026 #181433

Open

		// DrawGeometry invokes this before returning, so capturing locals by
		// reference is safe.

	const CreateGeometryCallback& create_geom_callback =
	DefaultCreateGeometryCallback) {

	return ColorSourceContents::DrawGeometry<VS>(
	renderer, entity, pass, pipeline_callback, frame_info,
	[&frag_info, &data_host_buffer](RenderPass& pass) {
	FS::BindFragInfo(pass, data_host_buffer.EmplaceUniform(frag_info));
	pass.SetCommandLabel("Solid Fill");
	return true;
	});

Conversation

artemkulyk commented Sep 12, 2026

Pre-launch Checklist

Uh oh!

b-luk commented Sep 14, 2026

Uh oh!

artemkulyk commented Sep 14, 2026

Uh oh!

b-luk commented Sep 14, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

b-luk Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

artemkulyk Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

b-luk Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

artemkulyk Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

artemkulyk commented Sep 14, 2026

Uh oh!

artemkulyk commented Sep 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

b-luk commented Sep 15, 2026

Uh oh!

b-luk left a comment

Choose a reason for hiding this comment

Uh oh!

artemkulyk commented Sep 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

auto-submit Bot commented Sep 16, 2026

Uh oh!

b-luk commented Sep 17, 2026

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants