Closed billhollings closed 2 months ago
You should at least resolve the failing Quality Check.
You should at least resolve the failing Quality Check.
If tried many times.
clang-format
works slightly differently on macOS than on other platforms, so running ./scripts/clang_format.py main
ends up changing more than expected, and breaks the CI test here.https://github.com/KhronosGroup/Vulkan-Samples/actions/runs/10461719961?pr=1120
If you scroll to the bottom you can grab the diff that can be directly run and hopefully fix the issue.
For some reason, clang-format works slightly differently on macOS than on other platforms
Indeed, it should remove white spaces at the end of a line, and it should replace spaces with tabs. This patch shows the difference I would get when running clang-format: 0001-formatting.patch
@gpx1000 @asuessenbach
Thanks for the help on code formatting. I've got this formatting correctly now.
Plus I've added a couple of other iOS fixes, and updated the title and description above.
LGTM. Did full batch runs for iOS + macOS and both
main_loop_frame()
andmain_loop()
worked correctly. Still need fixes for non-success return codes in iOSviewDidLoad()
andrenderLoop()
, but these can be dealt with in a future PR.
@SRSaunders Would you mind giving this a formal approval, so it can be merged, please?
Once the approvals are in, what is the process for merging? Do I do that? Or is there a designated person responsible for merging?
We usually ask @marty-johnson59 to handle the merging. We usually merge on 3; but Marty, I think this is safe to merge on 2.
@SRSaunders Would you mind giving this a formal approval, so it can be merged, please?
@billhollings I don't have approval rights for this project. I am happy to provide feedback, but I cannot mark the PR as ready for merge. As you see above, @gpx1000 has that ability.
Thanks. Given this is Mac (we have limited folks who can test), I'm happy to merge w/ 2. We can revert if needed.
Refactor Platform::main_loop_frame() and main_loop(), to reduce fragility, and avoid unnecessary code duplication.
Avoid initial app_requested() test in main_loop_frame(), as it was causing looping to stop after one frame.
Since main_loop() is not a virtual function, remove declaration comment indicating it can be overridden.
Set iOS view to have a black background, to ensure transparency in HUD overlay displays in full on device.
Fix rendering scaling in IosWindow::get_dpi_factor(). Previous value was causing HUD overlay to render magnitudes too large.
Add macOS files to .gitignore.
I created this from a branch on this repo instead of a separate fork, because I was not expecting to need to make a fix.
Fixes #1121 Fixes #1131
General Checklist:
Please ensure the following points are checked:
[x] This PR describes the scope and expected impact of the changes I am making
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batch
command line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: