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

uiOpenGLArea #405

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

uiOpenGLArea #405

wants to merge 52 commits into from

Conversation

mischnic
Copy link
Contributor

@mischnic mischnic commented Aug 6, 2018

I updated @pcwalton's work onto master.
Closes #265. Closes #34

On Linux, mesa-common-dev is needed for building (or libgl1-mesa-dev?)
bildschirmfoto 2018-09-02 um 23 34 43

TODO:

  • Windows: SetPixelFormat can only be called once per window
    This is the modern way of creating an OpenGL context (needed for multisampling and sRGB frambuffers):
    • Create a "fake context" only to get function pointers for a newer context creation method (involves SetPixelFormat)
    • Create the actual context with the requested attributes and SetPixelFormat the actual format.
    • But SetPixelFormat can only be called once per Window, so a "fake window" needs to be created for the fake context.
    • How should that dummy window be created & destroyed?
  • Windows: key events not working
    • areaevents.cpp areaDoEvents:
      • some uMsg == msgAreaKeyDown for uiArea
      • no uMsg == WM_KEYDOW for uiOpenGLArea
    • is areaFilter working for uiOpenGLArea as well?
  • Windows: onMouseCrossed stops working after clicking once onto the area (does this happen with uiArea as well?)

To discuss:

General:

  • How to do error handling (e.g. if requested attributes like OpenGL version area not available):
    • Possible situations:
      • missing required extensions/functions (needed by libui code)
      • missing requested extensions/functions/versions (requested by user)
    • Solutions:
      • return an error code in uiNewOpenGLArea and add an enum for these OR return NULL and pass an error string in some other way
      • a uiprivUserbug can't be caught so a program can't even display "Your graphics card is not supported".
  • What about scrolling OpenGL areas?

Linux:

  • Does including X11 libraries always use XWayland, is there a difference to before?
    • Instead of GLX, EGL could be used on Wayland (the native interface there). But I don't know how to detect this at runtime and whether these libraries would need to be loaded dynamically regarding these:
  • What if the Linux GL library is missing at runtime?
  • Linux: use GtkGLArea / GdkGLContext ?
  • Ubuntu 18.10 with libgl1-mesa-dev:
-- Found X11: /usr/lib/x86_64-linux-gnu/libX11.so
CMake Warning (dev) at /usr/share/cmake-3.12/Modules/FindOpenGL.cmake:270 (message):
  Policy CMP0072 is not set: FindOpenGL prefers GLVND by default when
  available.  Run "cmake --help-policy CMP0072" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.

  FindOpenGL found both a legacy GL library:

    OPENGL_gl_LIBRARY: /usr/lib/x86_64-linux-gnu/libGL.so

  and GLVND libraries for OpenGL and GLX:

    OPENGL_opengl_LIBRARY: /usr/lib/x86_64-linux-gnu/libOpenGL.so
    OPENGL_glx_LIBRARY: /usr/lib/x86_64-linux-gnu/libGLX.so

  OpenGL_GL_PREFERENCE has not been set to "GLVND" or "LEGACY", so for
  compatibility with CMake 3.10 and below the legacy GL library will be used.

macOS:

  • Does this apply:

bildschirmfoto 2018-10-25 um 20 47 12

https://twitter.com/pcwalton/status/1055528025247240192

@andlabs
Copy link
Owner

andlabs commented Aug 6, 2018

Try putting the area in a box and marking it stretchy, then adding a button. There's glitches involving the sizing of areas that are by themselves in windows on GTK+.

@andlabs
Copy link
Owner

andlabs commented Aug 7, 2018

Did that work?

@mischnic
Copy link
Contributor Author

mischnic commented Aug 7, 2018

Did that work?

Yes!

@mischnic

This comment has been minimized.

@mischnic
Copy link
Contributor Author

mischnic commented Aug 7, 2018

@andlabs Is the macOS area refactoring acceptable? areaCommonView now handles all events and is the superclass of openGLAreaView and areaView .

