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

Fix swap buffers (update the visible window contents) on macOS #15

Open
wants to merge 2 commits into
base: GL-version
Choose a base branch
from

Conversation

michaliskambi
Copy link

( Note: This branch is done on top of #14 . Once you apply #14 , this PR will contain only 1 commit. )

I'm testing GLPT on macOS 12.3.1 x86_64 . (I do not have a physical machine, I'm actually using remote session to a macOS machine provided by MacStadium for Castle Game Engine development.)

Observed problem: The screen contents do not update. E.g. ./gears run and look like a static image, even though adding logs shows that angle += 0.02; is done and the screen is redrawn. Same with triangle in ./simple demo. The demo ./two_windows is a bit weird, with windows updating irregularly once you move them.

This suggests a problem with not doing OpenGL "swap buffers". I see that TOpenGLView.drawRect calls openGLContext.flushBuffer (which should swap buffers, https://developer.apple.com/documentation/appkit/nsopenglcontext/1436211-flushbuffer ), but it does not seem to do anything on my setup. Doing it from Cocoa_SwapBuffers makes things update reliably.

Reading Apple docs, and with my zero Cocoa knowledge today, I cannot really tell why does the change work. Reading docs https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_drawing/opengl_drawing.html , they show using [context flushBuffer]; from -(void) drawRect, so exactly what you're doing before the PR. Yet it doesn't work for me, moving it to Cocoa_SwapBuffers works.

administrator added 2 commits May 17, 2022 00:22
Since FPC 3.2.0, one has to use `ObjCBool` instead of `Boolean`, otherwise GLPT compilation fails at various overrides because the methods are declared in ancestor with `ObjCBool` (not compatible with Pascal `Boolean`).

E.g. in latest FPC, `packages/cocoaint/src/appkit/NSWindow.inc` defines

```
function initWithContentRect_styleMask_backing_defer (contentRect: NSRect; aStyle: NSUInteger; bufferingType: NSBackingStoreType; flag: ObjCBOOL): instancetype; message 'initWithContentRect:styleMask:backing:defer:';
```

In GLPT, this should be overridden with `flag: ObjCBool`, not `flag: Boolean`.

See https://fpcwiki.coderetro.net/User_Changes_3.2.0#objcbase .
flushBuffer call is necessary from Cocoa_SwapBuffers
@genericptr
Copy link
Collaborator

in Cocoa_SwapBuffers try using win^.ref.openGLView.setNeedsDisplay_(true) instead of display and keep the flush buffers call in drawRect. Look at my branch in https://github.com/genericptr/GLPT also if you want. I made so many changes for my own use without merging into main I've made a massive mess of things. :)

@michaliskambi
Copy link
Author

in Cocoa_SwapBuffers try using win^.ref.openGLView.setNeedsDisplay_(true) instead of display and keep the flush buffers call in drawRect. Look at my branch in https://github.com/genericptr/GLPT also if you want. I made so many changes for my own use without merging into main I've made a massive mess of things. :)

Thank you!

  1. Testing your described fix: It works but not ideally (result is a bit different, I see different rotation speed).

    What I did:

    • In Cocoa_SwapBuffers I removed win^.ref.contentView.display, added win^.ref.contentView.setNeedsDisplay_(true)

    • And I left the win^.glcontext.flushBuffer call from TOpenGLView.drawRect (just as it is in the original code in this repo)

    It works... but the gears rotate now with quite crazy fast speed. Possibly a lot of frames are dropped in this approach?

    I see the gears.pp just does angle += 0.02;, without multiplying the increase by the passed time. So indeed the gears can rotate much quicker if GLPT_SwapBuffers doesn't really limit the frames to 60 FPS.

    The rotation speed after this PR from me, and on your fork on https://github.com/genericptr/GLPT, are OK on macOS. They also match rotation speed I observe on Linux from the same example.

  2. I also tested your fork, https://github.com/genericptr/GLPT , and it works great without any modificatons. (I submitted Fixes to gears.pp example genericptr/GLPT#2 but I didn't need any modifications to base GLPT unit.) The screen updates, the wheels rotate with the correct speed.

    So your fork is also cool, and apparently you did more fixes than just win^.ref.contentView.setNeedsDisplay_(true) change above :)

  3. Admittedly I am a bit fuzzy why this change is even necessary. Reading the https://stackoverflow.com/questions/52938516/opengl-not-rendering-on-macos-mojave you linked in your fork comments didn't clear this for me enough. Reading about the difference between display (draw now) and setNeedsDisplay_ (set a flag to draw later), my gut reaction is that:

    • The current approach (in this repo, without my PR) should work. Cocoa_SwapBuffers calls display, and then TOpenGLView.drawRect calls win^.glcontext.flushBuffer. So this an indirect way to call win^.glcontext.flushBuffer from Cocoa_SwapBuffers. But the tests show it doesn't work -- I do not understand why, even though I invented a workaround.

    • My approach (in this repo, after applying this PR) should be equivalent. I just call win^.glcontext.flushBuffer directly. But the tests show it's not equivalent: my change solves the problem.

@daar
Copy link
Owner

daar commented May 22, 2022

Hi thanks for the PR! I’m now traveling without much access to internet. In two weeks I will return and will process your request.

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

3 participants