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

Macos support #14

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

dominickm
Copy link

@dominickm dominickm commented Apr 1, 2019

For your consideration: I have added macOS Mojave command line support

  • Now checking for macOS special case
  • In case of macOS, loading GLSL 330 vertex and shared files

I am happy to make any additions to the ReadMe re macOS configuration if you like. However, the only real dependency is Mojave and the standard Rust tool-chain. I would also be happy to create macOS binaries for the release page.

@dominickm
Copy link
Author

Saw your comment re Arch in the other PR. That line is removed here as well because it breaks macOS as well as 18.10. I understand if you reject this for the same reason. I'd like to keep my fork up as an macOS port in that case if it's all the same to you and we can't find a way to catch the Arch case. I don't have an Arch install myself, so it would be difficult for me to help on that.

@dominickm dominickm closed this Apr 1, 2019
@dominickm
Copy link
Author

Closing for now. Going to try to special case arch https://github.com/schultyy/os_type

@dominickm dominickm reopened this Apr 1, 2019
@dominickm
Copy link
Author

I have no Arch install but this (in theory) handles the Arch case. If someone could test that to be safe that would probably be best. If the macOS changes have to stay separate, that's cool too.

@unlimitedbacon
Copy link
Owner

I'll try to test this out once I get home from work tonight.

@dominickm
Copy link
Author

I appreciate that a lot and sorry for all the Github emails I must have generated for you today :)

@unlimitedbacon
Copy link
Owner

Don't worry about it. The contribution is definitely welcome.

@unlimitedbacon
Copy link
Owner

I tested this on my system and unfortunately it doesn't work. Here is what is returned for os.

OSInformation { os_type: Unknown, version: "0.0.0" }

I think this is because I am running Antergos instead of straight Arch. This is the output of lsb_release -a.

LSB Version:	1.4
Distributor ID:	Antergos
Description:	Antergos Linux
Release:	rolling
Codename:	n/a

I suspect that Manjaro and any other Arch based distros would have the same problem. I think a different approach is required.

@dominickm
Copy link
Author

Yeah, so I did some digging and it looks like any rolling distro is going to have this problem and there are too many of them to catch individually if we go this route.

It's a big catch 22 though even ignoring macOS, because the version in master is broken on Ubuntu in most common configurations. Also, the root issue that forced the env for Arch actually seems to be an underlying bug not having to do with this project.

That leaves us a few options that all aren't great:

  1. Attempt to figure out the root cause of needing the env line for Arch etc. I'm sceptical on this one because I am sure you would have figured it out well in the past and my research has been less than fruitful on tht front.
  2. Add 'Unknown' to the method os_type I currently have. I don't like this one too much either because it will likely create issues for users in the future.
  3. For a short time (until we figure out the issue or its just fixed due to the passage of time and Mesa versions), I could just keep my fork up and maintain for macOS and Ubuntu until the issue gets resolved upstream or one of us or someone else figures it out. This is problematic in the way all forks are but for at least a short time, I am going to need to maintain macOS support anyway, so maybe that's the way to go?

Let me know what you think and if you have any other ideas / ways to skin this cat.

Thanks!

@unlimitedbacon
Copy link
Owner

I think that, for the purpose of this specific PR (macOS support), we should be fine using conditional compilation instead of os_type. We can worry about the problem on Ubuntu separately, and get this merged.

The original env lines were

    #[cfg(target_os = "linux")]
    env::set_var("MESA_GL_VERSION_OVERRIDE", "2.1");

which should prevent it from trying to set the environment variable on macOS. Was this not the case? If so, you can use the same syntax for choosing the correct shaders.

    #[cfg(target_os = "macos")]

@dominickm
Copy link
Author

OK, I will make those changes and update the PR after work today.

@unlimitedbacon
Copy link
Owner

Any progress on this?

@dominickm
Copy link
Author

Got swamped in day job. I'll jump back on this over the weekend. Sorry for the delay.

@unlimitedbacon
Copy link
Owner

No problem.

@unlimitedbacon unlimitedbacon mentioned this pull request Jul 20, 2019
@awatertrevi
Copy link

Is there a reason why this hasn't been merged?

@unlimitedbacon
Copy link
Owner

unlimitedbacon commented Nov 15, 2019

Currently this breaks some OSs besides macOS. It needs some tweaks to detect which OS is being used correctly.

@dominickm
Copy link
Author

Hey I'm back from under the lurch here. Which OS does this break?
I'm cool with conditional compilation if that solves the issue. Does it? @unlimitedbacon

@unlimitedbacon
Copy link
Owner

Yes I think conditional compilation is probably the way to go here. We'll have to test it on each platform to be sure, though.

@awatertrevi
Copy link

@unlimitedbacon I can help with the testing if desired.

@dominickm
Copy link
Author

Actually, this has gotten more complicated on Catalina. Basically by default a ton of the required libs no longer exist. I am not sure macOS support is really worth perusing, given the level of effort by my estimation.

@unlimitedbacon
Copy link
Owner

That is unfortunate. Is it because Apple has deprecated OpenGL?

@lotuc
Copy link

lotuc commented May 22, 2023

Hi, I'm on macOS Ventura 3.3.1 (a). Here I created a commit that works on my system, in case anyone wants it.

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.

None yet

4 participants