google / google/filament

Exceptions are disabled in Windows prebuilt library?

Open
#9,306 4 comments 0 reactions 0 assignees View on GitHub
build
Dominant language
C++
Stars
20.5k
Forks
2.3k
Avg merge
2d 16h
Merged PRs (30d)
74

Description

Hi,

I'm using the prebuilt Windows zips from the Releases section (https://github.com/google/filament/releases).

It looks like exceptions are disabled in that build. I'm building a material and trying to catch errors, so that my program doesn't terminate if there's a problem (e.g. material version mismatch):

```
const std::vector material_bytes = read_bytes(material_path);
filament::Material* material = nullptr;
try
{
material = filament::Material::Builder()
.package(material_bytes.data(), material_bytes.size())
.build(*impl->engine);
} catch (const utils::PostconditionPanic& e)
{
std::string error_msg =
std::format("Failed to build material from file {}: {}", material_path.string(), e.what());
spdlog::error(error_msg);
throw std::runtime_error(error_msg);
} catch (const utils::PreconditionPanic& e)
{
std::string error_msg = std::format("Precondition failed while building material from file {}: {}",
material_path.string(), e.what());
spdlog::error(error_msg);
throw std::runtime_error(error_msg);
}
```

However this doesn't reach the `catch` sections, and it looks like `std::abort()` is called in `TPanic` in `Panic.cpp`. If exceptions were enabled, I believe it should not reach `std::abort()`, but rather throw: https://github.com/google/filament/blob/a89711b006b0230d0d4514c7b621f402ff969f7f/libs/utils/src/Panic.cpp#L227

If I'm indeed right, I'd say this is a bad default, because users of Filament likely want to catch Filament exceptions and not have their programs abort. Of course I could build Filament myself with exceptions enabled, but I'd argue that's beside the point - the pre-built zips are more convenient and should cater for the most common use case, which I'd say is throwing, not calling `std::abort()`. If someone **does** want exceptions disabled, I'd consider it a special use case, and then those users can build Filament without exceptions if they need that.

Thoughts?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.