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

Force colorbuffer to use the size of the screen, not the scene #2012

Closed
wants to merge 18 commits into from

Conversation

asinghvi17
Copy link
Member

@asinghvi17 asinghvi17 commented May 31, 2022

Description

This also changes the VideoStream constructor similarly, allowing the user to use patterns like this:

CairoMakie.activate!(px_per_unit=3)

record(fig, "streamplot_anim_keener.mp4") do io
    ...
end

This works with GLMakie and CairoMakie, but will probably break WGLMakie since there seems to be no GeometryBasics.width definitions for either WGLDisplay or ThreeDisplay, and I'm not sure how to go about implementing those since the display structs don't contain their Scenes directly.

I built this off my other PR which accelerates recording speed in the presence of rasterized plots, but will rebase once that is merged.

Type of change

Delete options that do not apply:

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation

This allows arbitrary scaling in CairoMakie, remains constant in GLMakie, and probably breaks WGLMakie.
@asinghvi17 asinghvi17 changed the base branch from master to as/faster_cairo_png May 31, 2022 22:41
Base automatically changed from as/faster_cairo_png to master June 3, 2022 10:35
src/display.jl Outdated Show resolved Hide resolved
@MakieOrg MakieOrg deleted a comment from MakieBot Jul 11, 2022
@MakieBot
Copy link
Collaborator

MakieBot commented Jul 11, 2022

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 20.13s (19.93, 20.37) 0.16+- 22.14s (21.56, 22.82) 0.54+- 20.38s (19.75, 21.02) 0.51+- 19.19ms (18.21, 19.73) 0.52+- 117.74ms (112.65, 119.96) 2.67+-
master 19.99s (19.49, 20.34) 0.37+- 22.21s (21.60, 22.64) 0.39+- 20.24s (19.78, 20.71) 0.37+- 19.36ms (18.88, 19.83) 0.35+- 119.11ms (116.92, 121.57) 1.57+-
evaluation +0.71%, 0.14s invariant (0.51d, 0.37p, 0.26std) -0.35%, -0.08s invariant (-0.17d, 0.76p, 0.46std) +0.72%, 0.15s invariant (0.33d, 0.55p, 0.44std) -0.90%, -0.17ms invariant (-0.39d, 0.48p, 0.44std) -1.17%, -1.38ms invariant (-0.63d, 0.27p, 2.12std)
CairoMakie 12.65s (12.48, 12.92) 0.15+- 19.35s (18.80, 20.05) 0.39+- 3.15s (3.08, 3.23) 0.06+- 15.84ms (15.47, 16.22) 0.26+- 26.39ms (25.70, 29.51) 1.38+-
master 12.73s (12.54, 12.91) 0.12+- 19.34s (18.89, 19.66) 0.26+- 3.10s (3.03, 3.16) 0.04+- 15.75ms (15.33, 16.08) 0.26+- 25.97ms (25.73, 26.64) 0.31+-
evaluation -0.69%, -0.09s invariant (-0.64d, 0.25p, 0.14std) +0.04%, 0.01s invariant (0.02d, 0.96p, 0.32std) +1.66%, 0.05s invariant (1.06d, 0.07p, 0.05std) +0.58%, 0.09ms invariant (0.36d, 0.52p, 0.26std) +1.59%, 0.42ms invariant (0.42d, 0.46p, 0.85std)
WGLMakie 20.65s (19.81, 22.64) 1.05+- 27.98s (26.86, 30.38) 1.51+- 45.55s (43.69, 49.51) 2.28+- 23.45ms (22.45, 24.87) 0.98+- 1.77s (1.71, 1.83) 0.05+-
master 20.76s (20.08, 22.30) 0.95+- 27.83s (27.05, 29.30) 0.99+- 45.28s (43.82, 48.22) 1.89+- 24.10ms (22.14, 25.98) 1.55+- 1.78s (1.70, 1.83) 0.04+-
evaluation -0.49%, -0.1s invariant (-0.10d, 0.85p, 1.00std) +0.53%, 0.15s invariant (0.12d, 0.83p, 1.25std) +0.59%, 0.27s invariant (0.13d, 0.81p, 2.08std) -2.74%, -0.64ms invariant (-0.50d, 0.38p, 1.27std) -0.58%, -0.01s invariant (-0.22d, 0.68p, 0.05std)

@jkrumbiegel
Copy link
Member

I don't really understand where in the code the px_per_unit attribute is picked up by colorbuffer, is it possible to pass it to record as well, just like you can do it for save? That's what I'd want, I mainly see the hidden state as a convenience while plotting, but it should always be directly override-able.

@asinghvi17
Copy link
Member Author

Hmm, that's a good idea - might require a bit of refactor but will do that.

The px_per_unit is in this case read from the backend in backend_show, and in order to pass it on I'll refactor backend_display to pass some kwargs on to backend_show through an IOContext.

- Recording and colorbuffers use screen size instead of scene size, allowing e.g. the `px_per_unit` argument in CairoMakie to take effect.
It is sufficient to set the global `px_per_unit` setting by calling `CairoMakie.activate!(px_per_unit=3)` or whichever scaling factor is desired.
- Recording and colorbuffers use screen size instead of scene size, allowing e.g. the `px_per_unit` argument in CairoMakie to take effect.
It is sufficient to set the global `px_per_unit` setting by calling `CairoMakie.activate!(px_per_unit=3)` or whichever scaling factor is desired.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a change in behavior? I'm not sure if the CairoMakie.activate!(... settings were proliferated to save before. Do we want them to or not?

@SimonDanisch
Copy link
Member

closed in favor of #2306

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.

4 participants