Skip to content

Conversation

@ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Oct 25, 2024

Description

Fixes https://discourse.julialang.org/t/makie-observable-bug/121724 - surface not correctly resizing to updated args.

In GLMakie instances was lfiting off of a Texture, which doesn't actually propagate updates. Now it uses plot[3], i.e. the converted z matrix Observable.

In WGLMakie a tessellated rect is generated for the surface plot. In a second step faces connected to nan-points are filtered out. This desynchronizes on update, causing an error. Fixed by excluding faces referring to out-of-bounds points.

The added refimg tests reducing and increasing size for range, vector and matrix x/y.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Added an entry in CHANGELOG.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@MakieBot
Copy link
Collaborator

MakieBot commented Oct 25, 2024

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running:

using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(fig)
using create display create display
GLMakie 6.18s (6.02, 6.35) 0.10+- 162.34ms (156.92, 170.78) 4.31+- 473.67ms (462.53, 497.50) 10.91+- 6.63ms (6.54, 6.92) 0.10+- 25.81ms (25.50, 26.31) 0.27+-
master 6.32s (6.18, 6.54) 0.11+- 163.94ms (157.86, 174.13) 4.19+- 472.78ms (463.26, 488.81) 7.12+- 6.76ms (6.62, 7.18) 0.12+- 25.87ms (25.43, 26.23) 0.28+-
evaluation 1.02x faster ✓, -0.15s (-1.42d, 0.00p, 0.11std) 1.01x invariant, -1.6ms (-0.38d, 0.24p, 4.25std) 1.00x invariant, 0.89ms (0.10d, 0.76p, 9.01std) 1.02x faster ✓, -0.13ms (-1.19d, 0.00p, 0.11std) 1.00x invariant, -0.06ms (-0.23d, 0.47p, 0.27std)
CairoMakie 6.07s (5.97, 6.20) 0.06+- 167.28ms (162.19, 182.72) 6.34+- 204.12ms (197.54, 221.01) 5.95+- 6.95ms (6.79, 7.33) 0.14+- 1.08ms (1.06, 1.12) 0.02+-
master 6.04s (5.99, 6.14) 0.04+- 166.22ms (160.97, 182.42) 5.24+- 202.10ms (197.17, 213.46) 3.78+- 6.96ms (6.79, 7.16) 0.11+- 1.11ms (1.07, 1.17) 0.03+-
evaluation 0.99x slower X, 0.03s (0.67d, 0.04p, 0.05std) 0.99x invariant, 1.06ms (0.18d, 0.57p, 5.79std) 0.99x invariant, 2.02ms (0.41d, 0.21p, 4.86std) 1.00x invariant, -0.01ms (-0.08d, 0.81p, 0.13std) 1.02x faster ✓, -0.03ms (-1.11d, 0.00p, 0.02std)
WGLMakie 6.87s (6.61, 7.19) 0.15+- 180.62ms (163.67, 227.39) 16.34+- 5.94s (5.72, 6.12) 0.11+- 11.12ms (10.68, 11.96) 0.31+- 151.52ms (145.97, 191.62) 9.52+-
master 6.87s (6.49, 7.08) 0.14+- 179.21ms (163.28, 196.73) 9.16+- 6.16s (5.89, 6.44) 0.15+- 11.32ms (10.32, 13.26) 0.59+- 148.91ms (146.63, 152.47) 1.47+-
evaluation 1.00x invariant, -0.0s (-0.03d, 0.92p, 0.14std) 0.99x invariant, 1.42ms (0.11d, 0.74p, 12.75std) 1.04x faster ✓, -0.23s (-1.73d, 0.00p, 0.13std) 1.02x invariant, -0.2ms (-0.43d, 0.18p, 0.45std) 0.98x invariant, 2.61ms (0.38d, 0.24p, 5.49std)

@ffreyer ffreyer closed this Oct 25, 2024
@ffreyer ffreyer reopened this Oct 25, 2024
@ffreyer ffreyer merged commit f0b3a86 into master Oct 25, 2024
37 of 38 checks passed
@ffreyer ffreyer deleted the ff/surface-update branch October 25, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

4 participants