Unverified Commit 340cb069 authored by Igor Demin's avatar Igor Demin Committed by GitHub

Fixes for multiple windows on Linux: proper dispose and vsync (#241)

* Fix a short memory leak in Windows/Linux OpenGL redrawers

* Call makeCurrent in LinuxOpenGLRedrawer

* Fix vsync on multiple monitors on Linux

* Apply vsync only if it is enabled in properties

* Fix WindowsOpenGLRedrawer.dispose
parent 2a0a1044
...@@ -48,6 +48,9 @@ internal class LinuxOpenGLRedrawer( ...@@ -48,6 +48,9 @@ internal class LinuxOpenGLRedrawer(
override fun dispose() { override fun dispose() {
check(!isDisposed) { "LinuxOpenGLRedrawer is disposed" } check(!isDisposed) { "LinuxOpenGLRedrawer is disposed" }
layer.backedLayer.lockLinuxDrawingSurface { layer.backedLayer.lockLinuxDrawingSurface {
// makeCurrent is mandatory to destroy context, otherwise, OpenGL will destroy wrong context (from another window).
// see the official example: https://www.khronos.org/opengl/wiki/Tutorial:_OpenGL_3.0_Context_Creation_(GLX)
it.makeCurrent(context)
it.destroyContext(context) it.destroyContext(context)
} }
runBlocking { runBlocking {
...@@ -90,13 +93,12 @@ internal class LinuxOpenGLRedrawer( ...@@ -90,13 +93,12 @@ internal class LinuxOpenGLRedrawer(
.filter { it.layer.isShowing } .filter { it.layer.isShowing }
private val frameDispatcher = FrameDispatcher(Dispatchers.Swing) { private val frameDispatcher = FrameDispatcher(Dispatchers.Swing) {
// we should wait for the window with the maximum frame limit to avoid bottleneck when there is a window on a slower monitor
toRedrawVisible.maxByOrNull { it.frameLimit }?.limitFramesIfNeeded()
toRedrawCopy.clear()
toRedrawCopy.addAll(toRedraw) toRedrawCopy.addAll(toRedraw)
toRedraw.clear() toRedraw.clear()
// we should wait for the window with the maximum frame limit to avoid bottleneck when there is a window on a slower monitor
toRedrawVisible.maxByOrNull { it.frameLimit }?.limitFramesIfNeeded()
val nanoTime = System.nanoTime() val nanoTime = System.nanoTime()
for (redrawer in toRedrawVisible) { for (redrawer in toRedrawVisible) {
...@@ -109,25 +111,41 @@ internal class LinuxOpenGLRedrawer( ...@@ -109,25 +111,41 @@ internal class LinuxOpenGLRedrawer(
val drawingSurfaces = toRedrawVisible.associateWith { lockLinuxDrawingSurface(it.layer.backedLayer) } val drawingSurfaces = toRedrawVisible.associateWith { lockLinuxDrawingSurface(it.layer.backedLayer) }
try { try {
toRedrawVisible.forEach { redrawer -> for (redrawer in toRedrawVisible) {
drawingSurfaces[redrawer]!!.makeCurrent(redrawer.context) drawingSurfaces[redrawer]!!.makeCurrent(redrawer.context)
redrawer.draw() redrawer.draw()
} }
// TODO(demin) it seems now vsync doesn't work as expected with two windows (we have fps = refreshRate / windowCount) // TODO(demin): How can we properly synchronize multiple windows with multiple displays?
// perhaps we should create frameDispatcher for each display. // I checked, and without vsync there is no tearing. Is it only my case (Ubuntu, Nvidia, X11),
// Don't know what happened, but on 620547a commit everything was okay. maybe something changed in the code, maybe my system changed // or Ubuntu write all the screen content into an intermediate buffer? If so, then we probably only
toRedrawVisible.forEach { redrawer -> // need a frame limiter.
// Synchronize with vsync only for the fastest monitor, for the single window.
// Otherwise, 5 windows will wait for vsync 5 times.
val vsyncRedrawer = toRedrawVisible
.filter { it.properties.isVsyncEnabled }
.maxByOrNull { it.frameLimit }
for (redrawer in toRedrawVisible.filter { it != vsyncRedrawer }) {
drawingSurfaces[redrawer]!!.makeCurrent(redrawer.context)
drawingSurfaces[redrawer]!!.setSwapInterval(0)
drawingSurfaces[redrawer]!!.swapBuffers() drawingSurfaces[redrawer]!!.swapBuffers()
OpenGLApi.instance.glFinish()
} }
toRedrawVisible.forEach { redrawer -> if (vsyncRedrawer != null) {
drawingSurfaces[redrawer]!!.makeCurrent(redrawer.context) drawingSurfaces[vsyncRedrawer]!!.makeCurrent(vsyncRedrawer.context)
drawingSurfaces[vsyncRedrawer]!!.setSwapInterval(1)
drawingSurfaces[vsyncRedrawer]!!.swapBuffers()
OpenGLApi.instance.glFinish() OpenGLApi.instance.glFinish()
} }
} finally { } finally {
drawingSurfaces.values.forEach(::unlockLinuxDrawingSurface) drawingSurfaces.values.forEach(::unlockLinuxDrawingSurface)
} }
// Without clearing we will have a memory leak
toRedrawCopy.clear()
} }
} }
} }
......
...@@ -33,6 +33,7 @@ internal class WindowsOpenGLRedrawer( ...@@ -33,6 +33,7 @@ internal class WindowsOpenGLRedrawer(
override fun dispose() { override fun dispose() {
check(!isDisposed) { "WindowsOpenGLRedrawer is disposed" } check(!isDisposed) { "WindowsOpenGLRedrawer is disposed" }
makeCurrent()
deleteContext(context) deleteContext(context)
isDisposed = true isDisposed = true
} }
...@@ -72,7 +73,6 @@ internal class WindowsOpenGLRedrawer( ...@@ -72,7 +73,6 @@ internal class WindowsOpenGLRedrawer(
.filter { it.layer.isShowing } .filter { it.layer.isShowing }
private val frameDispatcher = FrameDispatcher(Dispatchers.Swing) { private val frameDispatcher = FrameDispatcher(Dispatchers.Swing) {
toRedrawCopy.clear()
toRedrawCopy.addAll(toRedraw) toRedrawCopy.addAll(toRedraw)
toRedraw.clear() toRedraw.clear()
...@@ -106,6 +106,9 @@ internal class WindowsOpenGLRedrawer( ...@@ -106,6 +106,9 @@ internal class WindowsOpenGLRedrawer(
dwmFlush() // wait for vsync dwmFlush() // wait for vsync
} }
} }
// Without clearing we will have a memory leak
toRedrawCopy.clear()
} }
} }
} }
......
...@@ -56,7 +56,7 @@ native crash in SkiaWindowTest "render single window" ...@@ -56,7 +56,7 @@ native crash in SkiaWindowTest "render single window"
private val expectedDeviatePercent3 = 0.30 private val expectedDeviatePercent3 = 0.30
private val expectedDeviatePercentTerminal = 0.50 private val expectedDeviatePercentTerminal = 0.50
private val expectedFrameNanos = 1E9 / graphicsConfiguration.device.displayMode.refreshRate private val expectedFrameNanos get() = 1E9 / layer.backedLayer.getDisplayRefreshRate()
private val frameTimes = mutableListOf<Long>() private val frameTimes = mutableListOf<Long>()
private var canCollect = false private var canCollect = false
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment