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

Mac os10 3 screensaver fix #2149

Merged
merged 6 commits into from
Oct 5, 2017
Merged

Conversation

CharlieFenton
Copy link
Contributor

BOINC screensavers have 2 parts. The BOINC "screensaver coordinator" is an actual "native" screensaver for MS Windows (boinc.scr) or Macintosh (BOINCSaver.saver). It communicates with the BOINC client via RPCs to determine which project applications (tasks) are currently running, and which of those have associated graphics executables. Depending on the user's screensaver preference settings, it cycles through the running tasks, launching one of the associated graphics executables at a time. The graphics executables usually communicate with their associated worker app to display information about the work being done by that task.

In addition to being run by the BOINC screensaver coordinator, users can run the graphics executables from the BOINC Manager via the "Show graphics" button when they select a graphics-capable running task.

When no active tasks with associated graphics are running, BOINC's screensaver coordinator launches BOINC's default graphics executable (boincscr). Depending on the user's screensaver preference settings, it may also intersperse displaying boincscr with project application graphics.

On MS Windows, and also on Macs prior to OS 10.13, the graphics executables display their windows over the screensaver coordinator's window, giving the appearance that they are part of the native screensaver. But Mac OS 10.13 has changes which prevent any application from covering the native screensaver's window.

I designed the changes in this PR with several main goals:

  • Minimize the effort required for projects to update their graphics executables.

  • Use the same logic and methodology as before for displaying updated graphics executables on MS Windows and on Macs running versions of OS X prior to OS 10.13 (backward compatibility.)

  • Use the same logic and methodology as before for displaying graphics executables via BOINC Manager's "Show graphics" button, whether or not the graphics executable has been updated, and on any version of OS X we currently support (OS 10.6 - OS 10.13) (backward compatibility).

  • On Mac OS 10.13, give the user an explanation of why graphics executables that have not yet been updated don't appear in the screensaver, and move on as quickly and efficiently as possible to ones that can be displayed.

The fix for this problem on OS 10.13 involves establishing communication between the screensaver coordinator and the graphics executable via Mach-O inter-process communication. It also takes advantage of a feature available in OS X called IOSurfaceBuffer. IOSurfaceBuffer allows OpenGL applications to share images with another process, while continuing to take advantage of the rendering acceleration provided by the GPU. We can use this approach because BOINC graphics executables built with the standard BOINC graphics libraries use OpenGL.

To update their graphics executables, projects simply need to follow these 3 steps:
[1] Rebuild the BOINC graphics library for the Mac (libboinc_graphics2.a) from the sources in this PR
[2] Relink the executable with the updated library
[3] Distribute the updated executable to BOINC clients

Here are a few more details about updating graphics executables:

The graphics library (libboinc_graphics2.a) is built as target gfx2libboinc in the Xcode project.

Do not be concerned if you see errors that any of these are missing, as they will be built by Xcode before they are needed: MultiGPUMig.h, MultiGPUMigServer.c, MultiGPUMigServer.h or MultiGPUMigUser.c.

It is easiest to build libboinc_graphics2.a using Xcode. The following commands in the Mac's Terminal app (/Applications/Utilities/Terminal) will build all the BOINC libraries:
$ cd {path to boinc sources}/mac_build/
$ source BuildMacBOINC.sh -lib
For more information and additional options, see section [6] of the file "mac_build/HowToBuildBOINC_XCode.rtf" from the GIT repository.

If you must build libboinc_graphics2.a using a make file, you must modify the make file to first compile api/MultiGPUMig.defs, which will create the files MultiGPUMig.h, MultiGPUMigServer.c, MultiGPUMigServer.h and MultiGPUMigUser.c.

If you use Xcode to build your graphics app, you must add IOSurface.framework to the list of "External Frameworks and Libraries" in your project. If you build your graphics app using the command line (for example, with cc and ld), you must also add "-framework IOSurface" to your linker flags (LDFLAGS).

Charlie Fenton added 3 commits September 30, 2017 04:57
…OS 10.13.

NOTE: This needs to be merged into both master and 7.8 branch. Xcode project in master builds and links keyword.cpp and keyword.h, which are not present i 7.8 branch, so this commit does not include those.
@CharlieFenton
Copy link
Contributor Author

NOTE: This will need to be merged into both master and the 7.8 branch. The Xcode project in master builds and links keyword.cpp and keyword.h, which are not present in the 7.8 branch, so this commit does not include those. As a result, the automatic build may fail.

@AenBleidd
Copy link
Member

