Fix QML build regressions on macOS #13334
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,12 +7,6 @@ | |
| #include <QQuickWindow> | ||
| #include <QtEndian> | ||
| #include <algorithm> | ||
| #endif | ||
|
|
||
| #include "control/controlobject.h" | ||
|
|
@@ -323,7 +317,7 @@ void ControllerScriptEngineLegacy::setScriptFiles( | |
| m_scriptFiles = scripts; | ||
|
|
||
| #ifdef MIXXX_USE_QML | ||
| setQMLMode(std::any_of( | ||
| m_scriptFiles.cbegin(), | ||
| m_scriptFiles.cend(), | ||
| [](const auto& scriptFileInfo) { | ||
|
|
@@ -660,7 +654,9 @@ void ControllerScriptEngineLegacy::handleScreenFrame( | |
| emit previewRenderedScreen(screenInfo, screenDebug); | ||
| } | ||
|
|
||
| // TODO: Refactor this to a `std::bit_cast` once we drop support for older | ||
| // compilers that don't support it (e.g. older than Xcode 14.3/macOS 13) | ||
| QByteArray input(reinterpret_cast<const char*>(frame.constBits()), frame.sizeInBytes()); | ||
|
There was a problem hiding this comment. AFAIR, this is what I was using, but I was requested to change it to There was a problem hiding this comment. My gut feeling would be that it would be better not to have different code paths here to avoid introducing subtle semantic differences here. From what I understand the semantics shouldn't differ, but I think it would be better to hit a bug on all platforms rather than just one (that isn't as well-tested as e.g. Linux). Perhaps a comment that this should be refactored to a There was a problem hiding this comment.
is this specific to the build environment or to the runner? Cuz the runner was recently updated #13296. Here's a little more context for reinterpret_cast vs std::bit_cast: https://stackoverflow.com/a/53402217 There was a problem hiding this comment. Should be specific to the compiler, i.e. the Xcode version that the runner uses. macOS 12 is two years old too, so my best guess would be that it doesn't support bit_cast either yet. There was a problem hiding this comment. https://developer.apple.com/xcode/cpp/ says bit-casting requires XCode 14.3. Seems like that's only available on MacOS 13... There was a problem hiding this comment. Ah okay, thanks for checking. I think as long as we're building against an SDK older than that (in CI), we are going to get an error, regardless of whether we use a newer runner. I'll update the comment. There was a problem hiding this comment. That's a neat site btw, have to bookmark that. There was a problem hiding this comment. Even better: https://en.cppreference.com/w/cpp/compiler_support Apparently they also track apple clang now, which they previously haven't which is why I defaulted to the apple site. |
||
| const TransformScreenFrameFunction& transformMethod = | ||
| m_transformScreenFrameFunctions[screenInfo.identifier]; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I understand, casting between
const char *andconst unsigned char *usingreinterpret_castseems to be fine, stackoverflow confirms that. This would fix the build on some older compilers wherestd::bit_castisn't available, such as the macOS 11 runners we are building on in CI.Would appreciate a second pair of eyes checking that this is fine though.