Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Encode directly to command buffer for Metal. #49785

Merged
merged 19 commits into from
Jan 20, 2024

Conversation

jonahwilliams
Copy link
Member

@jonahwilliams jonahwilliams commented Jan 14, 2024

Part of flutter/flutter#140804

Rather than using impeller::Command, the impeller::RenderPass records most state directly into the Vulkan command buffer. This should remove allocation/free overhead of the intermediary structures and make further improvements to the backend even easier.

This completely removes the background worker threads used for encoding, which should lower overall CPU usage without decreasing performance by much (though it may be a tad slower or a tad faster).

@jonahwilliams
Copy link
Member Author

Requires #49780 since these touch most of the same code.

@jonahwilliams jonahwilliams marked this pull request as ready for review January 16, 2024 22:57
MTLRenderPassDescriptor* desc_ = nil;
std::string label_;
bool is_metal_trace_active_ = false;
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: this

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@bdero bdero left a comment

Choose a reason for hiding this comment

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

LGTM

raster_message_loop_->PostTaskToAllWorkers([]() {
// See https://github.com/flutter/flutter/issues/65752
// Intentionally opt out of QoS for raster task workloads.
[[NSThread currentThread] setThreadPriority:1.0];
Copy link
Member

Choose a reason for hiding this comment

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

Get rid of the corresponding Foundation.h header import above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

/// There should be no change to rendering if this caching was
/// absent.
///
struct PassBindingsCache {
Copy link
Member

Choose a reason for hiding this comment

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

Move to its own TU and rename as PassBindingsCacheMTL.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

bool has_valid_pipeline_ = false;
bool has_label_ = false;
BufferView index_buffer_;
PrimitiveType primitive_type_;
Copy link
Member

Choose a reason for hiding this comment

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

Default this ivars.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 20, 2024
@auto-submit auto-submit bot merged commit 4d67f26 into flutter:main Jan 20, 2024
@jonahwilliams jonahwilliams deleted the direct_encoding_metal branch January 20, 2024 03:15
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 20, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 20, 2024
…141917)

flutter/engine@53a2436...4d67f26

2024-01-20 jonahwilliams@google.com [Impeller] Encode directly to command buffer for Metal. (flutter/engine#49785)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jonahwilliams@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants