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.

Sign up for GitHub

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

Jump to bottom

fix: making gl thing to compile and work on apple silicon #1922

Merged
merged 3 commits into from
Feb 20, 2021
Merged

fix: making gl thing to compile and work on apple silicon #1922

merged 3 commits into from
Feb 20, 2021

Conversation

sirkon
Copy link
Contributor

@sirkon sirkon commented Feb 7, 2021 •
edited
Loading

Description:

Changes to make fyne to compile and work on Apple Silicon. It is based on recent changes in Go, where GOOS for iPhones and iPads was changed to ios and GOOS=darwin now only refers to MacOS. golang/go#42100

Fixes #1739

PS This is not a final solution, rather show a way of workable configuration to build for Apple Silicon.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

@sirkon sirkon mentioned this pull request Feb 7, 2021
Closed
@Jacalz Jacalz mentioned this pull request Feb 8, 2021
Closed
andydotxyz
andydotxyz requested changes Feb 8, 2021
View reviewed changes
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

This is a great fix, thanks - but one of the build rules looks off (mentioned inline).

internal/painter/gl/gl_core.go
@@ -1,4 +1,4 @@
// +build !gles,!arm,!arm64,!android,!ios,!mobile
Copy link
Member

Choose a reason for hiding this comment

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

I think this change is not quite right.
If I have a linux arm64 this file should not be activated, but with that removed it will

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, know this. Some options were just off. Rewrite all things with the following ideas in mind:

  1. *_core.go files all getting x OR (arm64 AND darwin), this means they are to be used with MacOS on Apple M1.
  2. *_es.go files all getting `x AND (!arm64 AND darwin OR arm64 !darwin). This means they are excluded when compiling for MacOS on Apple M1.

Where x represents the existing set conditions.

BTW, !arm64 AND darwin OR arm64 !darwin == !(arms64 AND darwin), this is a rule of boolean algebra.
In fact I could simplify the negation in _es files for Apple M1 if keeping existing conditions in mind, but decided to stay it this way to keep things cleaner. You will be able to put another pairs if needed in inclusive conditions and append lines with exclusive conditions with just copy-pasting and minor editing.

Copy link
Member

Choose a reason for hiding this comment

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

I think this has overcomplicated it now, sorry. the ES stuff should never run on darwin, and the core always should (as iOS is a different flag). All of the others were OK as far as I could see, just this one not.
Re-adding the "!arm64" to this line should and adding "arm64,darwin" at the end should have done it.

Copy link
Member

Choose a reason for hiding this comment

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

I think the new change for this file is correct - but all of the other updates were not needed.
I'm not certain that your summary of boolean algebra is correct - you seem to have described exclusive or, which is not !(x and y).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which is not !(x and y)

shit, indeed. Turned all conditions into x AND !darwin for _es and x OR darwin for _core

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keep in mind though, this will work for Go > 1.16 only.

Copy link
Member

Choose a reason for hiding this comment

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

I cannot see which elements of this change are only supporting Go 1.16?
We support back to 1.12, we will upgrade to 1.14 or 1.15 once Debian releases testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GOOS=darwin for iOS prior Go 1.16. It is GOOS=ios since 1.16.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, we embed the gomobile project for building ios and that sets the tag.
Does that mean that there is no problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this indeed

andydotxyz
andydotxyz requested changes Feb 8, 2021
View reviewed changes
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

I think it was closer before (see inline)

internal/painter/gl/gl_core.go
@@ -1,4 +1,4 @@
// +build !gles,!arm,!arm64,!android,!ios,!mobile
Copy link
Member

Choose a reason for hiding this comment

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

I think this has overcomplicated it now, sorry. the ES stuff should never run on darwin, and the core always should (as iOS is a different flag). All of the others were OK as far as I could see, just this one not.
Re-adding the "!arm64" to this line should and adding "arm64,darwin" at the end should have done it.

internal/painter/gl/gl_core.go
@@ -1,4 +1,4 @@
// +build !gles,!arm,!arm64,!android,!ios,!mobile
Copy link
Member

Choose a reason for hiding this comment

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

I think the new change for this file is correct - but all of the other updates were not needed.
I'm not certain that your summary of boolean algebra is correct - you seem to have described exclusive or, which is not !(x and y).

zdima
zdima reviewed Feb 8, 2021
View reviewed changes
Copy link
Contributor

@zdima zdima left a comment

Choose a reason for hiding this comment

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

I still having issue with "service port" when application start.
GLFW: An uncaught error has occurred: PlatformError: Cocoa: Failed to find service port for display. Do I have to pull other versions of GL*?

internal/painter/gl/gl_es.go
@@ -1,5 +1,6 @@
// +build gles arm arm64,!darwin
// +build gles arm arm64
Copy link
Contributor

Choose a reason for hiding this comment

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

the previous change in this file was good.
it is compiled on M1 with previous changes.

internal/driver/glfw/glfw_es.go Outdated
Comment on lines 1 to 2
// +build gles arm arm64
// +build !arm64,!darwin
// +build !arm64,darwin arm64,!darwin
Copy link
Contributor

Choose a reason for hiding this comment

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

based on previous comments, I think this should be just one line:
// +build gles arm arm64,!darwin
i.e. build on any arm64 but not Darwin.

@andydotxyz
Copy link
Member

Could we get @ipfans to test this as well please?

@zdima some of the messages above are not totally clear. Can you confirm whether or not this latest version works for you please?

@zdima
Copy link
Contributor

zdima commented Feb 9, 2021

Could we get @ipfans to test this as well please?

@zdima some of the messages above are not totally clear. Can you confirm whether or not this latest version works for you please?

I adde to the go.mod in github.com/fyne-io/examples:
replace fyne.io/fyne/v2 => github.com/sirkon/fyne/v2 v2.0.1-0.20210209074931-f93d4e04b627

Run:
go clean -I -r; go mod tidy; go build ./examples
Project compiled without issues and program runs.
There is output on the console:

$ ./examples 
GLFW: An uncaught error has occurred: PlatformError: Cocoa: Failed to find service port for display
GLFW: Please report this bug in the Go package immediately.
2021/02/09 11:12:35 PlatformError: Cocoa: Failed to find service port for display

@andydotxyz andydotxyz merged commit 18d696c into fyne-io:develop Feb 20, 2021
andydotxyz pushed a commit that referenced this pull request Feb 20, 2021
@sirkon @andydotxyz
fix: making gl thing to compile and work on apple silicon (#1922)
7a58971 
darwin is pure desktop with gl core to use
@Jacalz Jacalz mentioned this pull request Mar 8, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zdima zdima zdima left review comments

@andydotxyz andydotxyz andydotxyz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sirkon @andydotxyz @zdima