Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
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
12 changes: 10 additions & 2 deletions fml/log_level.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ typedef int LogSeverity;
constexpr LogSeverity kLogInfo = 0;
constexpr LogSeverity kLogWarning = 1;
constexpr LogSeverity kLogError = 2;
constexpr LogSeverity kLogFatal = 3;
constexpr LogSeverity kLogNumSeverities = 4;
// A log that is not an error, is important enough to display even if ordinary
// info is hidden.
constexpr LogSeverity kLogImportant = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this break users who set a log level on the command line? If so, should we add a note in our release notes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't - I'm replacing the FATAL level with IMPORTANT - it's not actually possible to ignore FATAL, but I suppose someone who was suppressing ERROR will now get IMPORTANT. I doubt this is happening much if at all though.

constexpr LogSeverity kLogFatal = 4;
constexpr LogSeverity kLogNumSeverities = 5;

// DEPRECATED: Use |kLogInfo|.
// Ignoring Clang Tidy because this is used in a very common substitution macro.
Expand All @@ -31,6 +34,11 @@ constexpr LogSeverity LOG_WARNING = kLogWarning;
// NOLINTNEXTLINE(readability-identifier-naming)
constexpr LogSeverity LOG_ERROR = kLogError;

// DEPRECATED: Use |kLogImportant|.
// Ignoring Clang Tidy because this is used in a very common substitution macro.
// NOLINTNEXTLINE(readability-identifier-naming)
constexpr LogSeverity LOG_IMPORTANT = kLogImportant;

