Navigation Menu

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

[OSX 10.7] Fullscreen mode crashes and won't compile #4744

Closed
DorpsGek opened this issue Aug 24, 2011 · 34 comments
Closed

[OSX 10.7] Fullscreen mode crashes and won't compile #4744

DorpsGek opened this issue Aug 24, 2011 · 34 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

williamg opened the ticket and wrote:

Attempting to compile on Mac OS X 10.7 on trunk (and likely every version) fails with the message:

[SRC] Compiling video/cocoa/fullscreen.mm
/Users/william/Documents/OpenTTD-dev/trunk/src/video/cocoa/fullscreen.mm: In member function ‘bool FullscreenSubdriver::SetVideoMode(int, int)’:
/Users/william/Documents/OpenTTD-dev/trunk/src/video/cocoa/fullscreen.mm:312: error: ‘CGDisplayBaseAddress’ was not declared in this scope
/Users/william/Documents/OpenTTD-dev/trunk/src/video/cocoa/fullscreen.mm:313: error: ‘CGDisplayBytesPerRow’ was not declared in this scope
make[1]: *** [video/cocoa/fullscreen.o] Error 1

According to the developer library (http://developer.apple.com/library/mac/# documentation/GraphicsImaging/Reference/Quartz_Services_Ref/DeprecationAppendix/AppendixADeprecatedAPI.html) these two functions have been obsolete for 2 years and are now removed under 10.7. The library entry suggests using Quarts or OpenGL to draw to the screen instead of writing directly to the raw framebuffer.

Reported version: trunk
Operating system: Mac OS X


This issue was imported from FlySpray: https://bugs.openttd.org/task/4744
@DorpsGek
Copy link
Member Author

planetmaker wrote:

Patch attempt inf #4752


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10251

@DorpsGek
Copy link
Member Author

planetmaker wrote:

Official binaries also crash when switching to full screen: #4695


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10252

@DorpsGek
Copy link
Member Author

leecbaker wrote:

planetmaker, in response to your question in the other # 4752, I created a new bug because I didn't realize that the Lion FS feature could potentially be a solution to the problem that is described here. I'm not sure why the patch didn't work for you, but I've added a bit more work and reattached the patch.

--

The fullscreen feature has been integrated in more with the driver, so that both the window buttons and the OTTD commands and shortcuts work to switch in and out of fullscreen. I've modified the windowed subdriver to handle both the fullscreen and windowed cases for 10.7 and above, and left the remaining code intact for 10.6 and below (switches between different subdrivers for windowed and fullscreen).

To keep the 10.6 and below fullscreen functionality intact, we still need the CGDisplayBaseAddress and CGDisplayBytesPerRow. I have commented these out in this patch, as I can't compile with them present- however, they are still needed (for 10.6 and below). It think (but I am not sure) that the code should compile fine on a 10.6 system, keeping the old fullscreen subdriver as well as the Lion fullscreen features working.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10253

@DorpsGek
Copy link
Member Author

planetmaker wrote:

This patch is complete, but same compile error as before actually (SDK 10.4u):
[i386] Compiling video/cocoa/wnd_quickdraw.mm
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm: In member function ‘virtual bool WindowQuartzSubdriver::ToggleFullscreen()’:
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:238: warning: no ‘-toggleFullScreen:’ method found
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:238: warning: (Messages without a matching method signature
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:238: warning: will be assumed to return ‘id’ and accept
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:238: warning: ‘...’ as arguments.)
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm: In member function ‘virtual bool WindowQuartzSubdriver::SetVideoMode(int, int)’:
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:281: error: ‘NSWindowCollectionBehavior’ was not declared in this scope
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:281: error: expected `;' before ‘behavior’
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:282: error: ‘behavior’ was not declared in this scope
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:283: warning: no ‘-setCollectionBehavior:’ method found
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:286: error: invalid conversion from ‘int’ to ‘NSWindowButton’
/Users/ingo/ottd/trunk/src/video/cocoa/wnd_quartz.mm:290: warning: no ‘-setCollectionBehavior:’ method found
make[1]: *** [video/cocoa/wnd_quartz.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [all] Error 1


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10254

@DorpsGek
Copy link
Member Author

leecbaker wrote:

Added include guards to solve the above issues.

Do we know what MAC_OS_X_VERSION_MAX_ALLOWED the project is currently targeting?

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10255

@DorpsGek
Copy link
Member Author

planetmaker wrote:

the compile farm still targets 10.3.9 - 10.5, thus MAC_OS_X_VERSION_MAX_ALLOWED = 10.3, using SDK 10.4u

It might be time to drop ppc support and go for 10.4 - 10.7 and maybe using a newer SDK... but that probably won't happen very quickly


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10257

@DorpsGek
Copy link
Member Author

leecbaker wrote:

What would be required to make the decision to sunset PPC? How are those decisions made?

I'm interested in implementing a few features that would improve the experience for SL and Lion users- for instance, smoother scrolling using the touchpad (NSEvent hasPreciseScrollingDeltas, 10.7+), magnify gestures (10.6+) for the map, and the fullscreen patch(10.7+) I've already started above. However, I'm unlikely to work on these if the build system doesn't support developing with newer features. A switch to a build system that did 10.4-10.7 (as you suggested above) would allow for development of these features.

As I thought about the original problem, I became curious if the current windowed quartz driver could be adapted to work in fullscreen for pre-Lion systems as well. This would allow the elimination of the fullscreen driver, including the CGDisplayBaseAddress and CGDisplayBytesPerRow issue detailed above, allowing a reduction / simplification of the cocoa video code. While these changes would probably involve quite a bit of code motion (ie. introduce more bugs), I think it would be a better way forward than trying to continue to support the deprecated direct access to graphics memory.

I think I'll stop putting more work on the Lion FS patch until I know it will be supported by the build system.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10259

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 4, 2011

planetmaker wrote:

The decision to to ditch 10.3 is rather easily made. On the other hand 10.4 is the last version to support ppc - something which should not be dropped lightly; we still support windows 95 and OpenTTD compiles still on DOS.

Anyhow we could in principle consider making separate binaries for 10.4/10.5 and 10.6/10.7 - or maybe for each version separately. Due to the way binaries are build this means an enormous amount of work though: it means to create a new cross-compile environment which can produce OSX 10.7 binaries with a linux-based compiler; something which is rather a task for months than for a weekend.

Independent of the build system, any patch must not break the compilation on an earlier OSX system but must guard itself accordingly.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10313

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 4, 2011

planetmaker wrote:

A modified version of this patch is found in r22893. The official binaries will NOT allow to switch to fullscreen when executed on OSX 10.7 while self-compiled binaries on OSX 10.7 now will compile and also allow to use the fullscreen mode.

I keep this task open until the binaries from our CF support fullscreen mode on Lion, too.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment10315

@DorpsGek
Copy link
Member Author

somaen wrote:

planetmaker: 10.5 Is the last version to support PPC, while there are legitimate reasons for users to stay on 10.4 too.

I looked into similar issues with SDL-based projects like ScummVM and ResidualVM, which both got this fixed when SDL 1.2.15 came out, and they still support PPC.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment11258

@DorpsGek
Copy link
Member Author

soulfairer wrote:

Hi,

I've made fullscreen mode work in OSX 10.8. It also uses OSX's native fullscreen mode.
Please test in as many machines with 10.7+ (I put code inside if (MacOSVersionIsAtLeast(10, 7, 0)).

I just created a default OpenGL context and set it as default, it is the only necessary code for fullscreen to work. Works entering and exiting fullscreen, seems fixed for me, but please test! Used sample code from manual, feel free to change naming & whatsoever!

Don't know if window_buffer or window_pitch have to be set to something, so I left the two lines specifying them as NULL.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment11676

@DorpsGek
Copy link
Member Author

soulfairer wrote:

Ah. I felt guilty about not destroying the GL context when we come back from fullscreen.

Here's a patch (forget about the first) also with the code to destroy the GL context when releasing resources. Probably GC would catch it, but better be safe than sorry.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment11680

@DorpsGek
Copy link
Member Author

michi_cc wrote:

Could you please use 'diff -u' to generate this (and the other) patch? Context-less and filename-less diffs aren't the most fun to work with.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment11681

@DorpsGek
Copy link
Member Author

soulfairer wrote:

Ah, sorry! I haven't programmed for this in years.
The patch (also for #4392) again.

Thank you for pointing out!

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment11682

@DorpsGek
Copy link
Member Author

soulfairer wrote:

Ops, not diff -i. Sorry! (EDIT: please disregard patch 4; also, I can't delete it)
(okay, last try! :) the last diff I did was years, years ago)

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment11684

@DorpsGek
Copy link
Member Author

asdf4812 wrote:

Are there any updates for this? Full Screen support on max os would be great ;-)

Am I right that when compiling it for myself on my local mac OS machine full screen support will work?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12036

@DorpsGek
Copy link
Member Author

somaen wrote:

I haven't tried myself for ages, although I seem to remember playing openTTD in fullscreen on my Mac (which hasn't ever run anything below 10.7). That MIGHT however have been a version from MacPorts/Fink, OR a version where I applied the patches above. It's been a while.

As I said earlier, 10.7 broke fullscreen in SDL 1.2.14, using SDL 1.2.15 (which was basically a release that fixed THAT and very little else)'s solution
should fix it unless OpenTTD has any additional code that is incompatible. (I haven't looked at the OpenTTD-sources, but I note that SDL is not a requirement
for OS X, so I expect there to be separate code paths for it.)

From experience with the projects I do code on (ResidualVM/ScummVM), SDL 1.2.15's solution has no negative effects on supporting older OS X (ResidualVM compiles for 10.4+, ScummVM for 10.2+).

So, solutions exist at least


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12042

@DorpsGek
Copy link
Member Author

michi_cc wrote:

You should be able to compile OTTD using the SDL backend for OS X, but I suspect nobody tried that in a long time. The reason we have our own backend is that there where several problems using SDL on OS X (at least for earlier SDL version).

which was basically a release that fixed THAT and very little else
The diff between 1.2.14 and 1.2.15 just for the quartz video driver is 59 kB, which doesn't feel quite that simple to me (for anybody interested: http://www.icosahedron.de/openttd/patches/SDL-quartz.diff).

There are two different patches posted in this thread, and both can't be good the way they are. osx_quartz_fullscreen.patch looks to break anything pre 10.7 (as it simply removes the call to CGDisplayBaseAddress) and will be a no-op with the current compile farm setup (10.5 SDK, and unless somebody can create an updated cross-compiler, this is unlikely to change). openttd-1.2.3-macos-fullscreen-6.patch can't work either as far as I can see, as it still sets this->window_buffer to NULL, which would lead to a crash when drawing. But as it doesn't change the immediate exit in QZ_CreateFullscreenSubdriver, the code's never going to be executed anyway.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12044

@DorpsGek
Copy link
Member Author

michi_cc wrote:

Okay, I did some more digging in the current OTTD code, and it apparently suggests that a binary compiled with the 10.7 SDK might properly support fullscreen in 10.7+. I still have no idea if openttd-1.2.3-macos-fullscreen-6.patch interacts in any way with this, because all modifications are in fullscreen.mm which isn't used at all for 10.7+.

I tried to modify the current code to produce reasonable results with our compile farm setup that uses the 10.5 SDK. The result is http://www.icosahedron.de/openttd/patches/openttd-custom-geeea764dM-OSX.zip, please try testing it on all systems you come across. Please note that it is completely untested by me.

This was created by a source code patch and a second patch for the compile farm binaries.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12045

@DorpsGek
Copy link
Member Author

somaen wrote:

Works fine on 10.8.2 (MacBook Pro, Radeon HD 6750M)


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12047

@DorpsGek
Copy link
Member Author

sanderevers wrote:

Also working on my 13" 2012 MacBook Pro. (non-retina) Also running 10.8.2


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12049

@DorpsGek
Copy link
Member Author

asdf4812 wrote:

Hi Michael,

your .zip works fine with MacBook Pro 15'' (non-retina, AMD Radeon HD 6750M) running 10.7.5 Thank you this is really great! Any chance that this will then be included in one of the next releases?

cheers

Kai


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12050

@DorpsGek
Copy link
Member Author

NG wrote:

Doesn't break anything on 10.6.8.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12086

@DorpsGek
Copy link
Member Author

ksc wrote:

Michael Lutz executable works as intended on OSX My. Lion 10.8.3

Hope anyone include this patch to the release version 1.3


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12105

@DorpsGek
Copy link
Member Author

michi_cc wrote:

Too late in the 1.3.0 release cycle to do that, but hopefully it can get into 1.3.1.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12106

@DorpsGek
Copy link
Member Author

DorpsGek commented Apr 2, 2013

ksc wrote:

1.3.0 is out without patch.
Is thera any chance to see it in 1.3.x.
And how can we contribute in this process?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12138

@DorpsGek
Copy link
Member Author

asdf4812 wrote:

OpenTTD 1.3.1-RC1 was released without the patch. Any chance that we can support you with it?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12243

@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 1, 2013

ksc wrote:

1.3.1 is out.
No patch included.
Anyone knows how to get attention of senior developers?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12256

@DorpsGek
Copy link
Member Author

Rubidium wrote:

Well, michi_cc is a developer.

I won't burn my hands on OS X as it has backfired seriously too often, and I do not have access to such a machine to test it myself which is a prerequisite for me to commit patches.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12321

@DorpsGek
Copy link
Member Author

asdf4812 wrote:

hm, looks like it should have been fixed with r25657 but I justed test it with the r25749 nightly on my Mac box and I can't even select the fullscreen checkbox. http://www.icosahedron.de/openttd/patches/openttd-custom-geeea764dM-OSX.zip was working just fine when I tested it. Any idea what could be the difference between the two? @other Mac guys: is the latest trunk working on your machine in fullscreen mode?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12571

@DorpsGek
Copy link
Member Author

ksc wrote:

OSX 10.8.4 r25749 - no Fullscreen Button.
It looks like the patched data was removed from trunc or nightly was build using old SDK version?


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12573

@DorpsGek
Copy link
Member Author

michi_cc wrote:

Hmm, it doesn't work for the compile farm binary but it does work if I compile myself. Needs investigating.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12574

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 8, 2013

ksc wrote:

r25755 nightly build from official site - no full screen button and no "full screen" menu item greyed out.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744#comment12604

@DorpsGek
Copy link
Member Author

michi_cc closed the ticket.

Reason for closing: Fixed

Starting with r25777 after correcting a misconfiguration on our compile farm.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4744

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

1 participant