From 3b5110a1329c0ad81db4f435bb6b18658226723c Mon Sep 17 00:00:00 2001
From: Justin Willmert <justin@willmert.me>
Date: Fri, 30 Dec 2022 08:49:35 -0600
Subject: [PATCH 1/3] Make GLMakie aware of the content scale factor on HiDPI
 screens

This is accomplished by making the window and rendered framebuffer
sizes independent of one another. By separating the two concepts, we
can create rasterized graphics at higher (or lower) resolution than
what is shown on screen.

The window scale factor is used to scale the sizes of elements within
the window (and on Linux and Windows, the window size itself from the
requested logical sizes), and the px-per-unit scale factor dictates
the size of the rasterized render.

The default scaling factors are initialized by requesting the content
scale factor from GLFW (and dynamically responding to changes in the
content scale factor on platforms where GLFW provides support).

Testing is added on Linux where xvfb-run can be used to force a
HiDPI context.
---
 .github/workflows/glmakie.yaml            |   2 +-
 GLMakie/Project.toml                      |   2 +-
 GLMakie/assets/shader/distance_shape.frag |  12 ++-
 GLMakie/assets/shader/sprites.geom        |   3 +-
 GLMakie/src/drawing_primitives.jl         |   1 +
 GLMakie/src/events.jl                     |  56 ++++-------
 GLMakie/src/glwindow.jl                   |  27 +++---
 GLMakie/src/picking.jl                    |  16 +--
 GLMakie/src/postprocessing.jl             |   5 +-
 GLMakie/src/rendering.jl                  |  20 ++--
 GLMakie/src/screen.jl                     |  90 +++++++++++------
 GLMakie/test/runtests.jl                  |   2 +-
 GLMakie/test/unit_tests.jl                | 113 ++++++++++++++++++++++
 NEWS.md                                   |   3 +
 docs/documentation/backends/glmakie.md    |  44 +++++++++
 src/theming.jl                            |   2 +
 16 files changed, 293 insertions(+), 105 deletions(-)

diff --git a/.github/workflows/glmakie.yaml b/.github/workflows/glmakie.yaml
index 90114e784eb..c494ba57d05 100644
--- a/.github/workflows/glmakie.yaml
+++ b/.github/workflows/glmakie.yaml
@@ -44,7 +44,7 @@ jobs:
           version: ${{ matrix.version }}
           arch: ${{ matrix.arch }}
       - uses: julia-actions/cache@v1
-      - run: sudo apt-get update && sudo apt-get install -y xorg-dev mesa-utils xvfb libgl1 freeglut3-dev libxrandr-dev libxinerama-dev libxcursor-dev libxi-dev libxext-dev
+      - run: sudo apt-get update && sudo apt-get install -y xorg-dev mesa-utils xvfb libgl1 freeglut3-dev libxrandr-dev libxinerama-dev libxcursor-dev libxi-dev libxext-dev xsettingsd x11-xserver-utils
       - name: Install Julia dependencies
         shell: julia --project=monorepo {0}
         run: |
diff --git a/GLMakie/Project.toml b/GLMakie/Project.toml
index dd4074be8cb..185f43f9eac 100644
--- a/GLMakie/Project.toml
+++ b/GLMakie/Project.toml
@@ -27,7 +27,7 @@ Colors = "0.11, 0.12"
 FileIO = "1.6"
 FixedPointNumbers = "0.7, 0.8"
 FreeTypeAbstraction = "0.10"
-GLFW = "3"
+GLFW = "3.3"
 GeometryBasics = "0.4.1"
 Makie = "=0.19.2"
 MeshIO = "0.4"
diff --git a/GLMakie/assets/shader/distance_shape.frag b/GLMakie/assets/shader/distance_shape.frag
index df3e66e064c..18c750e5a41 100644
--- a/GLMakie/assets/shader/distance_shape.frag
+++ b/GLMakie/assets/shader/distance_shape.frag
@@ -25,6 +25,7 @@ uniform float           stroke_width;
 uniform float           glow_width;
 uniform int             shape; // shape is a uniform for now. Making them a in && using them for control flow is expected to kill performance
 uniform vec2            resolution;
+uniform float           px_per_unit;
 uniform bool            transparent_picking;
 
 flat in float           f_viewport_from_u_scale;