@CharlieFenton, could you please take a look why CI is broken on OSX build?
Detailed log is here: https://travis-ci.org/BOINC/boinc/jobs/281874828
Thanks

@CharlieFenton
Copy link
Contributor Author

CharlieFenton commented Oct 3, 2017 via email

@AenBleidd
Copy link
Member

Hello @CharlieFenton,
But these files are presented in master branch but Xcode project doesn't contain them.
Does this mean that you want this to be merged to 7.8 branch first and then you will and these files to Xcode project for master branch too?
Because from my POV (I can make a mistake here) build will fail on master branch after merge too because those files are not presented in Xcode projects and are not linked to the executable.
In this case I'd prefer to retarget this pull request to 7.8 branch only and make another pull request to master branch with added files keywords.cpp and keywords.h to Xcode project.
What do you think?
Thanks

…OS 10.13. Added file needed for Mach-O communication.
@CharlieFenton
Copy link
Contributor Author

CharlieFenton commented Oct 3, 2017 via email

@AenBleidd
Copy link
Member

Thank you, @CharlieFenton

…ent and manager in Xcode project for compatibility with master branch
@CharlieFenton
Copy link
Contributor Author

CharlieFenton commented Oct 5, 2017 via email

@davidpanderson
Copy link
Contributor

fine w/ me

@ChristianBeer
Copy link
Member

I would like to point out that merging ones own pull request is forbidden by the BOINC development policy:

It is important to note that for the purposes of this workflow, anyone who is submitting a proposal for a change or contributing code is considered a 'contributor'. Even if that person is on the PMC or is a committer, they cannot take the actions of a committer with respect to their own proposals or code changes

see: https://github.com/BOINC/boinc-policy/blob/master/Development_Workflow.md

@davidpanderson davidpanderson merged commit dd2be87 into master Oct 5, 2017
@davidpanderson
Copy link
Contributor

Thanks, Charlie.

@CharlieFenton
Copy link
Contributor Author

CharlieFenton commented Oct 5, 2017 via email

@CharlieFenton CharlieFenton deleted the Mac_OS10_3_Screensaver_Fix branch October 5, 2017 09:56
@ChristianBeer
Copy link
Member

Charlie: I'm sorry if my comment came across as insensitive to you, I just wanted to prevent another policy violation. The development process was put in place to allow a proper review of changes by other committers and enforce a four eye principle for all changes. The relevant party here are the committers (not the PMC). While discussing the development process it was often pointed out that certain changes can only be implemented and reviewed by a single person and thus we should allow a committer to merge own pull requests. The problem is: where do we draw the line? The conclusion was that if the author of a change describes what the change does and why it is necessary any other committer who doesn't have a deep knowledge of the code in question should be able to review the change. As has happened here.

The description in the pull request is very detailed and contains all the important documentation. A review has happened and the PR was augmented as necessary. You also stated that this is urgent and another committer showed up to merge this. So the new development process worked.

I would like to point out that no one on the PMC has submitted a single comment regarding this PR, despite the urgency of the matter and the time I took to explain its purpose and methodology.

The PMC is not aware of pull requests and has no power regarding whether they are merged or not. They get called upon when there is a dispute and even then they rarely comment.

While I appreciate that we are all volunteers here, the rules must be realistic in terms of making progress on BOINC, so that progress does not get mired in bureaucracy, and so contributors don't give up in disgust.

I just want to say that I also worked lots of hours in my free time and had uncountable sleepless nights because of BOINC and the way BOINC is developed. I participated in creating and establishing the new development process mainly to steer BOINC towards a real and sustainable open source model with a consensual development process.

The process does not hinder progress in BOINC at all. It tries to make sure that changes introduced by contributors make sense and are free of bugs (to a certain degree). If there are urgent fixes they should be marked as such and the contributor can directly pester other committers and ask them to review and merge in this case.

@TheAspens
Copy link
Member

TheAspens commented Oct 5, 2017

I'm working on getting Jonathan Armstrong (WCG's primary science application developer) to test our screen savers against this code. He should be able to engage on this today. Is the best place to pull from the 7.8 client branch?

@TheAspens
Copy link
Member

I just talked to Jonathan - he says that this code is beyond his knowledge of mac development so he would not have been able to do a code review, but he has built some of our screen savers against this pull request and we are currently upgrading a mac to high sierra to test those screen savers.

@TheAspens
Copy link
Member

I know we are late to the party, but we have rebuilt our graphics and run them with the 7.8.3 client on high sierra and can confirm the fix. We have also regression tested the new build with a 7.6.33 client on sierra. So it looks good.

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