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

Fix clang tidy error and crash #42564

Merged
merged 1 commit into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion impeller/renderer/backend/metal/context_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
auto context = std::shared_ptr<ContextMTL>(new ContextMTL(
device,
MTLShaderLibraryFromFileData(device, shader_libraries_data, label),
worker_task_runner, std::move(is_gpu_disabled_sync_switch)));
std::move(worker_task_runner), std::move(is_gpu_disabled_sync_switch)));
if (!context->IsValid()) {
FML_LOG(ERROR) << "Could not create Metal context.";
return nullptr;
Expand Down
9 changes: 7 additions & 2 deletions impeller/renderer/backend/metal/formats_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,13 @@

auto des = [[MTLDepthStencilDescriptor alloc] init];

des.depthCompareFunction = ToMTLCompareFunction(depth->depth_compare);
des.depthWriteEnabled = depth->depth_write_enabled;
// These temporary variables are necessary for clang-tidy (Fuchsia LLVM
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the crash that this fixes happening on CI. Could we maybe split this out into a separate issue/PR?

Copy link
Member Author

@knopp knopp Jun 5, 2023

Choose a reason for hiding this comment

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

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8779203752399309425/+/u/test:_lint_host_debug/stdout?format=raw

Screenshot 2023-06-05 at 19 12 58

The first error is a crash (no error message). This is from #42399

I can try to split the PR.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, yeah, sorry. I see it now.

// version 17.0.0git) to not crash.
auto compare_function = ToMTLCompareFunction(depth->depth_compare);
auto depth_write_enabled = depth->depth_write_enabled;

des.depthCompareFunction = compare_function;
des.depthWriteEnabled = depth_write_enabled;

if (front.has_value()) {
des.frontFaceStencil = ToMTLStencilDescriptor(front.value());
Expand Down