From 21a78c8dd0615c1374bd398d13ebad432d1311b6 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 9 Oct 2024 16:35:32 +0200 Subject: [PATCH 01/10] Fix RPR examples (#4462) * Fix first RPR example * fix color palette --- docs/src/explanations/backends/rprmakie.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/src/explanations/backends/rprmakie.md b/docs/src/explanations/backends/rprmakie.md index 3b34bcb96b4..e697f59db95 100644 --- a/docs/src/explanations/backends/rprmakie.md +++ b/docs/src/explanations/backends/rprmakie.md @@ -59,8 +59,8 @@ mesh!(ax, Sphere(Point3f(0), 1), material=mat) image = colorbuffer(screen)::Matrix{RGB{N0f8}} # Replace a specific (sub) LScene with RPR, and display the whole scene interactively in RPRMakie using RPRMakie -refres = Observable(nothing) # Optional observable that triggers -RPRMakie.activate!(); display(fig) # Make sure to display scene first in RPRMakie +refresh = Observable(nothing) # Optional observable that triggers rerendering +display(ax.scene; backend=GLMakie) # Make sure to display scene first in GLMakie # Replace the scene with an interactively rendered RPR output. # See more about this in the RPRMakie interop example context, task = RPRMakie.replace_scene_rpr!(ax.scene, screen; refresh=refresh) @@ -105,7 +105,7 @@ materials = [glass chrome; emissive plastic] mesh!(ax, load(Makie.assetpath("matball_floor.obj")); color=:white) -palette = reshape(Makie.default_palettes.color[][1:6], size(materials)) +palette = reshape(Makie.wong_colors()[1:6], size(materials)) for i in CartesianIndices(materials) x, y = Tuple(i) From 2116cfdceb0f7bc18027bd92f79f3d0442c30ec3 Mon Sep 17 00:00:00 2001 From: Frederic Freyer Date: Thu, 10 Oct 2024 14:41:56 +0200 Subject: [PATCH 02/10] More improvements for ReferenceUpdater (#4431) * use latest merge run if multiple are present * track missing recordings * rename test for testing * fix tracking of missing recordings * add missing refimage removal * group old & new reference images, add some page documentation * use latest refimages for update * add toggle-all for new and missing refence images * undo test rename * remove console.log [skip ci] --- .github/workflows/reference_tests.yml | 3 +- CairoMakie/test/runtests.jl | 2 +- GLMakie/test/runtests.jl | 2 +- ReferenceTests/src/database.jl | 5 + ReferenceTests/src/runtests.jl | 22 ++- ReferenceUpdater/Project.toml | 1 + ReferenceUpdater/src/ReferenceUpdater.jl | 1 + ReferenceUpdater/src/image_download.jl | 10 ++ ReferenceUpdater/src/local_server.jl | 83 +++++++++-- ReferenceUpdater/src/reference_images.html | 154 ++++++++++++++++++--- WGLMakie/test/runtests.jl | 2 +- 11 files changed, 246 insertions(+), 39 deletions(-) diff --git a/.github/workflows/reference_tests.yml b/.github/workflows/reference_tests.yml index 07b60c28037..654ec9f2881 100644 --- a/.github/workflows/reference_tests.yml +++ b/.github/workflows/reference_tests.yml @@ -198,9 +198,10 @@ jobs: # 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 + # Concatenate scores.tsv, new_files.txt and missing_files.txt cat "${baseDir}/${dir}/scores.tsv" >> "./ReferenceImagesCombined/scores.tsv" cat "${baseDir}/${dir}/new_files.txt" >> "./ReferenceImagesCombined/new_files.txt" + cat "${baseDir}/${dir}/missing_files.txt" >> "./ReferenceImagesCombined/missing_files.txt" # Copy recorded folder mkdir -p "./ReferenceImagesCombined/recorded/${dir}/" diff --git a/CairoMakie/test/runtests.jl b/CairoMakie/test/runtests.jl index 2b0528ca431..b131a1dff32 100644 --- a/CairoMakie/test/runtests.jl +++ b/CairoMakie/test/runtests.jl @@ -193,7 +193,7 @@ functions = [:volume, :volume!, :uv_mesh] CairoMakie.activate!(type = "png", px_per_unit = 1) ReferenceTests.mark_broken_tests(excludes, functions=functions) recorded_files, recording_dir = @include_reference_tests CairoMakie "refimages.jl" - missing_images, scores = ReferenceTests.record_comparison(recording_dir) + missing_images, scores = ReferenceTests.record_comparison(recording_dir, "CairoMakie") ReferenceTests.test_comparison(scores; threshold = 0.05) end diff --git a/GLMakie/test/runtests.jl b/GLMakie/test/runtests.jl index 9bb97615171..2aa9d1d5765 100644 --- a/GLMakie/test/runtests.jl +++ b/GLMakie/test/runtests.jl @@ -30,7 +30,7 @@ include("unit_tests.jl") @testset "refimages" begin ReferenceTests.mark_broken_tests() recorded_files, recording_dir = @include_reference_tests GLMakie "refimages.jl" joinpath(@__DIR__, "glmakie_refimages.jl") - missing_images, scores = ReferenceTests.record_comparison(recording_dir) + missing_images, scores = ReferenceTests.record_comparison(recording_dir, "GLMakie") ReferenceTests.test_comparison(scores; threshold = 0.05) end diff --git a/ReferenceTests/src/database.jl b/ReferenceTests/src/database.jl index 49c5bafdbf0..6bb2b835fd7 100644 --- a/ReferenceTests/src/database.jl +++ b/ReferenceTests/src/database.jl @@ -17,6 +17,7 @@ const RECORDING_DIR = Base.RefValue{String}() const SKIP_TITLES = Set{String}() const SKIP_FUNCTIONS = Set{Symbol}() const COUNTER = Ref(0) +const SKIPPED_NAMES = Set{String}() # names skipped due to title exclusion or function exclusion """ @reference_test(name, code) @@ -32,6 +33,7 @@ macro reference_test(name, code) @testset $(title) begin if $skip @test_broken false + mark_skipped!($title) else t1 = time() if $title in $REGISTERED_TESTS @@ -84,10 +86,13 @@ end function mark_broken_tests(title_excludes = []; functions=[]) empty!(SKIP_TITLES) empty!(SKIP_FUNCTIONS) + empty!(SKIPPED_NAMES) union!(SKIP_TITLES, title_excludes) union!(SKIP_FUNCTIONS, functions) end +mark_skipped!(name::String) = push!(SKIPPED_NAMES, name) + macro include_reference_tests(backend::Symbol, path, paths...) toplevel_folder = dirname(string(__source__.file)) return esc(quote diff --git a/ReferenceTests/src/runtests.jl b/ReferenceTests/src/runtests.jl index cb0d0672322..52a689289d0 100644 --- a/ReferenceTests/src/runtests.jl +++ b/ReferenceTests/src/runtests.jl @@ -81,7 +81,7 @@ function get_all_relative_filepaths_recursively(dir) end end -function record_comparison(base_folder::String; record_folder_name="recorded", tag=last_major_version()) +function record_comparison(base_folder::String, backend::String; record_folder_name="recorded", tag=last_major_version()) record_folder = joinpath(base_folder, record_folder_name) @info "Downloading reference images" reference_folder = download_refimages(tag) @@ -99,6 +99,19 @@ function record_comparison(base_folder::String; record_folder_name="recorded", t println(file, path) end end + + open(joinpath(base_folder, "missing_files.txt"), "w") do file + backend_ref_dir = joinpath(reference_folder, backend) + recorded_paths = mapreduce(vcat, walkdir(backend_ref_dir)) do (root, dirs, files) + relpath.(joinpath.(root, files), reference_folder) + end + skipped = Set([joinpath(backend, "$name.png") for name in SKIPPED_NAMES]) + missing_recordings = setdiff(Set(recorded_paths), Set(testimage_paths), skipped) + + for path in missing_recordings + println(file, path) + end + end open(joinpath(base_folder, "scores.tsv"), "w") do file paths_scores = sort(collect(pairs(scores)), by = last, rev = true) @@ -120,7 +133,12 @@ function test_comparison(scores; threshold) end end -function compare(relative_test_paths::Vector{String}, reference_dir::String, record_dir; o_refdir=reference_dir, missing_refimages=String[], scores=Dict{String,Float64}()) +function compare( + relative_test_paths::Vector{String}, reference_dir::String, record_dir; + o_refdir = reference_dir, missing_refimages = String[], + scores = Dict{String,Float64}() + ) + for relative_test_path in relative_test_paths ref_path = joinpath(reference_dir, relative_test_path) rec_path = joinpath(record_dir, relative_test_path) diff --git a/ReferenceUpdater/Project.toml b/ReferenceUpdater/Project.toml index a8d2097449d..59313979845 100644 --- a/ReferenceUpdater/Project.toml +++ b/ReferenceUpdater/Project.toml @@ -4,6 +4,7 @@ authors = ["Julius Krumbiegel "] version = "0.1.0" [deps] +Dates = "ade2ca70-3891-5945-98fb-dc099432e06a" Downloads = "f43a241f-c20a-4ad4-852c-f6b1247861c6" HTTP = "cd3eb016-35fb-5094-929b-558a96fad6f3" JSON3 = "0f8b85d8-7281-11e9-16c2-39a750bddbf1" diff --git a/ReferenceUpdater/src/ReferenceUpdater.jl b/ReferenceUpdater/src/ReferenceUpdater.jl index 6acf2060e64..2be2461db16 100644 --- a/ReferenceUpdater/src/ReferenceUpdater.jl +++ b/ReferenceUpdater/src/ReferenceUpdater.jl @@ -8,6 +8,7 @@ import JSON3 import ZipFile import REPL import TOML +using Dates function github_token() get(ENV, "GITHUB_TOKEN") do diff --git a/ReferenceUpdater/src/image_download.jl b/ReferenceUpdater/src/image_download.jl index 063edf9860b..bc3098a0faa 100644 --- a/ReferenceUpdater/src/image_download.jl +++ b/ReferenceUpdater/src/image_download.jl @@ -18,3 +18,13 @@ function upload_reference_images(path=basedir("recorded"), tag=last_major_versio upload_release("MakieOrg", "Makie.jl", github_token(), tag, tarfile) end end + +function download_refimages(tag=last_major_version()) + url = "https://github.com/MakieOrg/Makie.jl/releases/download/$(tag)/reference_images.tar" + images_tar = Downloads.download(url) + images = tempname() + isdir(images) && rm(images, recursive=true, force=true) + Tar.extract(images_tar, images) + rm(images_tar) + return images +end diff --git a/ReferenceUpdater/src/local_server.jl b/ReferenceUpdater/src/local_server.jl index 44fb0dbaf3d..1c4d3ec6b9e 100644 --- a/ReferenceUpdater/src/local_server.jl +++ b/ReferenceUpdater/src/local_server.jl @@ -12,24 +12,28 @@ function serve_update_page_from_dir(folder) folder = realpath(folder) @assert isdir(folder) "$folder is not a valid directory." - split_scores(folder) + group_scores(folder) + group_files(folder, "new_files.txt", "new_files_grouped.txt") + group_files(folder, "missing_files.txt", "missing_files_grouped.txt") + router = HTTP.Router() function receive_update(req) data = JSON3.read(req.body) - images = data["images"] + images_to_update = data["images_to_update"] + images_to_delete = data["images_to_delete"] tag = data["tag"] - tempdir = tempname() recorded_folder = joinpath(folder, "recorded") - reference_folder = joinpath(folder, "reference") - @info "Copying reference folder to \"$tempdir\"" - cp(reference_folder, tempdir) + @info "Downloading latest reference folder for $tag" + tempdir = download_refimages(tag) + + @info "Updating files in $tempdir" - for image in images - @info "Overwriting \"$image\" in new reference folder" + for image in images_to_update + @info "Overwriting or adding $image" copy_filepath = joinpath(tempdir, image) copy_dir = splitdir(copy_filepath)[1] # make the path in case a new refimage is in a not yet existing folder @@ -37,6 +41,16 @@ function serve_update_page_from_dir(folder) cp(joinpath(recorded_folder, image), copy_filepath, force = true) end + for image in images_to_delete + @info "Deleting $image" + copy_filepath = joinpath(tempdir, image) + if isfile(copy_filepath) + rm(copy_filepath, recursive = true) + else + @warn "Cannot delete $image - it has already been deleted." + end + end + @info "Uploading updated reference images under tag \"$tag\"" try upload_reference_images(tempdir, tag) @@ -121,14 +135,20 @@ function serve_update_page(; commit = nothing, pr = nothing) return false end end + if isempty(checkruns) error("\"Merge artifacts\" run is not available.") end if length(checkruns) > 1 - error("Found multiple checkruns for \"Merge artifacts\", this is unexpected.") - end - + datetimes = map(checkruns) do checkrun + DateTime(checkrun["completed_at"], dateformat"y-m-dTH:M:SZ") + end + datetime, idx = findmax(datetimes) + @warn("Found multiple checkruns for \"Merge artifacts\". Using latest with timestamp: $datetime") + check = checkruns[idx] + else check = only(checkruns) + end job = JSON3.read(authget("https://api.github.com/repos/MakieOrg/Makie.jl/actions/jobs/$(check["id"])").body) run = JSON3.read(authget(job["run_url"]).body) @@ -185,7 +205,7 @@ function unzip(file, exdir = "") end -function split_scores(path) +function group_scores(path) isfile(joinpath(path, "scores_table.tsv")) && return # Load all refimg scores into a Dict @@ -231,3 +251,42 @@ function split_scores(path) return end + +function group_files(path, input_filename, output_filename) + isfile(joinpath(path, output_filename)) && return + + # Group files in new_files/missing_files into a table like layout: + # GLMakie CairoMakie WGLMakie + + # collect refimg names and which backends they exist for + data = Dict{String, Vector{Bool}}() + open(joinpath(path, input_filename), "r") do file + for filepath in eachline(file) + pieces = split(filepath, '/') + backend = pieces[1] + if !(backend in ("GLMakie", "CairoMakie", "WGLMakie")) + error("Failed to parse backend in \"$line\", got \"$backend\"") + end + + filename = join(pieces[2:end], '/') + exists = get!(data, filename, [false, false, false]) + + exists[1] |= backend == "GLMakie" + exists[2] |= backend == "CairoMakie" + exists[3] |= backend == "WGLMakie" + end + end + + # generate new structed file + open(joinpath(path, output_filename), "w") do file + for (filename, valid) in data + println(file, + ifelse(valid[1], "GLMakie/$filename", "INVALID"), '\t', + ifelse(valid[2], "CairoMakie/$filename", "INVALID"), '\t', + ifelse(valid[3], "WGLMakie/$filename", "INVALID") + ) + end + end + + return +end diff --git a/ReferenceUpdater/src/reference_images.html b/ReferenceUpdater/src/reference_images.html index 39181a9aef3..fc58c9739a8 100644 --- a/ReferenceUpdater/src/reference_images.html +++ b/ReferenceUpdater/src/reference_images.html @@ -34,9 +34,31 @@

Reference images

+

+
    +
+

New images without references

+ The selected CI run produced an image for which no reference image exists. + Selected images will be added as new reference images. +

+ Toggle All
+ +

Old reference images without recordings

+ The selected CI run did not produce an image, but a reference image exists. + This implies that a reference test was deleted or renamed. + Selected images will be deleted from the reference images. +

+ Toggle All
+
+

Images with references

+ This is the normal case where the selected CI run produced an image and the reference image exists. + Each row shows one image per backend from the same reference image test, which can be compared with its reference image. + Rows are sorted based on the maximum row score (bigger = more different). + Red cells fail CI (assuming the thresholds are up to date), yellow cells may but likely don't have signficant visual difference and gray cells are visually equivalent. +

@@ -111,53 +133,137 @@

Images with references

}) }) - fetch('new_files.txt') + fetch('new_files_grouped.txt') .then(response => response.text()) .then(data => { di = document.querySelector("#new-images-list") - data.split(/\r?\n/).forEach(path => { - if (path == ""){ + data.split(/\r?\n/).forEach(line => { + if (line == ""){ return } + parts = line.split('\t') - div = document.createElement("div") - di.append(div) - div.innerHTML = ` - - ${path} - ` - if (path.endsWith(".png")){ - div.innerHTML += `` - } else if (path.endsWith(".mp4")){ - div.innerHTML += `