Open
Conversation
Collaborator
Benchmark ResultsSHA: 2efd32737c3deee42303ac204b07ae1687b2a6fe Warning These results are subject to substantial noise because GitHub's CI runs on shared machines that are not ideally suited for benchmarking. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Leftover Observables
Translates
Axisto compute graph, includingLineAxis. There are some exceptions that still use Observables to interface with other Observable structures:layoutobservables.computedbboxandscene.viewportfeed into the compute graph to calculategraph.sceneareawithgraph.finallimitsandgraph.aspectwhich then feeds back intoscene.viewportgraph.layout_protrusionfeeds intolayoutobservables.protrusionsgraph.xscale,graph.yscaleindirectly updatescene.transform_func(irrelevant)graph.sharedxlimitsandgraph.sharedylimitseach have an observable output to force them to evaluate whenever their inputs change. See (1)graph.projectionmatrixfeeds intoscene.camera.projectionviewetc which then feeds intoscene.compute. We could probably update the Observable as a side effect of pulling camera matrices into backends without issues?LineAxis: ticklabel boundingboxes feed into an Observable to feed back into a graph input that's used beforehand. This might just be and ordering issue?LineAxis
For the most part this is just a translation from Observables to compute graph. Some things got moved around, some split up, some defaults moved to
generic_plot_attributes()which should all be irrelevant. In terms relevant changes:boundingbox()were replaced withraw_string_boundingboxes, which completely ignore projections, positions and offset. This should be fine since all these calls only care aboutwidths(bb)anywayax.(x/y)axis.(...)ComputeGraph
graph.output -> Observable -> ... -> graph.input -> ... graph.outputloops could discard other Observable updatesforce_updatefield toInputto allow propagation of equal values (2)ExplicitUpdatewrapper to control the propagation of an update from the outside. Updates can be forced or denied this way (or use the default rules)set_type!()for initializing just the type of a compute node (similar tounsafe_init!()Axis Limits
Axis limits were difficult to translate because there are a lot of different things that need to work:
autolimits!()requires(nothing, nothing)to propagate up to the point where limits are determined from the plots in the Axis. This requires at leastforce_update(2), even if we kept using Observables immediately afterax.limits.finallimitsto update no later thanxscaleoryscaleto avoid applying e.g.log10to limits outside the domain oflog10. Since Observables pulled from a compute graph only update after all edges evaluate, i.e. after ticks are calculated with the updated(x/y)scale, the whole path from(x/y)scaletofinallimitsmust be compute nodes.ax.limitsbut something later down the line to preserve user set limits forreset_limits!()and theLimitResetinteractionxlims!()andylims!()must be to update one dimension ofax.limitswithout affecting any limits in the other dimension. This means they need to be able to deny an update in one dimension, which I addedExplicitUpdate(3) for.The way I set things up is like this:
ax.(x/y)scalecomputetransform_funcwhich setsscene.transformations.transform_funcas a side effectax.limitsis marked to always propagateax.limitssplits intolocalxlimitsandlocalylimitstogether withtransform_func,(x/y)autolimitmarginand_limit_update_rule. This step generates numeric limits based on the limits given to this axis, the bounding boxes of its plots and thedefaultlimits()of the(x/y)scale(which is intransform_func). By default the resulting x and y limits will be wrapped inExplicitUpdate(lims, :force)so they always make it to the next step. Ifxlims!()orylims!()are called_update_limit_ruleis updated alongside the limits, which will cause one of the limits to not propagate.local(x/y)limitsfeeds intoshared(x/y)limits. This computation validates the limits and sets theshared(x/y)limitsof every linked axis. This node is also backed by aComputePipeline.get_observable!()to force the linking code to run asap. Note that this node can desync with local(x/y)limits through updates of a linked axis. That's why local limits need to always update. Also note that updating local limits through linking would cause an update loop/recursion.shared(x/y)limitsrecombine intotargetlimitstargetlimitsmap tofinallimitsviaadjustlimits()like before, includingviewport,autolimitaspectand(x/y)autolimitmarginfinallimitsmap toprojectionmatrixlike beforeInteractions must now read from
shared(x/y)limitsor later (e.g. targetlimits is also fine), and input intolocal(x/y)limitsto work correctly with linked axes. The former is basically "limits with linking" and the latter is the input that gets communicated to linked axes. Note that this is affects everything that previously settargetlimits.Other Changes
automaticnot being considered inget_label_suffixfor DynamicQuantitiestargetlimitsandfinallimitsas fields fromAxis. This can still be accessed asax.targetlimits/finallimitsbut now return compute nodesblock_limit_linkingfrom Axis as it's not neededmark_dirty!()which requiresmark_resolved!()to resolve all parent edgesTODO:
LineAxistypeLineAxiscode to avoid intermediate ObservableExplicitUpdateandinput.force_updateExplicitUpdateandinput.force_updateType of change
Checklist