Improve Relocatability for GLMakie and fix tests (#4461)

* clean up shader loading

* dont empty after precompile

* try better relocatability test

* 1.11 doesn't work yet

* add changelog entry

* Improvements to relocatability test script (#4468)

use `run` to get printouts and `finally` to not swallow errors in `catch`

---------

Co-authored-by: Julius Krumbiegel <22495855+jkrumbiegel@users.noreply.github.com>

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [Unreleased]
 
+- Fix relocatability of GLMakie [#4461](https://github.com/MakieOrg/Makie.jl/pull/4461).
+
 ## [0.21.13] - 2024-10-07
 
 - Optimize SpecApi, re-use Blocks better and add API to access the created block objects [#4354](https://github.com/MakieOrg/Makie.jl/pull/4354).

GLMakie/src/GLMakie.jl

@@ -55,31 +55,42 @@ function ShaderSource(path)
     return ShaderSource(typ, source, name)
 end
 
-const GL_ASSET_DIR = RelocatableFolders.@path joinpath(@__DIR__, "..", "assets")
-const SHADER_DIR = RelocatableFolders.@path joinpath(GL_ASSET_DIR, "shader")
+const SHADER_DIR = normpath(joinpath(@__DIR__, "..", "assets", "shader"))
 const LOADED_SHADERS = Dict{String, ShaderSource}()
+const WARN_ON_LOAD = Ref(false)
 
-const SHADER_PATHS = Dict{String,Union{RelocatableFolders.Path, String}}()
-
-# Turns out, loading shaders is so slow, that it actually makes sense to memoize it :-O
-# when creating 1000 plots with the PlotSpec API, timing drop from 1.5s to 1s just from this change:
-function shader_path(name)
-    return get!(SHADER_PATHS, name) do
-        return joinpath(SHADER_DIR, name)
+function loadshader(name)
+    return get!(LOADED_SHADERS, name) do
+        if WARN_ON_LOAD[]
+            @warn("Reloading shader")
+        end
+        return ShaderSource(joinpath(SHADER_DIR, name))
     end
 end
 
-function loadshader(name)
-    return get!(LOADED_SHADERS, name) do
-        return ShaderSource(shader_path(name))
+function load_all_shaders(folder)
+    for name in readdir(folder)
+        path = joinpath(folder, name)
+        if isdir(path)
+            load_all_shaders(path)
+        elseif any(x -> endswith(name, x), [".frag", ".vert", ".geom"])
+            path = relpath(path, SHADER_DIR)
+            loadshader(replace(path, "\\" => "/"))
+        end
     end
 end
 
+
 gl_texture_atlas() = Makie.get_texture_atlas(2048, 64)
 
 # don't put this into try catch, to not mess with normal errors
 include("gl_backend.jl")
 
+# We load all shaders to compile them into the package Image
+# Making them relocatable
+load_all_shaders(SHADER_DIR)
+WARN_ON_LOAD[] = true
+
 function __init__()
     activate!()
 end

GLMakie/src/precompiles.jl

@@ -47,7 +47,7 @@ let
             close(screen)
 
             empty!(atlas_texture_cache)
-            closeall()
+            closeall(; empty_shader=false)
             @assert isempty(SCREEN_REUSE_POOL)
             @assert isempty(ALL_SCREENS)
             @assert isempty(SINGLETON_SCREEN)

GLMakie/src/screen.jl

@@ -677,10 +677,13 @@ function Base.close(screen::Screen; reuse=true)
     return
 end
 
-function closeall()
+function closeall(; empty_shader=true)
     # Since we call closeall to reload any shader
     # We empty the shader source cache here
-    empty!(LOADED_SHADERS)
+    if empty_shader
+        empty!(LOADED_SHADERS)
+        WARN_ON_LOAD[] = false
+    end
     while !isempty(SCREEN_REUSE_POOL)
         screen = pop!(SCREEN_REUSE_POOL)
         delete!(ALL_SCREENS, screen)
@@ -811,52 +814,6 @@ end
 
 Makie.to_native(x::Screen) = x.glscreen
 
-"""
-    get_loading_image(resolution)
-
-Loads the makie loading icon, embeds it in an image the size of `resolution`,
-and returns the image.
-"""
-function get_loading_image(resolution)
-    icon = Matrix{N0f8}(undef, 192, 192)
-    open(joinpath(GL_ASSET_DIR, "loading.bin")) do io
-        read!(io, icon)
-    end
-    img = zeros(RGBA{N0f8}, resolution...)
-    center = resolution .÷ 2
-    center_icon = size(icon) .÷ 2
-    start = CartesianIndex(max.(center .- center_icon, 1))
-    I1 = CartesianIndex(1, 1)
-    stop = min(start + CartesianIndex(size(icon)) - I1, CartesianIndex(resolution))
-    for idx in start:stop
-        gray = icon[idx - start + I1]
-        img[idx] = RGBA{N0f8}(gray, gray, gray, 1.0)
-    end
-    return img
-end
-
-function display_loading_image(screen::Screen)
-    fb = screen.framebuffer
-    fbsize = size(fb)
-    image = get_loading_image(fbsize)
-    if size(image) == fbsize
-        nw = to_native(screen)
-        # transfer loading image to gpu framebuffer
-        fb.buffers[:color][1:size(image, 1), 1:size(image, 2)] = image
-        ShaderAbstractions.is_context_active(nw) || return
-        w, h = fbsize
-        glBindFramebuffer(GL_FRAMEBUFFER, 0) # transfer back to window
-        glViewport(0, 0, w, h)
-        glClearColor(0, 0, 0, 0)
-        glClear(GL_COLOR_BUFFER_BIT)
-        # GLAbstraction.render(fb.postprocess[end]) # copy postprocess
-        GLAbstraction.render(screen.postprocessors[end].robjs[1])
-        GLFW.SwapBuffers(nw)
-    else
-        error("loading_image needs to be Matrix{RGBA{N0f8}} with size(loading_image) == resolution")
-    end
-end
-
 function renderloop_running(screen::Screen)
     return !screen.stop_renderloop && !isnothing(screen.rendertask) && !istaskdone(screen.rendertask)
 end

relocatability.jl

@@ -14,17 +14,21 @@ end # module MakieApp
 """
 
 using Pkg, Test
-makie_dir = pwd()
 tmpdir = mktempdir()
 # create a temporary project
 cd(tmpdir)
 Pkg.generate("MakieApp")
 Pkg.activate("MakieApp")
 
+makie_dir = @__DIR__
+commit = cd(makie_dir) do
+    chomp(read(`git rev-parse --verify HEAD`, String))
+end
 
-paths = [makie_dir, joinpath(makie_dir, "MakieCore"), joinpath(makie_dir, "GLMakie")]
-
-Pkg.develop(map(x-> (;path=x), paths))
+# Add packages from branch, to make it easier to move the code later (e.g. when running this locally)
+# Since, package dir is much easier to move then the active project (on windows at least).
+paths = ["MakieCore", "Makie", "GLMakie"]
+Pkg.add(map(x -> (; name=x, rev=commit), paths))
 
 open("MakieApp/src/MakieApp.jl", "w") do io
     print(io, module_src)
@@ -37,14 +41,17 @@ using PackageCompiler
 
 create_app(joinpath(pwd(), "MakieApp"), "executable"; force=true, incremental=true, include_transitive_dependencies=false)
 exe = joinpath(pwd(), "executable", "bin", "MakieApp")
-@test success(`$(exe)`)
+# `run` allows to see potential informative printouts, `success` swallows those
+p = run(`$(exe)`)
+@test p.exitcode == 0
 julia_pkg_dir = joinpath(Base.DEPOT_PATH[1], "packages")
 @test isdir(julia_pkg_dir)
 mvd_julia_pkg_dir = julia_pkg_dir * ".old"
+mv(julia_pkg_dir, mvd_julia_pkg_dir, force = true)
 # Move package dir so that we can test relocatability (hardcoded paths to package dir being invalid now)
 try
-    mv(julia_pkg_dir, mvd_julia_pkg_dir)
-    @test success(`$(exe)`)
-catch e
+    p2 = run(`$(exe)`)
+    @test p2.exitcode == 0
+finally
     mv(mvd_julia_pkg_dir, julia_pkg_dir)
 end