Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework WGLMakie lines #3062

Merged
merged 41 commits into from
Aug 30, 2023
Merged

Rework WGLMakie lines #3062

merged 41 commits into from
Aug 30, 2023

Conversation

SimonDanisch
Copy link
Member

@SimonDanisch SimonDanisch commented Jul 13, 2023

This should also be a template for how we want to generate the plot objects in JS in the future, which should lead to a large TTFP improvement.
If I find the time to do the same treatment for all plot types in WGLMakie, it may just get the fastest TTFP of all backends, since it will basically just create dictionaries from plots.

This should also bring us closer to #2977, with a more comprehensive plotting API directly in JS.

@SimonDanisch SimonDanisch marked this pull request as draft July 13, 2023 11:13
@MakieBot
Copy link
Collaborator

MakieBot commented Jul 13, 2023

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(display(fig))
using create display create display
GLMakie 12.27s (11.74, 13.89) 0.80+- 1.10s (1.06, 1.26) 0.07+- 1.24s (1.16, 1.43) 0.09+- 10.59ms (10.37, 11.29) 0.32+- 150.17ms (145.25, 171.64) 9.52+-
master 12.16s (11.75, 13.81) 0.73+- 1.11s (1.04, 1.29) 0.08+- 1.26s (1.18, 1.41) 0.10+- 10.84ms (10.36, 12.23) 0.70+- 151.98ms (144.89, 172.07) 10.11+-
evaluation +0.92%, 0.11s invariant (0.15d, 0.79p, 0.77std) -1.53%, -0.02s invariant (-0.21d, 0.70p, 0.08std) -1.57%, -0.02s invariant (-0.20d, 0.72p, 0.10std) -2.37%, -0.25ms invariant (-0.46d, 0.41p, 0.51std) -1.20%, -1.81ms invariant (-0.18d, 0.74p, 9.82std)
CairoMakie 13.13s (12.98, 13.41) 0.14+- 1.37s (1.30, 1.45) 0.06+- 791.03ms (757.88, 864.86) 35.59+- 14.70ms (14.37, 15.68) 0.45+- 8.22ms (8.02, 8.36) 0.11+-
master 12.91s (12.65, 13.45) 0.26+- 1.33s (1.26, 1.41) 0.05+- 788.27ms (768.97, 817.15) 14.92+- 14.30ms (14.02, 14.41) 0.14+- 8.33ms (8.15, 8.53) 0.11+-
evaluation +1.70%, 0.22s invariant (1.07d, 0.07p, 0.20std) +2.74%, 0.04s invariant (0.66d, 0.24p, 0.06std) +0.35%, 2.77ms invariant (0.10d, 0.85p, 25.26std) +2.75%, 0.4ms invariant (1.21d, 0.06p, 0.30std) -1.32%, -0.11ms invariant (-0.97d, 0.10p, 0.11std)
WGLMakie 15.21s (15.03, 15.34) 0.12+- 1.36s (1.31, 1.42) 0.04+- 14.37s (14.18, 14.50) 0.13+- 16.39ms (15.40, 19.64) 1.51+- 1.19s (1.15, 1.22) 0.02+-
master 15.18s (14.98, 15.38) 0.13+- 1.34s (1.29, 1.37) 0.03+- 14.37s (14.15, 14.60) 0.19+- 15.87ms (15.43, 16.56) 0.41+- 1.15s (1.09, 1.20) 0.04+-
evaluation +0.17%, 0.03s invariant (0.22d, 0.69p, 0.12std) +1.83%, 0.02s invariant (0.69d, 0.22p, 0.04std) -0.04%, -0.01s invariant (-0.04d, 0.95p, 0.16std) +3.18%, 0.52ms invariant (0.47d, 0.41p, 0.96std) +3.54%, 0.04s slower X (1.24d, 0.04p, 0.03std)

@staticfloat
Copy link
Contributor

@SimonDanisch I just tried out the current state of the branch (b9e2bad) and it seems to have completely solved #3050! I can no longer trigger that kind of issue.

@SimonDanisch SimonDanisch changed the base branch from master to sd/beta-20 August 8, 2023 18:19
@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@SimonDanisch SimonDanisch force-pushed the sd/beta-20 branch 2 times, most recently from 010b27e to ee4f0cd Compare August 16, 2023 15:52
@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

Missing reference images

Found 8 new images without existing references.
Upload new reference images before merging this PR.

@SimonDanisch SimonDanisch marked this pull request as ready for review August 30, 2023 09:31
@SimonDanisch SimonDanisch merged commit bc0f611 into sd/beta-20 Aug 30, 2023
6 of 13 checks passed
@SimonDanisch SimonDanisch deleted the sd/wgl-lines branch August 30, 2023 09:31
@github-actions
Copy link
Contributor

Missing reference images

Found 8 new images without existing references.
Upload new reference images before merging this PR.

@ffreyer ffreyer mentioned this pull request Oct 28, 2023
16 tasks
SimonDanisch added a commit that referenced this pull request Nov 17, 2023
Continues #2831 !
Still needs to check, if I rebased correctly and didn't incorrectly
apply some of the changes!

## Merged PRs
- #2598
- #2746
- #2346
- #2544
- #3082
- #2868
- #3062
- #3106
- #3281
- #3246

## TODOS

- [x] fix flaky test `@test GLMakie.window_size(screen.glscreen) ==
scaled(screen, (W, H))`
- [x] Merge axis type inferences from #2220 
- [x] Test on different resolution screens, IJulia, Pluto, VSCode,
Windowed
- [x] rebase to only have merge commits from the PRs 
- [x] investigate unexpected speed ups
- [x] reset camera settings from tests
- [ ] check doc image generation
- [x] rethink default near/far in Camera3D (compatability with OIT)
- [x] merge #3246
- [x] fix WGLMakie issues/tests:
- [x] fix line depth issues (see tests: ~~hexbin colorrange~~ (not new),
LaTeXStrings in Axis3, Axis3 axis reversal)
  - [x] fix lighting of surface with nan points (fixed in #3246)
- ~~volume/3D contour artifacts (see 3D Contour with 2D contour
slices)~~ not new
  - ~~artifacting in "colorscale (lines)"~~ not new
- [x] GLMakie:
  - [x] slight outline in "scatter image markers" test
  - ~~clipping/z-fighting in "volume translated"~~ not new
- [x] CairoMakie:
  -  ~~Artfiacting in `colorscale (lines)"~~ not new
  - ~~markersize in "scatter rotations" changed?~~ not new
  - ~~color change in "colorscale (poly)"~~ not new
  - ~~transparency/render order of "OldAxis + Surface"~~ not new
  - ~~render order in "Merged color mesh"~~ not new
  - ~~render order of "Surface + wireframe + contour"~~ not new
- [x] Check "SpecApi in convert_arguments" (colors swapped?)


## Fixes the following errors

- fixes #2721 via #2746
- fixes #1600 via #2746
- fixes #1236 via #2746
- fixes MakieOrg/GeoMakie.jl#133 via #2598
- closes #2522
- closes #3239 via #3246
- fixes #3238 via #3246
- fixes #2985 via #3246
- fixes #3307 via #3281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants