-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
Fix some errors caused by updating Axis xscale or yscale #3084
Conversation
Compile Times benchmarkNote, 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))
|
Alternatively to this we could also lower the priority of limit updates in axis etc. Not sure what's better here. |
ok we do need this...
@jkrumbiegel @ffreyer can we merge this? |
This was ready on my side. I thought that Julius might have some edge cases in mind related to the changes in the update order in Axis. If not we can go ahead and merge I'd say. |
Description
Old Text
This should make the default behavior that transform_func is passed down to child scenes and plots first, before calculations happen.
This fixes #3065, but I haven't tested if it breaks anything else yet.
Side note - Why doesn't
map
acceptpriority
? And copy, given that it links to the original?This pr tweaks some behavior around axis scales/transform_funcs to avoid mostly limit related errors.
Changes:
priority
kwarg to our map, map! and lift implementations (Observables is still missing this)ax.xscale
orax.yscale
. This allows barplot to fix its bars before the axis checks them for limits. Basically it makes add smart fillto to enable logscale y-axis histogram barplot #3004 work with dynamic transform_func/scale changes.ax.scene.transformation.transform_func
, then axis camera matrices, then plotsax.scene.transformation.transform_func
first, then plots, then limits, then axis camera (order enforced by priority)Transformation(parent)
now has an overwrite fortransform_func
hvlines
andhvspan
no longer back-transform their relative limits to input space. Instead they apply their transform_func to their input data and remove it from their child plots. So rather than plotting to input space they now plot to transformed space. This fixes vlines! throws error when changing yscale from log10 back to identity #3065barplot
now runscalculate_bars!
at priority 1 to make sure it runs before transform_func is applied in its child plot(s). This shouldn't be necessary, but I left it in for now.errorbar
andrangebar
changed from using an array of pairs to just a flat array for its linesegments. This avoid a conversion error fromVector
toReinterpretedArray
.I have tested these changes with
Type of change
Checklist