From 8cec7d1a400caee75bdf9fffce302728828237c0 Mon Sep 17 00:00:00 2001 From: Elias Naur Date: Thu, 28 Nov 2019 18:46:33 +0100 Subject: [PATCH] app/internal/window: move context refresh to window Instead of calling from the low level context into the window for its surface and dimensions, add a Context.MakeCurrent method that does it directly. The result is simpler and clearer logic. For example, synchronization is obviously no longer needed. It wasn't necessary before, but the reason was unclear. Signed-off-by: Elias Naur --- app/internal/egl/egl.go | 141 ++++++++++------------------- app/internal/window/egl_android.go | 42 ++++++--- app/internal/window/egl_wayland.go | 87 +++++++++--------- app/internal/window/egl_windows.go | 43 ++++++--- app/internal/window/egl_x11.go | 40 ++++++-- app/internal/window/os_wayland.go | 2 - app/internal/window/os_x11.go | 8 +- 7 files changed, 187 insertions(+), 176 deletions(-) diff --git a/app/internal/egl/egl.go b/app/internal/egl/egl.go index 54bd6494..43722530 100644 --- a/app/internal/egl/egl.go +++ b/app/internal/egl/egl.go @@ -15,27 +15,15 @@ import ( type Context struct { c *gl.Functions - driver Driver + disp _EGLDisplay eglCtx *eglContext - eglWin NativeWindowType eglSurf _EGLSurface width, height int // For sRGB emulation. srgbFBO *gl.SRGBFBO } -type Driver interface { - EGLDisplay() NativeDisplayType - EGLDestroy() -} - -type WindowDriver interface { - EGLWindow(visID int) (NativeWindowType, int, int, error) - NeedVSync() bool -} - type eglContext struct { - disp _EGLDisplay config _EGLConfig ctx _EGLContext visualID int @@ -75,28 +63,20 @@ func (c *Context) Release() { c.srgbFBO.Release() c.srgbFBO = nil } - c.destroySurface() - c.eglWin = nilEGLNativeWindowType + c.ReleaseSurface() if c.eglCtx != nil { - eglDestroyContext(c.eglCtx.disp, c.eglCtx.ctx) - eglTerminate(c.eglCtx.disp) + eglDestroyContext(c.disp, c.eglCtx.ctx) + eglTerminate(c.disp) eglReleaseThread() c.eglCtx = nil } - if c.driver != nil { - c.driver.EGLDestroy() - c.driver = nil - } } func (c *Context) Present() error { - if c.eglWin == nilEGLNativeWindowType { - panic("context is not active") - } if c.srgbFBO != nil { c.srgbFBO.Blit() } - if !eglSwapBuffers(c.eglCtx.disp, c.eglSurf) { + if !eglSwapBuffers(c.disp, c.eglSurf) { return fmt.Errorf("eglSwapBuffers failed (%x)", eglGetError()) } if c.srgbFBO != nil { @@ -105,17 +85,17 @@ func (c *Context) Present() error { return nil } -func NewContext(d Driver) (*Context, error) { - eglCtx, err := createContext(d.EGLDisplay()) +func NewContext(disp NativeDisplayType) (*Context, error) { + eglDisp := eglGetDisplay(disp) + if eglDisp == nilEGLDisplay { + return nil, fmt.Errorf("eglGetDisplay failed: 0x%x", eglGetError()) + } + eglCtx, err := createContext(eglDisp) if err != nil { return nil, err } - if _, windowed := d.(WindowDriver); !windowed && !eglCtx.surfaceless { - eglDestroyContext(eglCtx.disp, eglCtx.ctx) - return nil, errors.New("EGL_KHR_surfaceless_context not supported") - } c := &Context{ - driver: d, + disp: eglDisp, eglCtx: eglCtx, c: new(gl.Functions), } @@ -130,59 +110,36 @@ func (c *Context) Lock() {} func (c *Context) Unlock() {} -func (c *Context) destroySurface() { +func (c *Context) ReleaseSurface() { if c.eglSurf == nilEGLSurface { return } // Make sure any in-flight GL commands are complete. c.c.Finish() - eglMakeCurrent(c.eglCtx.disp, nilEGLSurface, nilEGLSurface, nilEGLContext) - eglDestroySurface(c.eglCtx.disp, c.eglSurf) + eglMakeCurrent(c.disp, nilEGLSurface, nilEGLSurface, nilEGLContext) + eglDestroySurface(c.disp, c.eglSurf) c.eglSurf = nilEGLSurface } -func (c *Context) MakeCurrent() error { - wdriver, ok := c.driver.(WindowDriver) - if !ok { - if !eglMakeCurrent(c.eglCtx.disp, nilEGLSurface, nilEGLSurface, c.eglCtx.ctx) { - return fmt.Errorf("eglMakeCurrent error 0x%x", eglGetError()) - } - return nil - } - win, width, height, err := wdriver.EGLWindow(int(c.eglCtx.visualID)) - if err != nil { - return err - } - if c.eglWin == win && width == c.width && height == c.height { - return nil - } - c.width, c.height = width, height - if win == nilEGLNativeWindowType && c.srgbFBO != nil { - c.srgbFBO.Release() - c.srgbFBO = nil - } - c.destroySurface() - c.eglWin = win - if c.eglWin == nilEGLNativeWindowType { - return nil - } - eglSurf, err := createSurface(c.eglCtx, win) +func (c *Context) VisualID() int { + return c.eglCtx.visualID +} + +func (c *Context) CreateSurface(win NativeWindowType, width, height int) error { + eglSurf, err := createSurface(c.disp, c.eglCtx, win) c.eglSurf = eglSurf - if err != nil { - c.eglWin = nilEGLNativeWindowType - return err + c.width = width + c.height = height + return err +} + +func (c *Context) MakeCurrent() error { + if c.eglSurf == nilEGLSurface && !c.eglCtx.surfaceless { + return errors.New("no surface created yet EGL_KHR_surfaceless_context is not supported") } - if !eglMakeCurrent(c.eglCtx.disp, eglSurf, eglSurf, c.eglCtx.ctx) { + if !eglMakeCurrent(c.disp, c.eglSurf, c.eglSurf, c.eglCtx.ctx) { return fmt.Errorf("eglMakeCurrent error 0x%x", eglGetError()) } - // eglSwapInterval 1 leads to erratic frame rates and unnecessary blocking. - // We rely on platform specific frame rate limiting instead, except on Windows - // and X11 where eglSwapInterval is all there is. - if wdriver.NeedVSync() { - eglSwapInterval(c.eglCtx.disp, 1) - } else { - eglSwapInterval(c.eglCtx.disp, 0) - } if c.eglCtx.srgb { return nil } @@ -190,15 +147,18 @@ func (c *Context) MakeCurrent() error { var err error c.srgbFBO, err = gl.NewSRGBFBO(c.c) if err != nil { - c.Release() return err } } - if err := c.srgbFBO.Refresh(c.width, c.height); err != nil { - c.Release() - return err + return c.srgbFBO.Refresh(c.width, c.height) +} + +func (c *Context) EnableVSync(enable bool) { + if enable { + eglSwapInterval(c.disp, 1) + } else { + eglSwapInterval(c.disp, 0) } - return nil } func hasExtension(exts []string, ext string) bool { @@ -210,17 +170,13 @@ func hasExtension(exts []string, ext string) bool { return false } -func createContext(disp NativeDisplayType) (*eglContext, error) { - eglDisp := eglGetDisplay(disp) - if eglDisp == nilEGLDisplay { - return nil, fmt.Errorf("eglGetDisplay(_EGL_DEFAULT_DISPLAY) failed: 0x%x", eglGetError()) - } - major, minor, ret := eglInitialize(eglDisp) +func createContext(disp _EGLDisplay) (*eglContext, error) { + major, minor, ret := eglInitialize(disp) if !ret { return nil, fmt.Errorf("eglInitialize failed: 0x%x", eglGetError()) } // sRGB framebuffer support on EGL 1.5 or if EGL_KHR_gl_colorspace is supported. - exts := strings.Split(eglQueryString(eglDisp, _EGL_EXTENSIONS), " ") + exts := strings.Split(eglQueryString(disp, _EGL_EXTENSIONS), " ") srgb := major > 1 || minor >= 5 || hasExtension(exts, "EGL_KHR_gl_colorspace") attribs := []_EGLint{ _EGL_RENDERABLE_TYPE, _EGL_OPENGL_ES2_BIT, @@ -240,14 +196,14 @@ func createContext(disp NativeDisplayType) (*eglContext, error) { attribs = append(attribs, _EGL_DEPTH_SIZE, 16) } attribs = append(attribs, _EGL_NONE) - eglCfg, ret := eglChooseConfig(eglDisp, attribs) + eglCfg, ret := eglChooseConfig(disp, attribs) if !ret { return nil, fmt.Errorf("eglChooseConfig failed: 0x%x", eglGetError()) } if eglCfg == nilEGLConfig { return nil, errors.New("eglChooseConfig returned 0 configs") } - visID, ret := eglGetConfigAttrib(eglDisp, eglCfg, _EGL_NATIVE_VISUAL_ID) + visID, ret := eglGetConfigAttrib(disp, eglCfg, _EGL_NATIVE_VISUAL_ID) if !ret { return nil, errors.New("newContext: eglGetConfigAttrib for _EGL_NATIVE_VISUAL_ID failed") } @@ -255,20 +211,19 @@ func createContext(disp NativeDisplayType) (*eglContext, error) { _EGL_CONTEXT_CLIENT_VERSION, 3, _EGL_NONE, } - eglCtx := eglCreateContext(eglDisp, eglCfg, nilEGLContext, ctxAttribs) + eglCtx := eglCreateContext(disp, eglCfg, nilEGLContext, ctxAttribs) if eglCtx == nilEGLContext { // Fall back to OpenGL ES 2 and rely on extensions. ctxAttribs := []_EGLint{ _EGL_CONTEXT_CLIENT_VERSION, 2, _EGL_NONE, } - eglCtx = eglCreateContext(eglDisp, eglCfg, nilEGLContext, ctxAttribs) + eglCtx = eglCreateContext(disp, eglCfg, nilEGLContext, ctxAttribs) if eglCtx == nilEGLContext { return nil, fmt.Errorf("eglCreateContext failed: 0x%x", eglGetError()) } } return &eglContext{ - disp: eglDisp, config: _EGLConfig(eglCfg), ctx: _EGLContext(eglCtx), visualID: int(visID), @@ -277,18 +232,18 @@ func createContext(disp NativeDisplayType) (*eglContext, error) { }, nil } -func createSurface(eglCtx *eglContext, win NativeWindowType) (_EGLSurface, error) { +func createSurface(disp _EGLDisplay, eglCtx *eglContext, win NativeWindowType) (_EGLSurface, error) { var surfAttribs []_EGLint if eglCtx.srgb { surfAttribs = append(surfAttribs, _EGL_GL_COLORSPACE_KHR, _EGL_GL_COLORSPACE_SRGB_KHR) } surfAttribs = append(surfAttribs, _EGL_NONE) - eglSurf := eglCreateWindowSurface(eglCtx.disp, eglCtx.config, win, surfAttribs) + eglSurf := eglCreateWindowSurface(disp, eglCtx.config, win, surfAttribs) if eglSurf == nilEGLSurface && eglCtx.srgb { // Try again without sRGB eglCtx.srgb = false surfAttribs = []_EGLint{_EGL_NONE} - eglSurf = eglCreateWindowSurface(eglCtx.disp, eglCtx.config, win, surfAttribs) + eglSurf = eglCreateWindowSurface(disp, eglCtx.config, win, surfAttribs) } if eglSurf == nilEGLSurface { return nilEGLSurface, fmt.Errorf("newContext: eglCreateWindowSurface failed 0x%x (sRGB=%v)", eglGetError(), eglCtx.srgb) diff --git a/app/internal/window/egl_android.go b/app/internal/window/egl_android.go index 472ef94c..f73cbfde 100644 --- a/app/internal/window/egl_android.go +++ b/app/internal/window/egl_android.go @@ -14,20 +14,38 @@ import ( "gioui.org/app/internal/gl" ) -func (w *window) EGLDestroy() { -} - -func (w *window) EGLDisplay() egl.NativeDisplayType { - return nil -} - -func (w *window) EGLWindow(visID int) (egl.NativeWindowType, int, int, error) { - win, width, height := w.nativeWindow(visID) - return egl.NativeWindowType(unsafe.Pointer(win)), width, height, nil +type context struct { + win *window + *egl.Context } func (w *window) NewContext() (gl.Context, error) { - return egl.NewContext(w) + ctx, err := egl.NewContext(nil) + if err != nil { + return nil, err + } + return &context{win: w, Context: ctx}, nil } -func (w *window) NeedVSync() bool { return false } +func (c *context) Release() { + if c.Context != nil { + c.Context.Release() + c.Context = nil + } +} + +func (c *context) MakeCurrent() error { + c.Context.ReleaseSurface() + win, width, height := c.win.nativeWindow(c.Context.VisualID()) + if win == nil { + return nil + } + eglSurf := egl.NativeWindowType(unsafe.Pointer(win)) + if err := c.Context.CreateSurface(eglSurf, width, height); err != nil { + return err + } + if err := c.Context.MakeCurrent(); err != nil { + return err + } + return nil +} diff --git a/app/internal/window/egl_wayland.go b/app/internal/window/egl_wayland.go index 0d029c0d..0768a15c 100644 --- a/app/internal/window/egl_wayland.go +++ b/app/internal/window/egl_wayland.go @@ -6,7 +6,6 @@ package window import ( "errors" - "sync" "unsafe" "gioui.org/app/internal/egl" @@ -22,52 +21,50 @@ import ( */ import "C" -var eglWindows struct { - mu sync.Mutex - windows map[*C.struct_wl_surface]*C.struct_wl_egl_window -} - -func (w *window) EGLDestroy() { - surf, _, _ := w.surface() - if surf == nil { - return - } - eglWindows.mu.Lock() - defer eglWindows.mu.Unlock() - if eglWin, ok := eglWindows.windows[surf]; ok { - C.wl_egl_window_destroy(eglWin) - delete(eglWindows.windows, surf) - } -} - -func (w *window) EGLDisplay() egl.NativeDisplayType { - return egl.NativeDisplayType(unsafe.Pointer(w.display())) -} - -func (w *window) EGLWindow(visID int) (egl.NativeWindowType, int, int, error) { - surf, width, height := w.surface() - if surf == nil { - return 0, 0, 0, errors.New("wayland: no surface") - } - eglWindows.mu.Lock() - defer eglWindows.mu.Unlock() - eglWin, ok := eglWindows.windows[surf] - if !ok { - if eglWindows.windows == nil { - eglWindows.windows = make(map[*C.struct_wl_surface]*C.struct_wl_egl_window) - } - eglWin = C.wl_egl_window_create(surf, C.int(width), C.int(height)) - if eglWin == nil { - return 0, 0, 0, errors.New("wayland: wl_egl_window_create failed") - } - eglWindows.windows[surf] = eglWin - } - C.wl_egl_window_resize(eglWin, C.int(width), C.int(height), 0, 0) - return egl.NativeWindowType(uintptr(unsafe.Pointer(eglWin))), width, height, nil +type context struct { + win *window + *egl.Context + eglWin *C.struct_wl_egl_window } func (w *window) NewContext() (gl.Context, error) { - return egl.NewContext(w) + disp := egl.NativeDisplayType(unsafe.Pointer(w.display())) + ctx, err := egl.NewContext(disp) + if err != nil { + return nil, err + } + return &context{Context: ctx, win: w}, nil } -func (w *window) NeedVSync() bool { return false } +func (c *context) Release() { + if c.Context != nil { + c.Context.Release() + c.Context = nil + } + if c.eglWin != nil { + C.wl_egl_window_destroy(c.eglWin) + c.eglWin = nil + } +} + +func (c *context) MakeCurrent() error { + c.Context.ReleaseSurface() + if c.eglWin != nil { + C.wl_egl_window_destroy(c.eglWin) + c.eglWin = nil + } + surf, width, height := c.win.surface() + if surf == nil { + return errors.New("wayland: no surface") + } + eglWin := C.wl_egl_window_create(surf, C.int(width), C.int(height)) + if eglWin == nil { + return errors.New("wayland: wl_egl_window_create failed") + } + c.eglWin = eglWin + eglSurf := egl.NativeWindowType(uintptr(unsafe.Pointer(eglWin))) + if err := c.Context.CreateSurface(eglSurf, width, height); err != nil { + return err + } + return c.Context.MakeCurrent() +} diff --git a/app/internal/window/egl_windows.go b/app/internal/window/egl_windows.go index 2b1c3edd..d4ea94b4 100644 --- a/app/internal/window/egl_windows.go +++ b/app/internal/window/egl_windows.go @@ -3,24 +3,43 @@ package window import ( + "unsafe" + "gioui.org/app/internal/egl" "gioui.org/app/internal/gl" ) -func (w *window) EGLDestroy() { -} - -func (w *window) EGLDisplay() egl.NativeDisplayType { - return egl.NativeDisplayType(w.HDC()) -} - -func (w *window) EGLWindow(visID int) (egl.NativeWindowType, int, int, error) { - hwnd, width, height := w.HWND() - return egl.NativeWindowType(hwnd), width, height, nil +type context struct { + win *window + *egl.Context } func (w *window) NewContext() (gl.Context, error) { - return egl.NewContext(w) + disp := egl.NativeDisplayType(unsafe.Pointer(w.HDC())) + ctx, err := egl.NewContext(disp) + if err != nil { + return nil, err + } + return &context{win: w, Context: ctx}, nil } -func (w *window) NeedVSync() bool { return true } +func (c *context) Release() { + if c.Context != nil { + c.Context.Release() + c.Context = nil + } +} + +func (c *context) MakeCurrent() error { + c.Context.ReleaseSurface() + win, width, height := c.win.HWND() + eglSurf := egl.NativeWindowType(win) + if err := c.Context.CreateSurface(eglSurf, width, height); err != nil { + return err + } + if err := c.Context.MakeCurrent(); err != nil { + return err + } + c.Context.EnableVSync(true) + return nil +} diff --git a/app/internal/window/egl_x11.go b/app/internal/window/egl_x11.go index 3eaf4061..ac2360f8 100644 --- a/app/internal/window/egl_x11.go +++ b/app/internal/window/egl_x11.go @@ -5,23 +5,43 @@ package window import ( + "unsafe" + "gioui.org/app/internal/egl" "gioui.org/app/internal/gl" ) +type x11Context struct { + win *x11Window + *egl.Context +} + func (w *x11Window) NewContext() (gl.Context, error) { - return egl.NewContext(w) + disp := egl.NativeDisplayType(unsafe.Pointer(w.display())) + ctx, err := egl.NewContext(disp) + if err != nil { + return nil, err + } + return &x11Context{win: w, Context: ctx}, nil } -func (w *x11Window) EGLDestroy() { +func (c *x11Context) Release() { + if c.Context != nil { + c.Context.Release() + c.Context = nil + } } -func (w *x11Window) EGLDisplay() egl.NativeDisplayType { - return egl.NativeDisplayType(w.display()) +func (c *x11Context) MakeCurrent() error { + c.Context.ReleaseSurface() + win, width, height := c.win.window() + eglSurf := egl.NativeWindowType(uintptr(win)) + if err := c.Context.CreateSurface(eglSurf, width, height); err != nil { + return err + } + if err := c.Context.MakeCurrent(); err != nil { + return err + } + c.Context.EnableVSync(true) + return nil } - -func (w *x11Window) EGLWindow(visID int) (egl.NativeWindowType, int, int, error) { - return egl.NativeWindowType(uintptr(w.xw)), w.width, w.height, nil -} - -func (w *x11Window) NeedVSync() bool { return true } diff --git a/app/internal/window/os_wayland.go b/app/internal/window/os_wayland.go index f8eda11b..67460736 100644 --- a/app/internal/window/os_wayland.go +++ b/app/internal/window/os_wayland.go @@ -1052,8 +1052,6 @@ func (w *window) display() *C.struct_wl_display { } func (w *window) surface() (*C.struct_wl_surface, int, int) { - w.mu.Lock() - defer w.mu.Unlock() if w.needAck { C.xdg_surface_ack_configure(w.wmSurf, w.serial) w.needAck = false diff --git a/app/internal/window/os_x11.go b/app/internal/window/os_x11.go index 2498a34d..b70b100a 100644 --- a/app/internal/window/os_x11.go +++ b/app/internal/window/os_x11.go @@ -78,8 +78,12 @@ func (w *x11Window) wakeup() { } } -func (w *x11Window) display() unsafe.Pointer { - return unsafe.Pointer(w.x) +func (w *x11Window) display() *C.Display { + return w.x +} + +func (w *x11Window) window() (C.Window, int, int) { + return w.xw, w.width, w.height } func (w *x11Window) setStage(s system.Stage) {