The flakiness is theoretically limited to Linux where we have to
use xvfb-run in CI. Because the tests are ran on Linux via ctest
we can more easily use CTest's built-in support for rerunning
failed tests. We have yet to actually observe this flakiness after
the other changes added in #2474 so it's possible the flakiness
has been entirely addressed.
While I was at it I de-duplicated some code for printing OpenGL
information.
Version 12 changes to URCT instead of MSVCRT which causes linker
issues with all the prebuilt MinGW binaries. This is still a liability
that will have to eventually be fixed.
There were 6 copies of InputImpl.hpp for Windows, macOS, Linux,
Linux DRM, iOS, and Android. All but Windows and Linux DRM were
identical so there's already an easy opportunity for consolidation.
The Windows header included some additional private static functions
and data. An equivalent way of expressing this is via more data and
functions instead the anonymous namespace of Win32/InputImpl.cpp so
that's what I did. To fix some issues with functions being used
before they're defined I moved number of functions into the anonymous
namespace of that file which resulted in adding `sf::` in lots of places.
InputImplUDev.hpp used by the DRM backend was trickier because includes
WindowImplDRM as a friend. This creates a weird dependency graph where a
distant class can call private functions that mutate private state. I
kept the definitions of those functions inside DRM/InputImpl.cpp while
moving their declaration to WindowImplDRM.cpp which is the only file
where they're used. This is a bit odd but it avoids using the
preprocessor and ensures that only the file that needs these functions
can call them.
In the end I was able to delete all 6 separate copies of InputImpl.hpp
and consolidate them into src/SFML/Window/InputImpl.hpp saving nearly
1,000 lines of code.
Because `sf::priv::InputImpl` is merely a set of static functions I
converted this class to a simple namespace. The use of a namespace is
what ultimately allowed me to untangle the DRM functions that were not
present in other implementations
In testing I tried passing `nullptr` to this function which caused
a segfault on Linux. If we know that's an invalid input then lets
assert against it.
sf::JoystickManager is a private type but the sf::Joystick API calls
into it so those asserts are still prone to fail due to incorrect
user input.
Yay for catching more UB :)
https://github.com/actions/runner-images/issues/8343#issuecomment-1727810519
Ninja's presence in the Windows images was due to it being a
transitive dependency of another packages. It was never guaranteed
to be present. The actions/runner-images devs do not plan on adding
it so we're forced to install it ourselves.
This takes a 4 second configuration time for the entire project with
examples and tests and turns that into 3! Whopping 25% reduction in
configuration time. This effect is likely to be even larger on machines
without exceptionally good internet connections.
Before:
$ hyperfine 'rm -rf build && cmake --preset dev'
Benchmark 1: rm -rf build && cmake --preset dev
Time (mean ± σ): 4.076 s ± 0.130 s [User: 2.148 s, System: 1.376 s]
Range (min … max): 3.963 s … 4.321 s 10 runs
After:
$ hyperfine 'rm -rf build && cmake --preset dev'
Benchmark 1: rm -rf build && cmake --preset dev
Time (mean ± σ): 3.007 s ± 0.313 s [User: 1.061 s, System: 1.160 s]
Range (min … max): 2.783 s … 3.805 s 10 runs
Co-authored-by: binary1248 <binary1248@hotmail.com>
Out of bounds array access is undefined behavior so let's be sure
to catch potential UB in debug builds. It's too easy to accidentally
provide an invalid index to one of these public APIs.
A fun thing about runDisplayTests() is that it means that the tests
won't get run in CI on the DRM code. Technically you can run the
display tests locally when buidling with DRM so these tests will
still fail under those circumstances. Regardless I think this is a
net positive to run the tests in CI.
This is not the best default value because it imposes additional
requirements on user builds that are not strictly necessary. This
has caused many complaints in the past as people encounter build
failures that are merely due to warnings and not hard compiler
errors.
Changing this default value emakes it more likely that someone trying
to use SFML can use it without issue.