Also, why are the macOS classes in lowerCameCase (Cocoa's arent: NSOpenGLArea, NSView).

@mischnic
Copy link
Contributor Author

mischnic commented Aug 9, 2018

Do we want a scrolling opengl view?

@andlabs
Copy link
Owner

andlabs commented Aug 9, 2018

I'll look at all of that after releasing Alpha 4.

lowerCaseCamelCase is a habit I acquired from various places, including Go. The new convention is uiprivClassNameCamelCase, with uipriv still being lowercase to maximize collision avoidance.

@mischnic
Copy link
Contributor Author

mischnic commented Sep 3, 2018

Instead of GLX, EGL could be used on Wayland (the native interface there). But I dont know how to detect this at runtime and if these libraries would need to be loaded dynamically regarding these:

That should then be the same behaviour as the Gtk GLArea.
When was GdkGLContext added? Can it be used seperately?

@mischnic mischnic changed the title OpenGL control uiOpenGLArea Sep 10, 2018
@ThomasCassimon
Copy link

Hi, I was wondering what the current state of this feature is?

The 3.0 changelog seems to indicate this was planned for the 4.0 release, but I don't believe this feature has made it in yet, has it?

CI seems to be failing for 32-bit Linux, because it can't find "-lGL", (the GL library), but when looking at the CMakeLists.txt I was wondering why you are manually specifying the library names instead of using FindOpenGL?

@mischnic
Copy link
Contributor Author

Hi, I was wondering what the current state of this feature is?

Waiting for @andlabs' feedback/review.

I was wondering why you are manually specifying the library names instead of using FindOpenGL?

Fixed. 😄 Let's see what happens.

@mischnic
Copy link
Contributor Author

@andlabs Why isn't Travis running? For #424 neither.

@mischnic
Copy link
Contributor Author

mischnic commented Oct 6, 2018

@andlabs ?

@andlabs
Copy link
Owner

andlabs commented Oct 6, 2018

I don't know; that's weird. Travis still lets me see the status of libui, but it's behind...

@andlabs
Copy link
Owner

andlabs commented Oct 6, 2018

Okay I guess I needed to just log in again and it seems to be back.

I should probably get to updating both the issues and pull requests for this now (I was taking a break after releasing Alpha 4 and package ui, but IDK how long that will continue for)

@andlabs andlabs added this to the Post-Alpha 5 milestone Dec 31, 2018
@paulrouget
Copy link

The Mac example doesn't start rendering until the window is resized, and then, it flickers a lot.

Any idea what's going on?

bors-servo pushed a commit to servo/servo that referenced this pull request Feb 6, 2019
Some tweaks in libsimpleservo C API

- Adding logs.
- Adding OpenGL support for OSX.
- Fixing file reading issue.

With this, I can load Servo into a C app (with the C version of libui + andlabs/libui#405).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22832)
<!-- Reviewable:end -->
@mischnic
Copy link
Contributor Author

mischnic commented Feb 6, 2019

The Mac example doesn't start rendering until the window is resized

Could you please check if onDrawGL is called before resizing and if it is, whether the correct width & height (= non-zero) values are passed?

diff --git a/examples/opengl/main.c b/examples/opengl/main.c
index 1fff59c9..ab19e5a7 100644
--- a/examples/opengl/main.c
+++ b/examples/opengl/main.c
@@ -277,6 +277,7 @@ static void onInitGL(uiOpenGLAreaHandler *h, uiOpenGLArea *a)
 static void onDrawGL(uiOpenGLAreaHandler *h, uiOpenGLArea *a, double width, double height)
 {
        GLCall(glViewport(0, 0, width, height));
+       printf("%f %f\n", width, height);

        GLCall(glClear(GL_COLOR_BUFFER_BIT));

Should print: 600.000000 415.000000

and then, it flickers a lot.

Maybe disabling VSync (uiOpenGLAreaSetVSync(a, 0); in setup) helps?
Is there a difference with this change? (Still flickering when moving the mouse over the area?)

diff --git a/examples/opengl/main.c b/examples/opengl/main.c
index 1fff59c9..5e1c9914 100644
--- a/examples/opengl/main.c
+++ b/examples/opengl/main.c
@@ -362,8 +363,6 @@ int main(void)
        uiOpenGLArea *glarea = uiNewOpenGLArea(&AREA_HANDLER, attribs);
        uiBoxAppend(b, uiControl(glarea), 1);

-       uiTimer(1000/60, render, glarea);
-
        uiControlShow(uiControl(mainwin));
        uiMain();

Works fine for me (macOS 10.13.6, Intel Graphics).

@paulrouget
Copy link

onDrawGL is called before resizing.

It's printing the right size I believe (600x416).

Disabling vsync doesn't change anything.

Still flickers without the timer.

I think every odd frame is transparent:

Screencast: https://streamable.com/12xcn

bors-servo pushed a commit to servo/servo that referenced this pull request Feb 7, 2019
Some tweaks in libsimpleservo C API

- Adding logs.
- Adding OpenGL support for OSX.
- Fixing file reading issue.

With this, I can load Servo into a C app (with the C version of libui + andlabs/libui#405).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22832)
<!-- Reviewable:end -->
@mischnic
Copy link
Contributor Author

mischnic commented Feb 7, 2019

@paulrouget I guess you're running Mojave?

This should definitely fix the blank-before-resize-issue:

diff --git a/darwin/openglarea.m b/darwin/openglarea.m
index d98dbb9e..8d289c05 100644
--- a/darwin/openglarea.m
+++ b/darwin/openglarea.m
@@ -82,6 +83,8 @@ static void assignNextPixelFormatAttribute(NSOpenGLPixelFormatAttribute *as, uns
                self->libui_a->ctx = [[NSOpenGLContext alloc] initWithFormat:self->libui_a->pixFmt shareContext:nil];
                if(self->libui_a->ctx == nil)
                        uiprivUserBug("Couldn't create OpenGL context!");
+               [self->libui_a->ctx setView:self];
+               [self->libui_a->ctx update];

                [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(viewBoundsDidChange:) name:NSViewFrameDidChangeNotification object:self];
        }

When setting wantsLayer to true, the flickering happens for me as well, please try this change: (coincidentally, the tweet I mentioned in the PR description a while ago describes exactly this)

diff --git a/darwin/openglarea.m b/darwin/openglarea.m
index d98dbb9e..8d289c05 100644
--- a/darwin/openglarea.m
+++ b/darwin/openglarea.m
@@ -42,6 +42,8 @@ static void assignNextPixelFormatAttribute(NSOpenGLPixelFormatAttribute *as, uns
                [self setupNewTrackingArea];
                self->libui_enabled = YES;

+               [self setWantsLayer:NO];
+
                NSOpenGLPixelFormatAttribute attrs[ATTRIBUTE_LIST_SIZE];
                unsigned int attrIndex = 0;
                assignNextPixelFormatAttribute(attrs, &attrIndex, NSOpenGLPFAColorSize);

@paulrouget
Copy link

I guess you're running Mojave?

Yes. 10.14.1.

This should definitely fix the blank-before-resize-issue:
[self->libui_a->ctx setView:self];
[self->libui_a->ctx update];

This change changes nothing.

When setting wantsLayer to true, the flickering happens for me as well, please try this change

Setting setWantsLayer to NO changes nothing.

Setting setWantsLayer to YES fixes the blank-before-resize-issue.

No matter the configuration/changes, it's flickering.

@mischnic
Copy link
Contributor Author

mischnic commented Feb 8, 2019

setWantsLayer:NO on Mojave: Blank before resize and flickering on every other frame
setWantsLayer:NO on High Sierra: Works fine
setWantsLayer:YES on Mojave: Flickering
setWantsLayer:YES on High Sierra: Flickering

The flickering could be related to double buffering?

@pcwalton You have investigated this some time ago, any suggestion?

@mischnic mischnic mentioned this pull request Jul 4, 2020
@Shadowblitz16
Copy link

Will this work on mac os?
I know opengl is deprecated on mac os.

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.

OpenGL control
7 participants