// DEPRECATED: Use |kLogFatal|.
// Ignoring Clang Tidy because this is used in a very common substitution macro.
// NOLINTNEXTLINE(readability-identifier-naming)
Expand Down
19 changes: 5 additions & 14 deletions fml/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ namespace fml {
namespace {

#if !defined(OS_FUCHSIA)
const char* const kLogSeverityNames[kLogNumSeverities] = {"INFO", "WARNING",
"ERROR", "FATAL"};
const char* const kLogSeverityNames[kLogNumSeverities] = {
"INFO", "WARNING", "ERROR", "IMPORTANT", "FATAL"};

const char* GetNameForLogSeverity(LogSeverity severity) {
if (severity >= kLogInfo && severity < kLogNumSeverities) {
Expand All @@ -45,14 +45,6 @@ const char* StripDots(const char* path) {
return path;
}

const char* StripPath(const char* path) {
auto* p = strrchr(path, '/');
if (p) {
return p + 1;
}
return path;
}

#if defined(OS_FUCHSIA)

zx_koid_t GetKoid(zx_handle_t handle) {
Expand Down Expand Up @@ -98,9 +90,7 @@ LogMessage::LogMessage(LogSeverity severity,
const char* file,
int line,
const char* condition)
: severity_(severity),
file_(severity > kLogInfo ? StripDots(file) : StripPath(file)),
line_(line) {
: severity_(severity), file_(StripDots(file)), line_(line) {
#if !defined(OS_FUCHSIA)
stream_ << "[";
if (severity >= kLogInfo) {
Expand Down Expand Up @@ -144,7 +134,6 @@ LogMessage::~LogMessage() {
#if !defined(OS_FUCHSIA)
stream_ << std::endl;
#endif

if (capture_next_log_stream_) {
*capture_next_log_stream_ << stream_.str();
capture_next_log_stream_ = nullptr;
Expand All @@ -153,6 +142,7 @@ LogMessage::~LogMessage() {
android_LogPriority priority =
(severity_ < 0) ? ANDROID_LOG_VERBOSE : ANDROID_LOG_UNKNOWN;
switch (severity_) {
case kLogImportant:
case kLogInfo:
priority = ANDROID_LOG_INFO;
break;
Expand All @@ -172,6 +162,7 @@ LogMessage::~LogMessage() {
#elif defined(OS_FUCHSIA)
FuchsiaLogSeverity severity;
switch (severity_) {
case kLogImportant:
case kLogInfo:
severity = FUCHSIA_LOG_INFO;
break;
Expand Down
30 changes: 30 additions & 0 deletions fml/logging_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@
#include "flutter/fml/build_config.h"
#include "flutter/fml/log_settings.h"
#include "flutter/fml/logging.h"
#include "fml/log_level.h"
#include "gtest/gtest.h"

namespace fml {
namespace testing {

static_assert(fml::kLogFatal == fml::kLogNumSeverities - 1);
static_assert(fml::kLogImportant < fml::kLogFatal);
static_assert(fml::kLogImportant > fml::kLogError);

#ifndef OS_FUCHSIA
class MakeSureFmlLogDoesNotSegfaultWhenStaticallyCalled {
public:
Expand Down Expand Up @@ -67,5 +72,30 @@ TEST(LoggingTest, UnreachableKillProcessWithMacro) {
ASSERT_DEATH({ FML_UNREACHABLE(); }, "");
}

#ifndef OS_FUCHSIA
TEST(LoggingTest, SanityTests) {
::testing::FLAGS_gtest_death_test_style = "threadsafe";
std::vector<std::string> severities = {"INFO", "WARNING", "ERROR",
"IMPORTANT"};

for (size_t i = 0; i < severities.size(); i++) {
LogCapture capture;
{
LogMessage log(i, "path/to/file.cc", 4, nullptr);
log.stream() << "Hello!";
}
EXPECT_EQ(capture.str(), std::string("[" + severities[i] +
":path/to/file.cc(4)] Hello!\n"));
}

ASSERT_DEATH(
{
LogMessage log(kLogFatal, "path/to/file.cc", 5, nullptr);
log.stream() << "Goodbye";
},
R"(\[FATAL:path/to/file.cc\(5\)\] Goodbye)");
}
#endif // !OS_FUCHSIA

} // namespace testing
} // namespace fml
2 changes: 1 addition & 1 deletion shell/platform/android/android_context_gl_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static std::shared_ptr<impeller::Context> CreateImpellerContext(
FML_LOG(ERROR) << "Could not add reactor worker.";
return nullptr;
}
FML_LOG(ERROR) << "Using the Impeller rendering backend (OpenGLES).";
FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (OpenGLES).";
return context;
}

Expand Down
6 changes: 3 additions & 3 deletions shell/platform/android/android_context_vulkan_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ static std::shared_ptr<impeller::Context> CreateImpellerContext(

if (context && impeller::CapabilitiesVK::Cast(*context->GetCapabilities())
.AreValidationsEnabled()) {
FML_LOG(ERROR) << "Using the Impeller rendering backend (Vulkan with "
"Validation Layers).";
FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (Vulkan with "
"Validation Layers).";
} else {
FML_LOG(ERROR) << "Using the Impeller rendering backend (Vulkan).";
FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (Vulkan).";
}

return context;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
FML_LOG(ERROR) << "Could not create Metal Impeller Context.";
return nullptr;
}
FML_LOG(ERROR) << "Using the Impeller rendering backend.";

return context;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ - (instancetype)initWithMTLDevice:(id<MTLDevice>)device
return nil;
}

FML_LOG(IMPORTANT) << "Using the Skia rendering backend (Metal).";

_resourceContext->setResourceCacheLimit(0u);
}
return self;
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/embedder/embedder_surface_gl_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ EmbedderSurfaceGLImpeller::EmbedderSurfaceGLImpeller(
}

gl_dispatch_table_.gl_clear_current_callback();
FML_LOG(ERROR) << "Using the Impeller rendering backend (OpenGL).";
FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (OpenGL).";
valid_ = true;
}

Expand Down
18 changes: 9 additions & 9 deletions shell/platform/embedder/embedder_surface_metal_impeller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@
metal_dispatch_table_(std::move(metal_dispatch_table)),
external_view_embedder_(std::move(external_view_embedder)) {
std::vector<std::shared_ptr<fml::Mapping>> shader_mappings = {
std::make_shared<fml::NonOwnedMapping>(impeller_entity_shaders_data,
impeller_entity_shaders_length),
std::make_shared<fml::NonOwnedMapping>(impeller_entity_shaders_data,
impeller_entity_shaders_length),
#if IMPELLER_ENABLE_3D
std::make_shared<fml::NonOwnedMapping>(impeller_scene_shaders_data,
impeller_scene_shaders_length),
std::make_shared<fml::NonOwnedMapping>(impeller_scene_shaders_data,
impeller_scene_shaders_length),
#endif // IMPELLER_ENABLE_3D
std::make_shared<fml::NonOwnedMapping>(impeller_modern_shaders_data,
impeller_modern_shaders_length),
std::make_shared<fml::NonOwnedMapping>(impeller_framebuffer_blend_shaders_data,
impeller_framebuffer_blend_shaders_length),
std::make_shared<fml::NonOwnedMapping>(impeller_modern_shaders_data,
impeller_modern_shaders_length),
std::make_shared<fml::NonOwnedMapping>(impeller_framebuffer_blend_shaders_data,
impeller_framebuffer_blend_shaders_length),
};
context_ = impeller::ContextMTL::Create(
(id<MTLDevice>)device, // device
Expand All @@ -52,7 +52,7 @@
std::make_shared<fml::SyncSwitch>(false), // is_gpu_disabled_sync_switch
"Impeller Library" // library_label
);
FML_LOG(ERROR) << "Using the Impeller rendering backend (Metal).";
FML_LOG(IMPORTANT) << "Using the Impeller rendering backend (Metal).";

valid_ = !!context_;
}
Expand Down