[Impeller] Don't define CanvasRecorder if IMPELLER_TRACE_CANVAS is not set. (flutter/engine#46476)
Internal bug: b/303067268
https://github.com/flutter/engine/pull/46376 is causing a breakage to the internal engine build because of 543348a044/impeller/aiks/canvas_recorder.h (L58-L62). Internally, we do not set `IMPELLER_TRACE_CANVAS`.
It looks like the cause is that the internal toolchain causes the `static_assert` to be compiled even though the template is not instantiated.
@chingjun helped me to figure out the following:
https://stackoverflow.com/questions/5246049/c11-static-assert-and-template-instantiation points us to the spec. In the later version (ISO/IEC 14882:2017(E)):
> The program is ill-formed, no diagnostic required, if ... no valid specialization can be generated for a template or a substatement of a constexpr if statement (9.4.1) within a template and the template is not instantiated,
<details>
<summary>The relevant section</summary>

</details>
Interpretation: the compiler can either choose to emit the error caused by the `static_assert` or not. Currently the compiler used by the build here on LUCI does not; internally it does.
For example, the following links shows that simply changing the Clang version affects whether the error appears or not for a minimal template.
- ok: https://godbolt.org/z/n9nYrcvcP
- not ok: https://godbolt.org/z/fWcvdcn35
Hence, `#ifdef` out the class instead of using a `static_assert` for more consistent behavior across these two toolchains.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
@@ -47,6 +47,9 @@ enum CanvasRecorderOp : uint16_t {
|
||||
DrawAtlas,
|
||||
};
|
||||
|
||||
// Canvas recorder should only be used when IMPELLER_TRACE_CANVAS is defined
|
||||
// (never in production code).
|
||||
#ifdef IMPELLER_TRACE_CANVAS
|
||||
/// Static polymorphic replacement for impeller::Canvas that records methods
|
||||
/// called on an impeller::Canvas and forwards it to a real instance.
|
||||
/// TODO(https://github.com/flutter/flutter/issues/135718): Move this recorder
|
||||
@@ -55,12 +58,6 @@ enum CanvasRecorderOp : uint16_t {
|
||||
template <typename Serializer>
|
||||
class CanvasRecorder {
|
||||
public:
|
||||
#ifndef IMPELLER_TRACE_CANVAS
|
||||
// Canvas recorder should only be used when IMPELLER_TRACE_CANVAS is defined
|
||||
// (never in production code).
|
||||
static_assert(false);
|
||||
#endif
|
||||
|
||||
CanvasRecorder() : canvas_() { serializer_.Write(CanvasRecorderOp::New); }
|
||||
|
||||
explicit CanvasRecorder(Rect cull_rect) : canvas_(cull_rect) {
|
||||
@@ -286,5 +283,6 @@ class CanvasRecorder {
|
||||
Canvas canvas_;
|
||||
Serializer serializer_;
|
||||
};
|
||||
#endif
|
||||
|
||||
} // namespace impeller
|
||||
|
||||
Reference in New Issue
Block a user