@@ -97,7 +98,9 @@ void stroke(vec4 strokecolor, float signed_distance, float width, inout vec4 col
 
 void glow(vec4 glowcolor, float signed_distance, float inside, inout vec4 color){
     if (glow_width > 0.0){
-        float outside = (abs(signed_distance)-stroke_width)/glow_width;
+        float s_stroke_width = px_per_unit * stroke_width;
+        float s_glow_width = px_per_unit * glow_width;
+        float outside = (abs(signed_distance)-s_stroke_width)/s_glow_width;
         float alpha = 1-outside;
         color = mix(vec4(glowcolor.rgb, glowcolor.a*alpha), color, inside);
     }
@@ -145,13 +148,14 @@ void main(){
     // See notes in geometry shader where f_viewport_from_u_scale is computed.
     signed_distance *= f_viewport_from_u_scale;
 
-    float inside_start = max(-stroke_width, 0.0);
+    float s_stroke_width = px_per_unit * stroke_width;
+    float inside_start = max(-s_stroke_width, 0.0);
     float inside = aastep(inside_start, signed_distance);
     vec4 final_color = f_bg_color;
 
     fill(f_color, image, tex_uv, inside, final_color);
-    stroke(f_stroke_color, signed_distance, -stroke_width, final_color);
-    glow(f_glow_color, signed_distance, aastep(-stroke_width, signed_distance), final_color);
+    stroke(f_stroke_color, signed_distance, -s_stroke_width, final_color);
+    glow(f_glow_color, signed_distance, aastep(-s_stroke_width, signed_distance), final_color);
     // TODO: In 3D, we should arguably discard fragments outside the sprite
     //       But note that this may interfere with object picking.
     // if (final_color == f_bg_color)
diff --git a/GLMakie/assets/shader/sprites.geom b/GLMakie/assets/shader/sprites.geom
index 850b700bd20..cae60a31faf 100644
--- a/GLMakie/assets/shader/sprites.geom
+++ b/GLMakie/assets/shader/sprites.geom
@@ -38,6 +38,7 @@ uniform float stroke_width;
 uniform float glow_width;
 uniform int shape; // for RECTANGLE hack below
 uniform vec2 resolution;
+uniform float px_per_unit;
 uniform float depth_shift;
 
 in int g_primitive_index[];
@@ -138,7 +139,7 @@ void main(void)
                              0.0,         1.0/vclip.w, 0.0,         0.0,
                              0.0,         0.0,         1.0/vclip.w, 0.0,
                              -vclip.xyz/(vclip.w*vclip.w),          0.0);
-    mat2 dxyv_dxys = diagm(0.5*resolution) * mat2(d_ndc_d_clip*trans);
+    mat2 dxyv_dxys = diagm(0.5*px_per_unit*resolution) * mat2(d_ndc_d_clip*trans);
     // Now, our buffer size is expressed in viewport pixels but we get back to
     // the sprite coordinate system using the scale factor of the
     // transformation (for isotropic transformations). For anisotropic
diff --git a/GLMakie/src/drawing_primitives.jl b/GLMakie/src/drawing_primitives.jl
index 56727ae6ebd..8650111570a 100644
--- a/GLMakie/src/drawing_primitives.jl
+++ b/GLMakie/src/drawing_primitives.jl
@@ -107,6 +107,7 @@ function cached_robj!(robj_func, screen, scene, x::AbstractPlot)
             gl_attributes[:ambient] = ambientlight.color
         end
         gl_attributes[:track_updates] = screen.config.render_on_demand
+        gl_attributes[:px_per_unit] = screen.px_per_unit
 
         robj = robj_func(gl_attributes)
 
diff --git a/GLMakie/src/events.jl b/GLMakie/src/events.jl
index fced12ae9af..8f6231d8e31 100644
--- a/GLMakie/src/events.jl
+++ b/GLMakie/src/events.jl
@@ -42,20 +42,21 @@ function window_position(window::GLFW.Window)
 end
 
 struct WindowAreaUpdater
-    window::GLFW.Window
+    screen::Screen
     dpi::Observable{Float64}
     area::Observable{GeometryBasics.HyperRectangle{2, Int64}}
 end
 
 function (x::WindowAreaUpdater)(::Nothing)
-    ShaderAbstractions.switch_context!(x.window)
+    nw = to_native(x.screen)
+    ShaderAbstractions.switch_context!(nw)
     rect = x.area[]
     # TODO put back window position, but right now it makes more trouble than it helps#
-    # x, y = GLFW.GetWindowPos(window)
+    # x, y = GLFW.GetWindowPos(nw)
     # if minimum(rect) != Vec(x, y)
-    #     event[] = Recti(x, y, framebuffer_size(window))
+    #     event[] = Recti(x, y, framebuffer_size((window))
     # end
-    w, h = GLFW.GetFramebufferSize(x.window)
+    w, h = round.(Int, framebuffer_size(nw) ./ x.screen.scalefactor[])
     if Vec(w, h) != widths(rect)
         monitor = GLFW.GetPrimaryMonitor()
         props = MonitorProperties(monitor)
@@ -71,7 +72,7 @@ function Makie.window_area(scene::Scene, screen::Screen)
     disconnect!(screen, window_area)
 
     updater = WindowAreaUpdater(
-        to_native(screen), scene.events.window_dpi, scene.events.window_area
+        screen, scene.events.window_dpi, scene.events.window_area
     )
     on(updater, screen.render_tick)
 
@@ -167,44 +168,27 @@ function Makie.disconnect!(window::GLFW.Window, ::typeof(unicode_input))
     GLFW.SetCharCallback(window, nothing)
 end
 
-# TODO memoise? Or to bug ridden for the small performance gain?
-function retina_scaling_factor(w, fb)
-    (w[1] == 0 || w[2] == 0) && return (1.0, 1.0)
-    fb ./ w
-end
-
-# TODO both of these methods are slow!
-# ~90µs, ~80µs
-# This is too slow for events that may happen 100x per frame
-function framebuffer_size(window::GLFW.Window)
-    wh = GLFW.GetFramebufferSize(window)
-    (wh.width, wh.height)
-end
-function window_size(window::GLFW.Window)
-    wh = GLFW.GetWindowSize(window)
-    (wh.width, wh.height)
-end
-function retina_scaling_factor(window::GLFW.Window)
-    w, fb = window_size(window), framebuffer_size(window)
-    retina_scaling_factor(w, fb)
-end
-
-function correct_mouse(window::GLFW.Window, w, h)
-    ws, fb = window_size(window), framebuffer_size(window)
-    s = retina_scaling_factor(ws, fb)
-    (w * s[1], fb[2] - (h * s[2]))
+function correct_mouse(screen::Screen, w, h)
+    sf = screen.scalefactor[]
+    _, winh = framebuffer_size(to_native(screen))
+    @static if Sys.isapple()
+        return w, (winh / sf) - h
+    else
+        return w / sf, (winh - h) / sf
+    end
 end
 
 struct MousePositionUpdater
-    window::GLFW.Window
+    screen::Screen
     mouseposition::Observable{Tuple{Float64, Float64}}
     hasfocus::Observable{Bool}
 end
 
 function (p::MousePositionUpdater)(::Nothing)
     !p.hasfocus[] && return
-    x, y = GLFW.GetCursorPos(p.window)
-    pos = correct_mouse(p.window, x, y)
+    nw = to_native(p.screen)
+    x, y = GLFW.GetCursorPos(nw)
+    pos = correct_mouse(p.screen, x, y)
     if pos != p.mouseposition[]
         @print_error p.mouseposition[] = pos
         # notify!(e.mouseposition)
@@ -221,7 +205,7 @@ which is not in scene coordinates, with the upper left window corner being 0
 function Makie.mouse_position(scene::Scene, screen::Screen)
     disconnect!(screen, mouse_position)
     updater = MousePositionUpdater(
-        to_native(screen), scene.events.mouseposition, scene.events.hasfocus
+        screen, scene.events.mouseposition, scene.events.hasfocus
     )
     on(updater, screen.render_tick)
     return
diff --git a/GLMakie/src/glwindow.jl b/GLMakie/src/glwindow.jl
index d643f609c5b..65e62bdb5c5 100644
--- a/GLMakie/src/glwindow.jl
+++ b/GLMakie/src/glwindow.jl
@@ -124,15 +124,13 @@ function GLFramebuffer(fb_size::NTuple{2, Int})
     )
 end
 
-function Base.resize!(fb::GLFramebuffer, window_size)
-    ws = Int.((window_size[1], window_size[2]))
-    if ws != size(fb) && all(x-> x > 0, window_size)
-        for (name, buffer) in fb.buffers
-            resize_nocopy!(buffer, ws)
-        end
-        fb.resolution[] = ws
+function Base.resize!(fb::GLFramebuffer, w::Int, h::Int)
+    (w > 0 && h > 0 && (w, h) != size(fb)) || return
+    for (name, buffer) in fb.buffers
+        resize_nocopy!(buffer, (w, h))
     end
-    nothing
+    fb.resolution[] = (w, h)
+    return nothing
 end
 
 
@@ -188,10 +186,17 @@ function destroy!(nw::GLFW.Window)
     was_current && ShaderAbstractions.switch_context!()
 end
 
-function windowsize(nw::GLFW.Window)
+function window_size(nw::GLFW.Window)
+    was_destroyed(nw) && return (0, 0)
+    return Tuple(GLFW.GetWindowSize(nw))
+end
+function framebuffer_size(nw::GLFW.Window)
     was_destroyed(nw) && return (0, 0)
-    size = GLFW.GetFramebufferSize(nw)
-    return (size.width, size.height)
+    return Tuple(GLFW.GetFramebufferSize(nw))
+end
+function scale_factor(nw::GLFW.Window)
+    was_destroyed(nw) && return 1f0
+    return minimum(GLFW.GetWindowContentScale(nw))
 end
 
 function Base.isopen(window::GLFW.Window)
diff --git a/GLMakie/src/picking.jl b/GLMakie/src/picking.jl
index d83c4d6f9d2..0a7b74346e7 100644
--- a/GLMakie/src/picking.jl
+++ b/GLMakie/src/picking.jl
@@ -6,16 +6,16 @@
 function pick_native(screen::Screen, rect::Rect2i)
     isopen(screen) || return Matrix{SelectionID{Int}}(undef, 0, 0)
     ShaderAbstractions.switch_context!(screen.glscreen)
-    window_size = size(screen)
     fb = screen.framebuffer
     buff = fb.buffers[:objectid]
     glBindFramebuffer(GL_FRAMEBUFFER, fb.id[1])
     glReadBuffer(GL_COLOR_ATTACHMENT1)
     rx, ry = minimum(rect)
     rw, rh = widths(rect)
-    w, h = window_size
-    sid = zeros(SelectionID{UInt32}, widths(rect)...)
+    w, h = size(screen.root_scene)
     if rx > 0 && ry > 0 && rx + rw <= w && ry + rh <= h
+        rx, ry, rw, rh = round.(Int, screen.px_per_unit[] .* (rx, ry, rw, rh))
+        sid = zeros(SelectionID{UInt32}, rw, rh)
         glReadPixels(rx, ry, rw, rh, buff.format, buff.pixeltype, sid)
         return sid
     else
@@ -26,15 +26,15 @@ end
 function pick_native(screen::Screen, xy::Vec{2, Float64})
     isopen(screen) || return SelectionID{Int}(0, 0)
     ShaderAbstractions.switch_context!(screen.glscreen)
-    sid = Base.RefValue{SelectionID{UInt32}}()
-    window_size = size(screen)
     fb = screen.framebuffer
     buff = fb.buffers[:objectid]
     glBindFramebuffer(GL_FRAMEBUFFER, fb.id[1])
     glReadBuffer(GL_COLOR_ATTACHMENT1)
     x, y = floor.(Int, xy)
-    w, h = window_size
+    w, h = size(screen.root_scene)
     if x > 0 && y > 0 && x <= w && y <= h
+        x, y = round.(Int, screen.px_per_unit[] .* (x, y))
+        sid = Base.RefValue{SelectionID{UInt32}}()
         glReadPixels(x, y, 1, 1, buff.format, buff.pixeltype, sid)
         return convert(SelectionID{Int}, sid[])
     end
@@ -65,7 +65,7 @@ end
 # Skips one set of allocations
 function Makie.pick_closest(scene::Scene, screen::Screen, xy, range)
     isopen(screen) || return (nothing, 0)
-    w, h = size(screen)
+    w, h = size(scene)
     ((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h)) || return (nothing, 0)
 
     x0, y0 = max.(1, floor.(Int, xy .- range))
@@ -95,7 +95,7 @@ end
 # Skips some allocations
 function Makie.pick_sorted(scene::Scene, screen::Screen, xy, range)
     isopen(screen) || return (nothing, 0)
-    w, h = size(screen)
+    w, h = size(scene)
     if !((1.0 <= xy[1] <= w) && (1.0 <= xy[2] <= h))
         return Tuple{AbstractPlot, Int}[]
     end
diff --git a/GLMakie/src/postprocessing.jl b/GLMakie/src/postprocessing.jl
index 3c295d158a6..fa55afd4bfd 100644
--- a/GLMakie/src/postprocessing.jl
+++ b/GLMakie/src/postprocessing.jl
@@ -285,14 +285,11 @@ function to_screen_postprocessor(framebuffer, shader_cache, screen_fb_id = nothi
     pass.postrenderfunction = () -> draw_fullscreen(pass.vertexarray.id)
 
     full_render = screen -> begin
-        fb = screen.framebuffer
-        w, h = size(fb)
-
         # transfer everything to the screen
         default_id = isnothing(screen_fb_id) ? 0 : screen_fb_id[]
         # GLFW uses 0, Gtk uses a value that we have to probe at the beginning of rendering
         glBindFramebuffer(GL_FRAMEBUFFER, default_id)
-        glViewport(0, 0, w, h)
+        glViewport(0, 0, framebuffer_size(screen.glscreen)...)
         glClear(GL_COLOR_BUFFER_BIT)
         GLAbstraction.render(pass) # copy postprocess
     end
diff --git a/GLMakie/src/rendering.jl b/GLMakie/src/rendering.jl
index 811bb8094f0..e9bc680913a 100644
--- a/GLMakie/src/rendering.jl
+++ b/GLMakie/src/rendering.jl
@@ -1,14 +1,14 @@
-
-function setup!(screen)
+function setup!(screen::Screen)
     glEnable(GL_SCISSOR_TEST)
-    if isopen(screen)
-        glScissor(0, 0, size(screen)...)
+    if isopen(screen) && !isnothing(screen.root_scene)
+        sf = screen.px_per_unit[]
+        glScissor(0, 0, round.(Int, size(screen.root_scene) .* sf)...)
         glClearColor(1, 1, 1, 1)
         glClear(GL_COLOR_BUFFER_BIT)
         for (id, scene) in screen.screens
             if scene.visible[]
                 a = pixelarea(scene)[]
-                rt = (minimum(a)..., widths(a)...)
+                rt = (round.(Int, sf .* minimum(a))..., round.(Int, sf .* widths(a))...)
                 glViewport(rt...)
                 if scene.clear
                     c = scene.backgroundcolor[]
@@ -45,11 +45,10 @@ function render_frame(screen::Screen; resize_buffers=true)
     # render order here may introduce artifacts because of that.
 
     fb = screen.framebuffer
-    if resize_buffers
-        wh = Int.(framebuffer_size(nw))
-        resize!(fb, wh)
+    if resize_buffers && !isnothing(screen.root_scene)
+        sf = screen.px_per_unit[]
+        resize!(fb, round.(Int, sf .* size(screen.root_scene))...)
     end
-    w, h = size(fb)
 
     # prepare stencil (for sub-scenes)
     glBindFramebuffer(GL_FRAMEBUFFER, fb.id)
@@ -121,8 +120,9 @@ function GLAbstraction.render(filter_elem_func, screen::Screen)
             found, scene = id2scene(screen, screenid)
             found || continue
             scene.visible[] || continue
+            sf = screen.px_per_unit[]
             a = pixelarea(scene)[]
-            glViewport(minimum(a)..., widths(a)...)
+            glViewport(round.(Int, sf .* minimum(a))..., round.(Int, sf .* widths(a))...)
             render(elem)
         end
     catch e
diff --git a/GLMakie/src/screen.jl b/GLMakie/src/screen.jl
index 4afc2844a57..69e3eae7c31 100644
--- a/GLMakie/src/screen.jl
+++ b/GLMakie/src/screen.jl
@@ -8,19 +8,19 @@ function renderloop end
 """
 ## Renderloop
 
-* `renderloop = GLMakie.renderloop`: sets a function `renderloop(::GLMakie.Screen)` which starts a renderloop for the screen.
+* `renderloop = GLMakie.renderloop`: Sets a function `renderloop(::GLMakie.Screen)` which starts a renderloop for the screen.
 
-
-    !!! warning
+!!! warning
     The below are not effective if renderloop isn't set to `GLMakie.renderloop`, unless implemented in custom renderloop:
 
-
 * `pause_renderloop = false`: creates a screen with paused renderlooop. Can be started with `GLMakie.start_renderloop!(screen)` or paused again with `GLMakie.pause_renderloop!(screen)`.
 * `vsync = false`: enables vsync for the window.
 * `render_on_demand = true`: renders the scene only if something has changed in it.
 * `framerate = 30.0`: sets the currently rendered frames per second.
+* `px_per_unit = automatic`: Sets the ratio between the number of rendered pixels and the `Makie` resolution. It defaults to the value of `scalefactor` but may be any positive real number.
 
 ## GLFW window attributes
+
 * `float = false`: Lets the opened window float above anything else.
 * `focus_on_show = false`: Focusses the window when newly opened.
 * `decorated = true`: shows the window decorations or not.
@@ -28,6 +28,8 @@ function renderloop end
 * `fullscreen = false`: Starts the window in fullscreen.
 * `debugging = false`: Starts the GLFW.Window/OpenGL context with debug output.
 * `monitor::Union{Nothing, GLFW.Monitor} = nothing`: Sets the monitor on which the Window should be opened.
+* `visible = true`: Sets whether the window is user-visible.
+* `scalefactor = automatic`: Sets the window scaling factor, such as `2.0` on HiDPI/Retina displays. It is set automatically based on the display, but may be any positive real number.
 
 ## Postprocessor
 * `oit = false`: Enles order independent transparency for the window.
@@ -43,6 +45,7 @@ mutable struct ScreenConfig
     vsync::Bool
     render_on_demand::Bool
     framerate::Float64
+    px_per_unit::Union{Nothing, Float32}
 
     # GLFW window attributes
     float::Bool
@@ -53,6 +56,7 @@ mutable struct ScreenConfig
     debugging::Bool
     monitor::Union{Nothing, GLFW.Monitor}
     visible::Bool
+    scalefactor::Union{Nothing, Float32}
 
     # Postprocessor
     oit::Bool
@@ -67,6 +71,7 @@ mutable struct ScreenConfig
             vsync::Bool,
             render_on_demand::Bool,
             framerate::Number,
+            px_per_unit::Union{Makie.Automatic, Number},
             # GLFW window attributes
             float::Bool,
             focus_on_show::Bool,
@@ -76,6 +81,7 @@ mutable struct ScreenConfig
             debugging::Bool,
             monitor::Union{Nothing, GLFW.Monitor},
             visible::Bool,
+            scalefactor::Union{Makie.Automatic, Number},
 
             # Preproccessor
             oit::Bool,
@@ -90,6 +96,7 @@ mutable struct ScreenConfig
             vsync,
             render_on_demand,
             framerate,
+            px_per_unit isa Makie.Automatic ? nothing : Float32(px_per_unit),
             # GLFW window attributes
             float,
             focus_on_show,
@@ -99,6 +106,7 @@ mutable struct ScreenConfig
             debugging,
             monitor,
             visible,
+            scalefactor isa Makie.Automatic ? nothing : Float32(scalefactor),
             # Preproccessor
             oit,
             fxaa,
@@ -148,6 +156,7 @@ mutable struct Screen{GLWindow} <: MakieScreen
     config::Union{Nothing, ScreenConfig}
     stop_renderloop::Bool
     rendertask::Union{Task, Nothing}
+    px_per_unit::Observable{Float32}
 
     screen2scene::Dict{WeakRef, ScreenID}
     screens::Vector{ScreenArea}
@@ -158,6 +167,7 @@ mutable struct Screen{GLWindow} <: MakieScreen
     framecache::Matrix{RGB{N0f8}}
     render_tick::Observable{Nothing}
     window_open::Observable{Bool}
+    scalefactor::Observable{Float32}
 
     root_scene::Union{Scene, Nothing}
     reuse::Bool
@@ -186,10 +196,10 @@ mutable struct Screen{GLWindow} <: MakieScreen
         screen = new{GLWindow}(
             glscreen, shader_cache, framebuffer,
             config, stop_renderloop, rendertask,
-            screen2scene,
+            Observable(0f0), screen2scene,
             screens, renderlist, postprocessors, cache, cache2plot,
             Matrix{RGB{N0f8}}(undef, s), Observable(nothing),
-            Observable(true), nothing, reuse, true, false
+            Observable(true), Observable(0f0), nothing, reuse, true, false
         )
         push!(ALL_SCREENS, screen) # track all created screens
         return screen
@@ -214,6 +224,8 @@ function empty_screen(debugging::Bool; reuse=true)
 
         (GLFW.STENCIL_BITS, 0),
         (GLFW.AUX_BUFFERS,  0),
+
+        (GLFW.SCALE_TO_MONITOR, true),
     ]
     resolution = (10, 10)
     window = try
@@ -263,6 +275,14 @@ function empty_screen(debugging::Bool; reuse=true)
         reuse,
     )
     GLFW.SetWindowRefreshCallback(window, window -> refreshwindowcb(window, screen))
+    GLFW.SetWindowContentScaleCallback(window, (window, xs, ys) -> scalechangecb(screen, window, xs, ys))
+    on(screen.scalefactor) do sf
+        if !isnothing(screen.root_scene)
+            resize!(screen, size(screen.root_scene)...)
+        end
+        return nothing
+    end
+
     return screen
 end
 
@@ -302,8 +322,6 @@ function singleton_screen(debugging::Bool)
     return reopen!(screen)
 end
 
-const GLFW_FOCUS_ON_SHOW = 0x0002000C
-
 function Makie.apply_screen_config!(screen::Screen, config::ScreenConfig, scene::Scene, args...)
     apply_config!(screen, config)
 end
@@ -311,7 +329,7 @@ end
 function apply_config!(screen::Screen, config::ScreenConfig; start_renderloop::Bool=true)
     glw = screen.glscreen
     ShaderAbstractions.switch_context!(glw)
-    GLFW.SetWindowAttrib(glw, GLFW_FOCUS_ON_SHOW, config.focus_on_show)
+    GLFW.SetWindowAttrib(glw, GLFW.FOCUS_ON_SHOW, config.focus_on_show)
     GLFW.SetWindowAttrib(glw, GLFW.DECORATED, config.decorated)
     GLFW.SetWindowAttrib(glw, GLFW.FLOATING, config.float)
     GLFW.SetWindowTitle(glw, config.title)
@@ -319,6 +337,8 @@ function apply_config!(screen::Screen, config::ScreenConfig; start_renderloop::B
     if !isnothing(config.monitor)
         GLFW.SetWindowMonitor(glw, config.monitor)
     end
+    screen.scalefactor[] = !isnothing(config.scalefactor) ? config.scalefactor : scale_factor(glw)
+    screen.px_per_unit[] = !isnothing(config.px_per_unit) ? config.px_per_unit : screen.scalefactor[]
 
     function replace_processor!(postprocessor, idx)
         fb = screen.framebuffer
@@ -355,10 +375,10 @@ function Screen(;
     # Screen config is managed by the current active theme, so managed by Makie
     config = Makie.merge_screen_config(ScreenConfig, screen_config)
     screen = screen_from_pool(config.debugging)
+    apply_config!(screen, config; start_renderloop=start_renderloop)
     if !isnothing(resolution)
         resize!(screen, resolution...)
     end
-    apply_config!(screen, config; start_renderloop=start_renderloop)
     return screen
 end
 
@@ -600,24 +620,29 @@ function closeall()
     return
 end
 
-function resize_native!(window::GLFW.Window, resolution...)
-    if isopen(window)
-        ShaderAbstractions.switch_context!(window)
-        oldsize = windowsize(window)
-        retina_scale = retina_scaling_factor(window)
-        w, h = resolution ./ retina_scale
-        if oldsize == (w, h)
-            return
-        end
-        GLFW.SetWindowSize(window, round(Int, w), round(Int, h))
+function Base.resize!(screen::Screen, w::Int, h::Int)
+    window = to_native(screen)
+    (w > 0 && h > 0 && isopen(window)) || return nothing
+
+    # Resize the window which appears on the user desktop (if necessary).
+    # Apple Retina displays work in logical dimensions (and automatically scales the
+    # backing frame buffer by 2), whereas both Linux and Windows have window and buffer
+    # dimensions match (so we must scale manually from logical size to window size).
+    #
+    # N.B. The GLFW framebuffer is different from the rendering framebuffers resized below.
+    ShaderAbstractions.switch_context!(window)
+    winscale = @static Sys.isapple() ? 1f0 : screen.scalefactor[]
+    winw, winh = round.(Int, winscale .* (w, h))
+    if window_size(window) != (winw, winh)
+        GLFW.SetWindowSize(window, winw, winh)
     end
-end
 
-function Base.resize!(screen::Screen, w, h)
-    nw = to_native(screen)
-    resize_native!(nw, w, h)
-    fb = screen.framebuffer
-    resize!(fb, (w, h))
+    # Then resize the underlying rendering framebuffers as well, which can be scaled
+    # independently of the window scale factor.
+    fbscale = screen.px_per_unit[]
+    fbw, fbh = round.(Int, fbscale .* (w, h))
+    resize!(screen.framebuffer, fbw, fbh)
+    return nothing
 end
 
 function fast_color_data!(dest::Array{RGB{N0f8}, 2}, source::Texture{T, 2}) where T
@@ -662,8 +687,7 @@ function Makie.colorbuffer(screen::Screen, format::Makie.ImageStorageFormat = Ma
     # polling may change window size, when its bigger than monitor!
     # we still need to poll though, to get all the newest events!
     # GLFW.PollEvents()
-    # keep current buffer size to allows larger-than-window renders
-    render_frame(screen, resize_buffers=false) # let it render
+    render_frame(screen, resize_buffers=true) # let it render
     glFinish() # block until opengl is done rendering
     if size(ctex) != size(screen.framecache)
         screen.framecache = Matrix{RGB{N0f8}}(undef, size(ctex))
@@ -791,6 +815,16 @@ function refreshwindowcb(window, screen)
     return
 end
 
+function scalechangecb(screen, window, xscale, yscale)
+    sf = min(xscale, yscale)
+    if isnothing(screen.config.px_per_unit) && screen.scalefactor[] == screen.px_per_unit[]
+        screen.px_per_unit[] = sf
+    end
+    screen.scalefactor[] = sf
+    resize!(screen, size(screen.root_scene)...)
+    return
+end
+
 # TODO add render_tick event to scene events
 function vsynced_renderloop(screen)
     while isopen(screen) && !screen.stop_renderloop
diff --git a/GLMakie/test/runtests.jl b/GLMakie/test/runtests.jl
index d73329c6825..437f1022c5b 100644
--- a/GLMakie/test/runtests.jl
+++ b/GLMakie/test/runtests.jl
@@ -16,7 +16,7 @@ reference_tests_dir = normpath(joinpath(dirname(pathof(Makie)), "..", "Reference
 Pkg.develop(PackageSpec(path = reference_tests_dir))
 using ReferenceTests
 
-GLMakie.activate!(framerate=1.0)
+GLMakie.activate!(framerate=1.0, scalefactor=1.0)
 
 @testset "mimes" begin
     f, ax, pl = scatter(1:4)
diff --git a/GLMakie/test/unit_tests.jl b/GLMakie/test/unit_tests.jl
index ea16da8f284..63219f70b0c 100644
--- a/GLMakie/test/unit_tests.jl
+++ b/GLMakie/test/unit_tests.jl
@@ -254,3 +254,116 @@ end
     # now every screen should be gone
     @test isempty(GLMakie.SCREEN_REUSE_POOL)
 end
+
+@testset "HiDPI displays" begin
+    import FileIO: @format_str, File, load
+    GLMakie.closeall()
+
+    W, H = 400, 400
+    N = 51
+    x = collect(range(0.0, 2π, length=N))
+    y = sin.(x)
+    fig, ax, pl = scatter(x, y, figure = (; resolution = (W, H)));
+    hidedecorations!(ax)
+
+    screen = display(GLMakie.Screen(visible = false, scalefactor = 2), fig)
+    @test screen.scalefactor[] === 2f0
+    @test screen.px_per_unit[] === 2f0  # inherited from scale factor
+    @test size(screen.framebuffer) == (2W, 2H)
+    if !Sys.isapple()
+        @test GLMakie.window_size(screen.glscreen) == (2W, 2H)
+    else
+        @test GLMakie.window_size(screen.glscreen) == (W, H)
+    end
+
+    # check that picking works through the resized GL buffers
+    GLMakie.Makie.colorbuffer(screen)  # force render
+    # - point pick
+    point_px = project_sp(ax.scene, Point2f(x[end÷2], y[end÷2]))
+    elem, idx = pick(ax.scene, point_px)
+    @test elem === pl
+    @test idx == length(x) ÷ 2
+    # - area pick
+    bottom_px = project_sp(ax.scene, Point2f(π, -1))
+    right_px = project_sp(ax.scene, Point2f(2π, 0))
+    quadrant = Rect2i(round.(bottom_px)..., round.(right_px - bottom_px)...)
+    picks = pick(ax.scene, quadrant)
+    points = Set(Int(p[2]) for p in picks if p[1] isa Scatter)
+    @test points == Set(((N+1)÷2):N)
+
+    # render at lower resolution
+    screen = display(GLMakie.Screen(visible = false, scalefactor = 2, px_per_unit = 1), fig)
+    @test screen.scalefactor[] === 2f0
+    @test screen.px_per_unit[] === 1f0
+    @test size(screen.framebuffer) == (W, H)
+
+    # 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) == (W, H)
+
+    # save images of different resolutions
+    mktemp() do path, io
+        close(io)
+        file = File{format"PNG"}(path)
+
+        # save at current size
+        @test screen.px_per_unit[] == 1
+        save(file, fig)
+        img = load(file)
+        @test size(img) == (W, H)
+
+        # save with a different resolution
+        save(file, fig, px_per_unit = 2)
+        img = load(file)
+        @test size(img) == (2W, 2H)
+        # writing to file should not effect the visible figure
+        @test_broken screen.px_per_unit[] == 1
+    end
+
+    if Sys.islinux()
+        # Test that GLMakie is correctly getting the default scale factor from X11 in a
+        # HiDPI environment.
+
+        checkcmd = `which xrdb` & `which xsettingsd`
+        checkcmd = pipeline(ignorestatus(checkcmd), stdout = devnull, stderr = devnull)
+        hasxrdb = success(run(checkcmd))
+
+        # Only continue if running within an Xvfb environment where the setting is
+        # empty by default. Overriding during a user's session could be problematic
+        # (i.e. if running interactively rather than in CI).
+        inxvfb = hasxrdb ? isempty(readchomp(`xrdb -query`)) : false
+
+        if hasxrdb && inxvfb
+            # GLFW looks for Xft.dpi resource setting. Spawn a temporary xsettingsd daemon
+            # to be the X resource manager
+            xsettingsd = run(pipeline(`xsettingsd -c /dev/null`), wait = false)
+            try
+                # Then set the DPI to 192, i.e. 2 times the default of 96dpi
+                run(pipeline(`echo "Xft.dpi: 192"`, `xrdb -merge`))
+
+                # Print out the automatically-determined scale factor from the GLScreen
+                jlscript = raw"""
+                using GLMakie
+                fig, ax, pl = scatter(1:2, 3:4)
+                screen = display(GLMakie.Screen(visible = false), fig)
+                print(Int(screen.scalefactor[]))
+                """
+                cmd = ```
+                    $(Base.julia_cmd())
+                    --project=$(Base.active_project())
+                    --eval $jlscript
+                    ```
+                scalefactor = readchomp(cmd)
+                @test scalefactor == "2"
+            finally
+                # cleanup: kill the daemon before continuing with more tests
+                kill(xsettingsd)
+            end
+        else
+            @test_broken hasxrdb && inxvfb
+        end
+    else
+        @test_broken Sys.islinux()
+    end
+end
diff --git a/NEWS.md b/NEWS.md
index 1f29c947ae5..25a2c2dc020 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -6,6 +6,9 @@
 - Fixed an issue where `poly` plots with `Vector{<: MultiPolygon}` inputs with per-polygon color were mistakenly rendered as meshes using CairoMakie. [#2590](https://github.com/MakieOrg/Makie.jl/pulls/2478)
 - Fixed a small typo which caused an error in the `Stepper` constructor. [#2600](https://github.com/MakieOrg/Makie.jl/pulls/2478)
 - Fixed rectangle zoom for nonlinear axes [#2674](https://github.com/MakieOrg/Makie.jl/pull/2674)
+- GLMakie has gained support for HiDPI (aka Retina) screens.
+  This also enables saving images with higher resolution than screen pixel dimensions.
+  [#2544](https://github.com/MakieOrg/Makie.jl/pull/2544)
 
 ## v0.19.1
 
diff --git a/docs/documentation/backends/glmakie.md b/docs/documentation/backends/glmakie.md
index c270749dc8c..ff87cf7a6b6 100644
--- a/docs/documentation/backends/glmakie.md
+++ b/docs/documentation/backends/glmakie.md
@@ -15,6 +15,50 @@ println("~~~")
 ```
 \textoutput{docs}
 
+#### Window Scaling
+
+The sizes of figures are given in display-independent "logical" dimensions, and the
+GLMakie backend will scale the size of the displayed window on HiDPI/Retina displays
+automatically.
+For example, the default `resolution = (800, 600)` will be shown in a 1600 × 1200 window
+on a HiDPI display which is configured with a 200% scaling factor.
+
+The scaling factor may be overridden by displaying the figure with a different
+`scalefactor` value:
+```julia
+fig = Figure(resolution = (800, 600))
+# ...
+display(fig, scalefactor = 1.5)
+```
+
+If the scale factor is not changed from its default automatic configuration, the window
+will be resized to maintain its apparent size when moved across displays with different
+scaling factors on Windows and OSX.
+(Independent scaling factors are not supported by X11, and at this time the underlying
+GLFW library is not compiled with Wayland support.)
+
+#### Resolution Scaling
+
+Related to the window scaling factor, the mapping from figure sizes and positions to pixels
+can be scaled to achieve HiDPI/Retina resolution renderings.
+The resolution scaling defaults to the same factor as the window scaling, but it may
+be independently overridden with the `px_per_unit` argument when showing a figure:
+```julia
+fig = Figure(resolution = (800, 600))
+# ...
+display(fig, px_per_unit = 2)
+```
+
+The resolution scale factor may also be changed when saving pngs:
+```julia
+save("hires.png", fig, px_per_unit = 2)    # 1600 × 1200 px png
+save("lores.png", fig, px_per_unit = 0.5)  #  400 × 300 px png
+```
+If a script may run in interactive environments where the native screen DPI can vary,
+you may want to explicitly set `px_per_unit = 1` when saving figures to ensure consistency
+of results.
+
+
 #### Multiple Windows
 
 GLMakie has experimental support for displaying multiple independent figures (or scenes). To open a new window, use `display(GLMakie.Screen(), figure_or_scene)`.
diff --git a/src/theming.jl b/src/theming.jl
index 1cbbbe39d38..0098e0eea66 100644
--- a/src/theming.jl
+++ b/src/theming.jl
@@ -85,6 +85,7 @@ const minimal_default = Attributes(
         vsync = false,
         render_on_demand = true,
         framerate = 30.0,
+        px_per_unit = automatic,
 
         # GLFW window attributes
         float = false,
@@ -95,6 +96,7 @@ const minimal_default = Attributes(
         debugging = false,
         monitor = nothing,
         visible = true,
+        scalefactor = automatic,
 
         # Postproccessor
         oit = true,

From fe6d6cf09e498dc7d171746a5028dd0c3e5d7bc3 Mon Sep 17 00:00:00 2001
From: Justin Willmert <justin@willmert.me>
Date: Mon, 30 Jan 2023 17:43:44 -0600
Subject: [PATCH 2/3] Cleanup scalefactor/px_per_unit observables on close &
 resize on OSX

- Clear the `scalefactor` and `px_per_unit` observables when the figure
  is closed. It's possible both may be used during a figure's lifetime,
  but that should not persist past a close.

- In the system scale factor callback, do not unconditionally resize
  the native window. Instead, leverage the fact that the observable
  callback will invoke a resize event, and that correctly will check that
  the root scene is not `nothing`.

- Do not completely ignore the scale factor on OSX. The difference is
  that there is a native scaling factor applied by the OS, but if/when
  the desired scale factor differs from the native scaling, we must
  still make adjustments.
---
 GLMakie/src/events.jl      | 14 +++++++------
 GLMakie/src/screen.jl      | 43 ++++++++++++++++++++++++--------------
 GLMakie/test/unit_tests.jl | 18 ++++++++++------
 3 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/GLMakie/src/events.jl b/GLMakie/src/events.jl
index 8f6231d8e31..f736f1decaf 100644
--- a/GLMakie/src/events.jl
+++ b/GLMakie/src/events.jl
@@ -51,19 +51,20 @@ 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, framebuffer_size((window))
+    #     event[] = Recti(x, y, winw, winh)
     # end
-    w, h = round.(Int, framebuffer_size(nw) ./ x.screen.scalefactor[])
-    if Vec(w, h) != widths(rect)
+    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), w, h)
+        x.area[] = Recti(minimum(rect), winw, winh)
     end
     return
 end
@@ -169,8 +170,9 @@ function Makie.disconnect!(window::GLFW.Window, ::typeof(unicode_input))
 end
 
 function correct_mouse(screen::Screen, w, h)
-    sf = screen.scalefactor[]
-    _, winh = framebuffer_size(to_native(screen))
+    nw = to_native(screen)
+    sf = screen.scalefactor[] / (@static Sys.isapple() ? scale_factor(nw) : 1)
+    _, winh = window_size(nw)
     @static if Sys.isapple()
         return w, (winh / sf) - h
     else
diff --git a/GLMakie/src/screen.jl b/GLMakie/src/screen.jl
index 69e3eae7c31..6aa9013a23e 100644
--- a/GLMakie/src/screen.jl
+++ b/GLMakie/src/screen.jl
@@ -274,14 +274,8 @@ function empty_screen(debugging::Bool; reuse=true)
         Dict{UInt32, AbstractPlot}(),
         reuse,
     )
-    GLFW.SetWindowRefreshCallback(window, window -> refreshwindowcb(window, screen))
-    GLFW.SetWindowContentScaleCallback(window, (window, xs, ys) -> scalechangecb(screen, window, xs, ys))
-    on(screen.scalefactor) do sf
-        if !isnothing(screen.root_scene)
-            resize!(screen, size(screen.root_scene)...)
-        end
-        return nothing
-    end
+    GLFW.SetWindowRefreshCallback(window, refreshwindowcb(screen))
+    GLFW.SetWindowContentScaleCallback(window, scalechangecb(screen))
 
     return screen
 end
@@ -296,6 +290,7 @@ function reopen!(screen::Screen)
     end
     @assert isempty(screen.window_open.listeners)
     screen.window_open[] = true
+    on(scalechangeobs(screen), screen.scalefactor)
     @assert isopen(screen)
     return screen
 end
@@ -573,7 +568,10 @@ function destroy!(screen::Screen)
     # otherwise, during rendertask clean up we may run into a destroyed window
     wait(screen)
     screen.rendertask = nothing
-    destroy!(screen.glscreen)
+    window = screen.glscreen
+    GLFW.SetWindowRefreshCallback(window, nothing)
+    GLFW.SetWindowContentScaleCallback(window, nothing)
+    destroy!(window)
     # Since those are sets, we can just delete them from there, even if they weren't in there (e.g. reuse=false)
     delete!(SCREEN_REUSE_POOL, screen)
     delete!(ALL_SCREENS, screen)
@@ -595,6 +593,8 @@ function Base.close(screen::Screen; reuse=true)
         screen.window_open[] = false
     end
     empty!(screen)
+    Observables.clear(screen.px_per_unit)
+    Observables.clear(screen.scalefactor)
     if reuse && screen.reuse
         push!(SCREEN_REUSE_POOL, screen)
     end
@@ -625,13 +625,14 @@ function Base.resize!(screen::Screen, w::Int, h::Int)
     (w > 0 && h > 0 && isopen(window)) || return nothing
 
     # Resize the window which appears on the user desktop (if necessary).
-    # Apple Retina displays work in logical dimensions (and automatically scales the
-    # backing frame buffer by 2), whereas both Linux and Windows have window and buffer
-    # dimensions match (so we must scale manually from logical size to window size).
     #
-    # N.B. The GLFW framebuffer is different from the rendering framebuffers resized below.
+    # On OSX with a Retina display, the window size is given in logical dimensions and
+    # is automatically scaled by the OS. To support arbitrary scale factors, we must account
+    # for the native scale factor when calculating the effective scaling to apply.
+    #
+    # On Linux and Windows, scale from the logical size to the pixel size.
     ShaderAbstractions.switch_context!(window)
-    winscale = @static Sys.isapple() ? 1f0 : screen.scalefactor[]
+    winscale = screen.scalefactor[] / (@static Sys.isapple() ? scale_factor(window) : 1)
     winw, winh = round.(Int, winscale .* (w, h))
     if window_size(window) != (winw, winh)
         GLFW.SetWindowSize(window, winw, winh)
@@ -808,12 +809,13 @@ function set_framerate!(screen::Screen, fps=30)
     screen.config.framerate = fps
 end
 
-function refreshwindowcb(window, screen)
+function refreshwindowcb(screen, window)
     screen.render_tick[] = nothing
     render_frame(screen)
     GLFW.SwapBuffers(window)
     return
 end
+refreshwindowcb(screen) = window -> refreshwindowcb(screen, window)
 
 function scalechangecb(screen, window, xscale, yscale)
     sf = min(xscale, yscale)
@@ -821,9 +823,18 @@ function scalechangecb(screen, window, xscale, yscale)
         screen.px_per_unit[] = sf
     end
     screen.scalefactor[] = sf
-    resize!(screen, size(screen.root_scene)...)
     return
 end
+scalechangecb(screen) = (window, xscale, yscale) -> scalechangecb(screen, window, xscale, yscale)
+
+function scalechangeobs(screen, _)
+    if !isnothing(screen.root_scene)
+        resize!(screen, size(screen.root_scene)...)
+    end
+    return nothing
+end
+scalechangeobs(screen) = scalefactor -> scalechangeobs(screen, scalefactor)
+
 
 # TODO add render_tick event to scene events
 function vsynced_renderloop(screen)
diff --git a/GLMakie/test/unit_tests.jl b/GLMakie/test/unit_tests.jl
index 63219f70b0c..45325358606 100644
--- a/GLMakie/test/unit_tests.jl
+++ b/GLMakie/test/unit_tests.jl
@@ -242,6 +242,8 @@ end
 
         @test isempty(screen.window_open.listeners)
         @test isempty(screen.render_tick.listeners)
+        @test isempty(screen.px_per_unit.listeners)
+        @test isempty(screen.scalefactor.listeners)
 
         @test screen.root_scene === nothing
         @test screen.rendertask === nothing
@@ -266,15 +268,19 @@ end
     fig, ax, pl = scatter(x, y, figure = (; resolution = (W, H)));
     hidedecorations!(ax)
 
+    # On OSX, the native window size has an underlying scale factor that we need to account
+    # for when interpreting native window sizes with respect to the desired figure size
+    # and desired scaling factor.
+    function scaled(screen::GLMakie.Screen, dims::Tuple{Vararg{Int}})
+        sf = screen.scalefactor[] / (Sys.isapple() ? GLMakie.scale_factor(screen.glscreen) : 1)
+        return round.(Int, dims .* sf)
+    end
+
     screen = display(GLMakie.Screen(visible = false, scalefactor = 2), fig)
     @test screen.scalefactor[] === 2f0
     @test screen.px_per_unit[] === 2f0  # inherited from scale factor
     @test size(screen.framebuffer) == (2W, 2H)
-    if !Sys.isapple()
-        @test GLMakie.window_size(screen.glscreen) == (2W, 2H)
-    else
-        @test GLMakie.window_size(screen.glscreen) == (W, H)
-    end
+    @test GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
 
     # check that picking works through the resized GL buffers
     GLMakie.Makie.colorbuffer(screen)  # force render
@@ -300,7 +306,7 @@ 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) == (W, H)
+    @test GLMakie.window_size(screen.glscreen) == scaled(screen, (W, H))
 
     # save images of different resolutions
     mktemp() do path, io

From 5ba035c247620ac13b2952e3f20f1aee757284e2 Mon Sep 17 00:00:00 2001
From: Justin Willmert <justin@willmert.me>
Date: Sat, 4 Feb 2023 20:55:16 -0600
Subject: [PATCH 3/3] 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      | 72 ++++++++++++++++++--------------------
 GLMakie/src/glwindow.jl    |  4 +++
 GLMakie/test/unit_tests.jl | 13 ++++++-
 3 files changed, 51 insertions(+), 38 deletions(-)

diff --git a/GLMakie/src/events.jl b/GLMakie/src/events.jl
index f736f1decaf..3992d28d6fa 100644
--- a/GLMakie/src/events.jl
+++ b/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))
diff --git a/GLMakie/src/glwindow.jl b/GLMakie/src/glwindow.jl
index 65e62bdb5c5..3ccf5ca1ef0 100644
--- a/GLMakie/src/glwindow.jl
+++ b/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))
diff --git a/GLMakie/test/unit_tests.jl b/GLMakie/test/unit_tests.jl
index 45325358606..5659c5b2a4f 100644
--- a/GLMakie/test/unit_tests.jl
+++ b/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