Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Qt 5.12.3 to work around macOS issues #56

Closed
wants to merge 10 commits into from

Conversation

JulianGro
Copy link
Member

This PR works around the issues we are experiencing when mixing macOSXSDK10.12 with Qt 5.15.2.

It is a continuation of PR vircadia/vircadia-native-core#1558

This has been CR'd up to 92907bd by @akamicah

@JulianGro JulianGro added needs CR This pull request needs to be code reviewed needs testing labels Apr 4, 2022
//
// Distributed under the Apache License, Version 2.0.
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
//
#include "TestCreator.h"

#include <assert.h>
#include "../../../libraries/shared/src/QtCompatibility.h"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There must be a better way of handling this.

@JulianGro
Copy link
Member Author

DrFran reports this running on macOS Monterey

Copy link
Member

@ksuprynowicz ksuprynowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the changes and everything looks fine in my opinion.
I only have two questions:
Should libraries/avatars/src/AvatarData.h be empty?
Does libraries/networking/src/udt/SendQueue.h need #include "QtCompatibility.h"

@JulianGro
Copy link
Member Author

Should libraries/avatars/src/AvatarData.h be empty?

What happened here is that this was changed and then changed back. I assume it only shows in the diff because some end-line character changed or something.

Does libraries/networking/src/udt/SendQueue.h need #include "QtCompatibility.h"

Technically it only requires QtCompatibility.h on Qt versions lower than 5.15 because of "Q_DISABLE_COPY_MOVE"

@ksuprynowicz
Copy link
Member

It was just tested on M1 Mac Studio with Mac OS Monterey 12.3.1 and it works.
The executable is still called "Vircadia", even though it has Overte icon. Is there an easy way of changing it?

@JulianGro
Copy link
Member Author

It's more hard coded than it should be.
Definitely something for another PR.

@ksuprynowicz
Copy link
Member

It crashes on MacOS High Sierra 10.13.6
overte-log.txt

@JulianGro
Copy link
Member Author

The only clue I get from the log is that it stops logging very close to where the hifi.gl and hifi.gpu.gl should start logging.
Other than that, we would need a backtrace to get any more info.

I don't personally see a reason why this shouldn't run on High Sierra. Maybe something prebuilt like crashpad or webrtc is built for newer versions of macOS, but I don't see a warning indicating that.

@akamicah akamicah added CR approved This pull request has been successfully code reviewed and removed needs CR This pull request needs to be code reviewed labels Jun 20, 2022
@akamicah
Copy link

Putting back into work in progress due to the crashing on MacOS High Sierra 10.13.6

@akamicah akamicah added work in progress Do not merge yet and removed needs testing CR approved This pull request has been successfully code reviewed labels Jun 22, 2022
@JulianGro JulianGro removed the work in progress Do not merge yet label Nov 25, 2022
@JulianGro JulianGro closed this Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants