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

Move Linux OpenGL renderer to another thread. #351

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

igordmn
Copy link
Collaborator

@igordmn igordmn commented Nov 3, 2021

To avoid Swing thread to be wasted.

It should reduce input lag from input events.

To avoid Swing thread to be wasted.

It should reduce input lag from input events.
@igordmn igordmn requested review from olonho and Rsedaikin November 3, 2021 16:54
if (!isVideoCardSupported(layer.renderApi)) {
throw RenderException("Cannot create Linux GL context")
}
it.setSwapIntervalFast(defaultSwapInterval)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems here setSwapInterval shall be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setSwapIntervalFast implemented in a way, that it has uninitialized value -1, so setSwapIntervalFast will work, and it is more consistent with the other code than simple setSwapInterval

@@ -95,6 +125,12 @@ internal class LinuxOpenGLRedrawer(
.filterNot(LinuxOpenGLRedrawer::isDisposed)
.filter { it.layer.isShowing }

private val drawDispatcher = Executors.newSingleThreadExecutor {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So all drawing happens in this thread, no matter how many open windows we got?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe write comment, and also on how we ensure that no races on drawing context happen?

private fun draw() {
layer.inDrawScope(layer::draw)
private suspend fun draw(withVsync: Boolean) {
layer.inDrawScope {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shalln't we swap the order and do

inDrawThread {
   layer.inDrawScope {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, inDrawScope should be called in Swing thread, as it contain fallback logic

it.swapBuffers()
OpenGLApi.instance.glFinish()
it.setSwapInterval(swapInterval)
runBlocking {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we'll block AWT thread here? Shall we?

Copy link
Collaborator Author

@igordmn igordmn Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the semantic of this method is to draw the content in a blocking way, but without vsync synchronization

@olonho
Copy link
Contributor

olonho commented Nov 4, 2021

With this patch VirtualBox with OpenGL hardware acceleration hard locks the VM when running JVM sample.

@igordmn
Copy link
Collaborator Author

igordmn commented Nov 4, 2021

With this patch VirtualBox with OpenGL hardware acceleration hard locks the VM when running JVM sample.

I was afraid of it, but didn't reproduce on the real machine :(.

It seems it is because of fallback.

Will investigate, and if it is not simple, do it after Compose 1.0

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.

2 participants