Use separate reference images per backend (#3520)

* use separate reference images per backend

* fix paths

* fix path problem in html

* fix artifact name for upload

* consolidate reference image artifacts into one

* add comment

* don't need julia

* take number of missing refimages from the consolidated list

* directly use the "Merge artifacts" step for ReferenceUpdater

* use tile-based comparison of images

* more explanation

* fix bug

* disable 3d contour test for wglmakie

* set comparison thresholds to 0.05

* change movie threshold

* change movie threshold

* reenable storage of 1.6 artifacts for debugging

* disable test broken on 1.6

.github/workflows/reference_tests.yml

@@ -50,11 +50,10 @@ jobs:
           julia --color=yes --project=monorepo -e 'using Pkg; Pkg.test("CairoMakie", coverage=true)'
           && echo "TESTS_SUCCESSFUL=true" >> $GITHUB_ENV
       - name: Upload test Artifacts
-        if: matrix.version == '1'
         uses: actions/upload-artifact@v4
         with:
-          name: ReferenceImages_CairoMakie
-          path: ./CairoMakie/test/recorded_reference_images/
+          name: ReferenceImages_CairoMakie_${{ matrix.version }}
+          path: ./CairoMakie/test/reference_images/
       - name: Fail after artifacts if tests failed
         if: ${{ env.TESTS_SUCCESSFUL != 'true' }}
         run: exit 1
@@ -100,27 +99,11 @@ jobs:
           DISPLAY=:0 xvfb-run -s '-screen 0 1024x768x24' julia --color=yes --project=monorepo -e 'using Pkg; Pkg.test("GLMakie", coverage=true)'
           && echo "TESTS_SUCCESSFUL=true" >> $GITHUB_ENV
       - name: Upload test Artifacts
-        if: matrix.version == '1'
         uses: actions/upload-artifact@v4
         with:
-          name: ReferenceImages_GLMakie
+          name: ReferenceImages_GLMakie_${{ matrix.version }}
           path: |
-            ./GLMakie/test/recorded_reference_images/
-      - name: Save number of missing refimages to file
-        if: matrix.version == '1'
-        env:
-          N_MISSING: ${{ steps.referencetests.outputs.n_missing_refimages }}
-          COMMIT_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
-        run: |
-          mkdir -p ./n_missing
-          echo $N_MISSING > ./n_missing/n_missing_refimages
-          echo $COMMIT_SHA >> ./n_missing/n_missing_refimages
-      - name: Upload artifact with number of missing refimages
-        if: matrix.version == '1'
-        uses: actions/upload-artifact@v4
-        with:
-          name: n_missing_refimages
-          path: n_missing/
+            ./GLMakie/test/reference_images/
       - name: Fail after artifacts if tests failed
         if: ${{ env.TESTS_SUCCESSFUL != 'true' }}
         run: exit 1
@@ -163,15 +146,76 @@ jobs:
           DISPLAY=:0 xvfb-run -s '-screen 0 1024x768x24' julia --color=yes --project=monorepo -e 'using Pkg; Pkg.test("WGLMakie", coverage=true)'
           && echo "TESTS_SUCCESSFUL=true" >> $GITHUB_ENV
       - name: Upload test Artifacts
-        if: matrix.version == '1'
         uses: actions/upload-artifact@v4
         with:
-          name: ReferenceImages_WGLMakie
-          path: ./WGLMakie/test/recorded_reference_images/
+          name: ReferenceImages_WGLMakie_${{ matrix.version }}
+          path: ./WGLMakie/test/reference_images/
       - name: Fail after artifacts if tests failed
         if: ${{ env.TESTS_SUCCESSFUL != 'true' }}
         run: exit 1
       - uses: julia-actions/julia-processcoverage@v1
       - uses: codecov/codecov-action@v3
         with:
           file: lcov.info
+
+  consolidation:
+    name: Merge artifacts
+    runs-on: ubuntu-20.04
+    if: ${{ always() }} # run even if any of the three backend test jobs failed
+    needs: [cairomakie, glmakie, wglmakie]
+    steps:
+      - uses: actions/download-artifact@v4
+        with:
+          name: ReferenceImages_WGLMakie_1
+          path: ./ReferenceImages/WGLMakie
+      - uses: actions/download-artifact@v4
+        with:
+          name: ReferenceImages_CairoMakie_1
+          path: ./ReferenceImages/CairoMakie
+      - uses: actions/download-artifact@v4
+        with:
+          name: ReferenceImages_GLMakie_1
+          path: ./ReferenceImages/GLMakie
+      - name: Consolidate reference image folders
+        run: |
+          baseDir="./ReferenceImages"
+          
+          # Create new top-level directory for combined files
+          mkdir -p "./ReferenceImagesCombined"
+          
+          # Copy the reference folder from GLMakie, it's the same for all backends
+          cp -r "${baseDir}/GLMakie/reference/." "./ReferenceImagesCombined/reference/"
+          
+          # Initialize empty files for concatenation
+          > "./ReferenceImagesCombined/scores.tsv"
+          > "./ReferenceImagesCombined/new_files.txt"
+          
+          # Loop through the directories and concatenate the files, and copy recorded folders
+          for dir in WGLMakie CairoMakie GLMakie; do
+              # Concatenate scores.tsv and new_files.txt
+              cat "${baseDir}/${dir}/scores.tsv" >> "./ReferenceImagesCombined/scores.tsv"
+              cat "${baseDir}/${dir}/new_files.txt" >> "./ReferenceImagesCombined/new_files.txt"
+          
+              # Copy recorded folder
+              mkdir -p "./ReferenceImagesCombined/recorded/${dir}/"
+              cp -r "${baseDir}/${dir}/recorded/${dir}/." "./ReferenceImagesCombined/recorded/${dir}/"
+          done
+          
+          echo "Files and folders have been successfully combined into ReferenceImagesCombined."
+      - uses: actions/upload-artifact@v4
+        with:
+          name: ReferenceImages
+          path: ./ReferenceImagesCombined/
+      - name: Save number of missing refimages to file
+        env:
+          COMMIT_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
+        run: |
+          mkdir -p ./n_missing
+          N_MISSING=$(wc -l < ./ReferenceImagesCombined/new_files.txt)
+          echo $N_MISSING > ./n_missing/n_missing_refimages
+          echo $COMMIT_SHA >> ./n_missing/n_missing_refimages
+      - name: Upload artifact with number of missing refimages
+        uses: actions/upload-artifact@v4
+        with:
+          name: n_missing_refimages
+          path: n_missing/

.gitignore

@@ -10,9 +10,9 @@ Manifest.toml
 
 \.DS_Store
 
-WGLMakie/test/recorded_reference_images/
-GLMakie/test/recorded_reference_images/
-CairoMakie/test/recorded_reference_images/
+WGLMakie/test/reference_images/
+GLMakie/test/reference_images/
+CairoMakie/test/reference_images/
 RPRMakie/test/recorded/
 
 CairoMakie/src/display.svg

CairoMakie/test/runtests.jl

@@ -192,7 +192,7 @@ functions = [:volume, :volume!, :uv_mesh]
 @testset "refimages" begin
     CairoMakie.activate!(type = "png", px_per_unit = 1)
     ReferenceTests.mark_broken_tests(excludes, functions=functions)
-    recorded_files, recording_dir = @include_reference_tests "refimages.jl"
+    recorded_files, recording_dir = @include_reference_tests CairoMakie "refimages.jl"
     missing_images, scores = ReferenceTests.record_comparison(recording_dir)
-    ReferenceTests.test_comparison(scores; threshold = 0.032)
+    ReferenceTests.test_comparison(scores; threshold = 0.05)
 end

GLMakie/test/runtests.jl

@@ -27,23 +27,13 @@ end
 include("unit_tests.jl")
 
 @testset "Reference Tests" begin
-    n_missing_images = 0
     @testset "refimages" begin
         ReferenceTests.mark_broken_tests()
-        recorded_files, recording_dir = @include_reference_tests "refimages.jl"
+        recorded_files, recording_dir = @include_reference_tests GLMakie "refimages.jl" joinpath(@__DIR__, "glmakie_refimages.jl")
         missing_images, scores = ReferenceTests.record_comparison(recording_dir)
-        n_missing_images += length(missing_images)
-        ReferenceTests.test_comparison(scores; threshold = 0.032)
+        ReferenceTests.test_comparison(scores; threshold = 0.05)
     end
 
-    @testset "glmakie_refimages" begin
-        recorded_files, recording_dir = @include_reference_tests joinpath(@__DIR__, "glmakie_refimages.jl")
-        missing_images, scores = ReferenceTests.record_comparison(recording_dir)
-        n_missing_images += length(missing_images)
-        ReferenceTests.test_comparison(scores; threshold = 0.01)
-    end
     GLMakie.closeall()
     GC.gc(true) # make sure no finalizers act up!
-    # pass on status for Github Actions
-    println("::set-output name=n_missing_refimages::$n_missing_images")
 end

ReferenceTests/.gitignore

@@ -1,5 +1,3 @@
 recorded/
-refimages/
-refimages.tar
-glmakie_refimages/
-glmakie_refimages.tar
+reference_images/
+reference_images.tar

ReferenceTests/src/database.jl

@@ -88,21 +88,25 @@ function mark_broken_tests(title_excludes = []; functions=[])
     union!(SKIP_FUNCTIONS, functions)
 end
 
-macro include_reference_tests(path)
+macro include_reference_tests(backend::Symbol, path, paths...)
     toplevel_folder = dirname(string(__source__.file))
     return esc(quote
         using ReferenceTests: @reference_test
-        name = splitext(basename($(path)))[1]
-        include_path = isdir($path) ? $path : joinpath(@__DIR__, "tests", $path)
-        recording_dir = joinpath($toplevel_folder, "recorded_reference_images", name)
+        include_paths = map([$path, $(paths...)]) do p
+            isdir(p) ? p : joinpath(@__DIR__, "tests", p)
+        end
+        recording_dir = joinpath($toplevel_folder, "reference_images")
         if isdir(recording_dir)
             rm(recording_dir; force=true, recursive=true)
         end
-        ReferenceTests.RECORDING_DIR[] = joinpath(recording_dir, "recorded")
-        mkpath(joinpath(recording_dir, "recorded"))
-        @testset "$name" begin
+        # prefix the recordings with the backend name so that each backend has its own versions
+        ReferenceTests.RECORDING_DIR[] = joinpath(recording_dir, "recorded", $(string(backend)))
+        mkpath(ReferenceTests.RECORDING_DIR[])
+        @testset "Reference tests $($(string(backend)))" begin
             empty!(ReferenceTests.REGISTERED_TESTS)
-            include(include_path)
+            for include_path in include_paths
+                include(include_path)
+            end
         end
         recorded_files = collect(ReferenceTests.REGISTERED_TESTS)
         recording_dir = recording_dir

ReferenceTests/src/image_download.jl

@@ -5,10 +5,10 @@ function last_major_version()
     return "v" * string(VersionNumber(version.major, version.minor))
 end
 
-function download_refimages(tag=last_major_version(); name="refimages")
-    url = "https://github.com/MakieOrg/Makie.jl/releases/download/$(tag)/$(name).tar"
-    images_tar = basedir("$(name).tar")
-    images = basedir(name)
+function download_refimages(tag=last_major_version())
+    url = "https://github.com/MakieOrg/Makie.jl/releases/download/$(tag)/reference_images.tar"
+    images_tar = basedir("reference_images.tar")
+    images = basedir("reference_images")
     if isfile(images_tar)
         if Bool(parse(Int, get(ENV, "REUSE_IMAGES_TAR", "0")))
             @info "$images_tar already exists, skipping download as requested"

ReferenceTests/src/runtests.jl

@@ -2,7 +2,10 @@ function get_frames(a, b)
     return (get_frames(a), get_frames(b))
 end
 
-function get_frames(video)
+rgbf_convert(x::AbstractMatrix{<:RGB}) = convert(Matrix{RGBf}, x)
+rgbf_convert(x::AbstractMatrix{<:RGBA}) = convert(Matrix{RGBAf}, x)
+
+function get_frames(video::AbstractString)
     mktempdir() do folder
         afolder = joinpath(folder, "a")
         mkpath(afolder)
@@ -20,31 +23,53 @@ function get_frames(video)
     end
 end
 
-function compare_media(a::Matrix, b::Matrix; sigma=[1,1])
-    Images.test_approx_eq_sigma_eps(a, b, sigma, Inf)
+function compare_images(a::AbstractMatrix{<:Union{RGB,RGBA}}, b::AbstractMatrix{<:Union{RGB,RGBA}})
+
+    a = rgbf_convert(a)
+    b = rgbf_convert(b)
+
+    if size(a) != size(b)
+        @warn "images don't have the same size, difference will be Inf"
+        return Inf
+    end
+
+    approx_tile_size_px = 30
+
+    range_dim1 = round.(Int, range(0, size(a, 1), length = ceil(Int, size(a, 1) / approx_tile_size_px)))
+    range_dim2 = round.(Int, range(0, size(a, 2), length = ceil(Int, size(a, 2) / approx_tile_size_px)))
+
+    boundary_iter(boundaries) = zip(boundaries[1:end-1] .+ 1, boundaries[2:end])
+
+    _norm(rgb1::RGBf, rgb2::RGBf) = sqrt(sum(((rgb1.r - rgb2.r)^2, (rgb1.g - rgb2.g)^2, (rgb1.b - rgb2.b)^2)))
+    _norm(rgba1::RGBAf, rgba2::RGBAf) = sqrt(sum(((rgba1.r - rgba2.r)^2, (rgba1.g - rgba2.g)^2, (rgba1.b - rgba2.b)^2, (rgba1.alpha - rgba2.alpha)^2)))
+
+    # compute the difference score as the maximum of the mean squared differences over the color
+    # values of tiles over the image. using tiles is a simple way to increase the local sensitivity
+    # without directly going to pixel-based comparison
+    # it also makes the scores more comparable between reference images of different sizes, because the same
+    # local differences would be normed to different mean scores if the images have different numbers of pixels
+    return maximum(Iterators.product(boundary_iter(range_dim1), boundary_iter(range_dim2))) do ((mi1, ma1), (mi2, ma2))
+        @views mean(_norm.(a[mi1:ma1, mi2:ma2], b[mi1:ma1, mi2:ma2]))
+    end
 end
 
-function compare_media(a, b; sigma=[1,1])
-    file, ext = splitext(a)
+function compare_media(a::AbstractString, b::AbstractString)
+    _, ext = splitext(a)
     if ext in (".png", ".jpg", ".jpeg", ".JPEG", ".JPG")
         imga = load(a)
         imgb = load(b)
-        if size(imga) != size(imgb)
-            @warn "images don't have the same size, difference will be Inf"
-            return Inf
-        end
-        conv(x) = convert(Matrix{RGBf}, x)
-        return compare_media(conv(imga), conv(imgb), sigma=sigma)
+        return compare_images(imga, imgb)
     elseif ext in (".mp4", ".gif")
-        aframes, bframes = get_frames(a, b)
+        aframes = get_frames(a)
+        bframes = get_frames(b)
         # Frames can differ in length, which usually shouldn't be the case but can happen
         # when the implementation of record changes, or when the example changes its number of frames
         # In that case, we just return inf + warn
         if length(aframes) != length(bframes)
             @warn "not the same number of frames in video, difference will be Inf"
             return Inf
         end
-        return mean(compare_media.(aframes, bframes; sigma=sigma))
+        return maximum(compare_images.(aframes, bframes))
     else
         error("Unknown media extension: $ext")
     end
@@ -56,11 +81,16 @@ function get_all_relative_filepaths_recursively(dir)
     end
 end
 
-function record_comparison(base_folder::String; record_folder_name="recorded", reference_name = basename(base_folder), tag=last_major_version())
+function record_comparison(base_folder::String; record_folder_name="recorded", tag=last_major_version())
     record_folder = joinpath(base_folder, record_folder_name)
-    reference_folder = download_refimages(tag; name=reference_name)
+    @info "Downloading reference images"
+    reference_folder = download_refimages(tag)
     # we copy the reference images into the output folder, since we want to upload it all as an artifact, to know against what images we compared
-    cp(reference_folder, joinpath(base_folder, "reference"))
+    local_reference_copy_dir = joinpath(base_folder, "reference")
+    if isdir(local_reference_copy_dir)
+        rm(local_reference_copy_dir, recursive = true)
+    end
+    cp(reference_folder, local_reference_copy_dir)
     testimage_paths = get_all_relative_filepaths_recursively(record_folder)
     missing_refimages, scores = compare(testimage_paths, reference_folder, record_folder)