Replace window size polling with callback

Instead of polling for window size changes on every render tick, use
the size-changed callback from GLFW to only make changes when it is
known that there's been a change in the size of the window.

This solves a concurrency problem that can happen when the scale factor
is changed. The sequence of events looks like:

1. The `screen.scalefactor[]` value is changed
2. The listeners to `scalefactor` start to be notified.
3. Asynchronously, the `WindowAreaUpdater` listener attached to the
   `render_tick` observable runs.
4. The window area listener notices that the window size given the
   _new_ value of the scale factor doesn't match the scene size, so
   it updates the scene size.
5. The `scalefactor` listener responsible for resizing the window
   gets its slice of time to run, and it now sees that everything is
   already the "correct" size.

Therefore, the window is not resized, and instead the scene size is
rescaled in the opposite direction of the scale factor change.

GLMakie/src/events.jl

@@ -36,52 +36,50 @@ function Makie.disconnect!(window::GLFW.Window, ::typeof(window_open))
     GLFW.SetWindowCloseCallback(window, nothing)
 end
 
-function window_position(window::GLFW.Window)
-    xy = GLFW.GetWindowPos(window)
-    (xy.x, xy.y)
-end
+function Makie.window_area(scene::Scene, screen::Screen)
+    disconnect!(screen, window_area)
 
-struct WindowAreaUpdater
-    screen::Screen
-    dpi::Observable{Float64}
-    area::Observable{GeometryBasics.HyperRectangle{2, Int64}}
-end
+    # TODO: Figure out which monitor the window is on and react to DPI changes
+    monitor = GLFW.GetPrimaryMonitor()
+    props = MonitorProperties(monitor)
+    scene.events.window_dpi[] = minimum(props.dpi)
 
-function (x::WindowAreaUpdater)(::Nothing)
-    nw = to_native(x.screen)
-    ShaderAbstractions.switch_context!(nw)
-    rect = x.area[]
-    winscale = x.screen.scalefactor[] / (@static Sys.isapple() ? scale_factor(nw) : 1)
-    winw, winh = round.(Int, window_size(nw) ./ winscale)
-    # TODO put back window position, but right now it makes more trouble than it helps#
-    # x, y = GLFW.GetWindowPos(nw)
-    # if minimum(rect) != Vec(x, y)
-    #     event[] = Recti(x, y, winw, winh)
-    # end
-    if Vec(winw, winh) != widths(rect)
-        monitor = GLFW.GetPrimaryMonitor()
-        props = MonitorProperties(monitor)
-        # dpi of a monitor should be the same in x y direction.
-        # if not, minimum seems to be a fair default
-        x.dpi[] = minimum(props.dpi)
-        x.area[] = Recti(minimum(rect), winw, winh)
-    end
-    return
-end
+    function windowsizecb(window, width::Cint, height::Cint)
+        area = scene.events.window_area
+        sf = screen.scalefactor[]
 
-function Makie.window_area(scene::Scene, screen::Screen)
-    disconnect!(screen, window_area)
+        ShaderAbstractions.switch_context!(window)
+        winscale = sf / (@static Sys.isapple() ? scale_factor(window) : 1)
+        winw, winh = round.(Int, (width, height) ./ winscale)
+        if Vec(winw, winh) != widths(area[])
+            area[] = Recti(minimum(area[]), winw, winh)
+        end
+        return
+    end
+    # TODO put back window position, but right now it makes more trouble than it helps
+    #function windowposcb(window, x::Cint, y::Cint)
+    #    area = scene.events.window_area
+    #    ShaderAbstractions.switch_context!(window)
+    #    winscale = screen.scalefactor[] / (@static Sys.isapple() ? scale_factor(window) : 1)
+    #    xs, ys = round.(Int, (x, y) ./ winscale)
+    #    if Vec(xs, ys) != minimum(area[])
+    #        area[] = Recti(xs, ys, widths(area[]))
+    #    end
+    #    return
+    #end
 
-    updater = WindowAreaUpdater(
-        screen, scene.events.window_dpi, scene.events.window_area
-    )
-    on(updater, screen.render_tick)
+    window = to_native(screen)
+    GLFW.SetWindowSizeCallback(window, (win, w, h) -> windowsizecb(win, w, h))
+    #GLFW.SetWindowPosCallback(window, (win, x, y) -> windowposcb(win, x, y))
 
+    windowsizecb(window, Cint.(window_size(window))...)
     return
 end
 
 function Makie.disconnect!(screen::Screen, ::typeof(window_area))
-    filter!(p -> !isa(p[2], WindowAreaUpdater), screen.render_tick.listeners)
+    window = to_native(screen)
+    #GLFW.SetWindowPosCallback(window, nothing)
+    GLFW.SetWindowSizeCallback(window, nothing)
     return
 end
 function Makie.disconnect!(::GLFW.Window, ::typeof(window_area))

GLMakie/src/glwindow.jl

@@ -190,6 +190,10 @@ function window_size(nw::GLFW.Window)
     was_destroyed(nw) && return (0, 0)
     return Tuple(GLFW.GetWindowSize(nw))
 end
+function window_position(nw::GLFW.Window)
+    was_destroyed(nw) && return (0, 0)
+    return Tuple(GLFW.GetWindowPos(window))
+end
 function framebuffer_size(nw::GLFW.Window)
     was_destroyed(nw) && return (0, 0)
     return Tuple(GLFW.GetFramebufferSize(nw))

GLMakie/test/unit_tests.jl

@@ -305,7 +305,6 @@ end
 
     # decrease the scale factor after-the-fact
     screen.scalefactor[] = 1
-    sleep(0.1)  # TODO: Necessary?? Are observable callbacks asynchronous?
     @test GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
 
     # save images of different resolutions
@@ -327,6 +326,16 @@ end
         @test_broken screen.px_per_unit[] == 1
     end
 
+    # make sure there isn't a race between changing the scale factor and window_area updater
+    # see https://github.com/MakieOrg/Makie.jl/pull/2544#issuecomment-1416861800
+    screen = display(GLMakie.Screen(visible = false, scalefactor = 2, framerate = 60), fig)
+    @test GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
+    on(screen.scalefactor) do sf
+        sleep(0.5)
+    end
+    screen.scalefactor[] = 1
+    @test GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
+
     if Sys.islinux()
         # Test that GLMakie is correctly getting the default scale factor from X11 in a
         # HiDPI environment.
@@ -372,4 +381,6 @@ end
     else
         @test_broken Sys.islinux()
     end
+
+    GLMakie.closeall()
 end