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

Let the user specify which monitor OPENRNDR uses #303

Merged
merged 16 commits into from Jun 19, 2022

Conversation

Vechro
Copy link
Contributor

@Vechro Vechro commented May 15, 2022

This is a bit trickier than I first assumed. It's a complicated feature to test so I will initially be publishing this PR as a draft until I'm more confident it can handle all the edge cases.
Fixes #113. Fixes #304.

@Vechro
Copy link
Contributor Author

Vechro commented May 19, 2022

Would love to hear how it fares on macOS and Linux distributions. I've only been able to test this on Windows.

@hamoid
Copy link
Member

hamoid commented May 20, 2022

How and what should we test? I did

git fetch upstream pull/303/head:monitors
git checkout monitors
./gradlew publishToMavenLocal -Prelease.version=0.5.1-SNAPSHOT

Then tried this program

import org.openrndr.application

fun main() = application {
    configure {
        monitor = 0
    }
    program {
        extend {
            drawer.circle(drawer.bounds.center, 200.0)
        }
    }
}

monitor 0 and 1 display the program on the same window as the IDE, with 2 it fails to run.

What can I try next? Maybe you can add debug messages to the code and I see what comes out on Linux?

@hamoid
Copy link
Member

hamoid commented May 20, 2022

I wonder if it's because I'm using a tiling window manager (i3wm). I'll try with the default.

@hamoid
Copy link
Member

hamoid commented May 20, 2022

That was it. With the default Ubuntu wm it works as expected and I can choose the monitor with 0 or 1, both normal and full screen.

So with i3wm, I don't know if there's an issue in glfw or in this code. It's not the first time that something in OPENRNDR behaves different with the tiling window manager I use. There are 6 open issues in their repo, one dealing with multi monitor setup.

@Vechro
Copy link
Contributor Author

Vechro commented May 20, 2022

That's interesting, and yes I should have specified what to test. I've mainly been looking

  1. Whether monitor sets the window on the specified monitor.
  2. Does fullscreen work as expected?
  3. Does specifying the window position work as expected?
  4. If the window position is not specified, does it properly center the window on the specified monitor?

@hamoid
Copy link
Member

hamoid commented May 20, 2022

I didn't try 3, the others did work in the Ubuntu wm. I'll try 3 this evening (too lazy to close all apps, log out and log in again, then reverse).

Meanwhile I was trying this

        run {
            println("monitors!!!")
            val a = glfwGetMonitors()?.get(0)!!
            val b = glfwGetMonitors()?.get(1)!!
            val x = FloatArray(1)
            val y = FloatArray(1)
            glfwGetMonitorContentScale(a, x, y)
            println("MON 0: ${x[0]} ${y[0]}")
            glfwGetMonitorContentScale(b, x, y)
            println("MON 1: ${x[0]} ${y[0]}")

            val mode0 = glfwGetVideoMode(a)!!
            println("MON 0: ${mode0.width()} x ${mode0.height()}")
            val mode1 = glfwGetVideoMode(a)!!
            println("MON 1: ${mode1.width()} x ${mode1.height()}")
        }

which prints the expected values even if it doesn't work as expected in i3wm. At least it does get the right screen resolutions.

monitors!!!
MON 0: 1.0 1.0
MON 1: 1.0 1.0
MON 0: 1920 x 1200
MON 1: 1680 x 1050

@hamoid
Copy link
Member

hamoid commented May 20, 2022

From GLFW.java:2026
image

Can this affect in my case? is it an "initially hidden window"?

@hamoid
Copy link
Member

hamoid commented May 20, 2022

Trying to figure out what OPENRNDR knows about my window position...

    program {
        extend {
            drawer.circle(drawer.bounds.center, 200.0)
        }
        window.moved.listen {
            println(it.position)
        }
        window.focused.listen {
            println("focused")
        }
        window.unfocused.listen {
            println("unfocused")
        }
    }

I see that focused and unfocused events are triggered, but not moved. Anyway, I'm used to things working different in my system. At least it works fine with the common wm.

@Vechro
Copy link
Contributor Author

Vechro commented May 20, 2022

From GLFW.java:2026 image

Can this affect in my case? is it an "initially hidden window"?

That could very well be the case, you could try changing this window hint and see if that fixes it. I wouldn't change it in the PR because I feel like it would introduce some visual glitches for everyone but it could verify the cause of the problem.

@hamoid
Copy link
Member

hamoid commented May 26, 2022

Leaving this note here so it can be found together with the above:

Changing glfwWindowHint(GLFW_VISIBLE, GLFW_FALSE) to GLFW_TRUE makes the monitor selection work in i3. The issue then might be how to customize a setting only in this case? The env var DESKTOP_SESSION contains i3 in my case.

But instead of being smart and trying to detect window managers, maybe it's safer to allow to specify in some way tiling window manager present? Because there's not only i3, there are others. And monitor selection is not the only thing affected by using a tiling wm. At the same time, users choosing a tiling wm know they are outside well known explored territory, so I don't expect big changes to make things work just for me. If at least we were two users with this kind of issue... XD

@Vechro Vechro marked this pull request as ready for review June 9, 2022 13:38
Comment on lines 85 to 93
return when (applicationProperty) {
null, "", "ApplicationGLFW" -> Application::class.java.classLoader.loadClass("org.openrndr.internal.gl3.ApplicationGLFWGL3")
"ApplicationEGL" -> Application::class.java.classLoader.loadClass("org.openrndr.internal.gl3.ApplicationEGLGL3")
else -> throw IllegalArgumentException("Unknown value '${applicationProperty}' provided for org.openrndr.application")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are these names intuitive for the user? Should we instead simply use "GLFW" and "EGL"?

Vechro added 14 commits June 14, 2022 22:56
This was accidentally omitted from the previous commit
Switches stackPush().let {} out for stackPush.use {}
Introduce `org.openrndr.application` VM property to specify graphics backend.
Clarify synchronous/asynchronous application error messages.
Stop initializing Application twice in certain conditions.
Otherwise there's nothing preventing the user from calling them.
Due to it not respecting window placement otherwise.
This is an alternative to the two-stage initialization in Application implementations which made things far too fragile.
@edwinRNDR edwinRNDR merged commit 9c75e59 into openrndr:master Jun 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants