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

Bounding Box Prototyping #3540

Closed
wants to merge 3 commits into from
Closed

Bounding Box Prototyping #3540

wants to merge 3 commits into from

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Jan 8, 2024

Description

I wanted to put some of my thoughts on bounding boxes into writing/code.

What uses bounding boxes?

  • Axis, Axis3, PolarAxis, axis3d! use bounding boxes for limits, ticks
  • Axis, Axis3, PolarAxis, Scene use bounding boxes for camera placement
  • PolarAxis and more indirectly Axis, Axis3 and Scene use bounding boxes for clipping

Other uses/influences:

  • OIT uses depth as weight, so it works better if the depth range is filled completely, i.e. if near and far are tightly picked. It would benefit from tracking scene bounding boxes to correctly select near and far
  • shadow mapping would benefit from scene bounding boxes to more easily generate different camera perspectives
  • (full) ray based picking would require accurate bounding boxes to quickly discard missed plots and sort potential hits
  • collisions would require them for the same

Since limits are used for plotted data pretty much everywhere we should just have a common scene bounding box, derived from plot bounding boxes.

About coordinate spaces

Generally bounding boxes are needed in the space their data is in:

  • limits (Axis, Axis3, PolarAxis, axis3d) require data space bounding boxes
  • layouting (e.g. tick and axis label spacing) requires pixel space bounding boxes of text placed in pixel space (PolarAxis placed ticks in data space but skips the conversion for layouting. Axis3 maybe does something similar?)

Some require transformations down the pipeline:

  • cameras may require world space bounding boxes if not controlled via an Axis
  • shadow mapping would require world space bounding boxes

Moving up the pipeline is less clear:

  • a :pixel or :relative space text annotation in an Axis should not affect its bounding box / limits
  • a plot without a transform_func in and a Scene with one should maybe influence limits

Plots should probably have space conversion for their bounding boxes. Scenes should either filter (e.g. only consider :data space plots for a :data space bounding box) or transform only :data, :transformed and :world space into each other as data in those spaces depend on things the scene (may) control.

About accurate bounding boxes / marker bounding boxes

  • some cases require the inclusion of marker bboxes i.e. including the size of a scattered marker or mesh (e.g. ray based picking, collisions)
  • most care more about speed but would still like them to be included (limits, camera placement, clipping)
  • none need marker bounding boxes to be excluded

So I think we want a fast and an accurate version of boundingbox(). Fast can include marker bounding boxes if they are cheap, while accurate always includes them.

I'm not sure how fast the fast version should be/needs to be. E.g. should bounding boxes in other spaces just transform the data space bounding box or transform data and then calculate a new bounding box?

About caching / Lazy bounding boxes

Bounding Boxes are generally not something we need to process asap

  • limits and cameras don't update on bbox changes, only on plot insertions
  • layouting does react immediately and may need to do multiple round trips... if not it only needs to react up to once per (potential) frame
  • shadow mapping would require once per frame
  • ray based picking and DataInspector (e.g. for mesh) are on demand
  • collisions would be once per time step

And also something we need regardless of whether they changed

  • layouting may require bounding boxes of unchanged objects when other things change (other plots, scene resolution) (?)
  • camera may need bounding box again to keep clipping correct when zooming
  • shadow mapping, DataInspector, ray based picking and collisions all need a bounding box regardless of whether it changed

To avoid lots of redundant calculations bounding boxes should be cached and lazy. It would be nice to have an on_bounding_box_change though.

Other Notes

  • Some cases only care about marker bounding boxes (i.e. position argument ignored). E.g. DataInspector for meshscatter, maybe layouting for ticks. (May apply to: scatter, text, meshscatter, linesegments, voxels, arrows, ...)
  • Many bboxes are only 2d. I think this can be handled by just setting the z values to NaN
  • vlines (...) should not result in y (...) limits changes in Axis etc. Imo this should not discard a dimension of the bounding box, as that dimension still affects the bounds of the plot/visualization. This should be handled by the Axis instead. Maybe that dimension should be Inf, if that's not impractical...
  • Adding plot.transformed = apply_transform(transform_func(plot), plot.converted) would give us cheaper/more accurate bounding boxes in :transformed space

Summary of Goals

  • add cached bounding boxes to plots and scenes
  • make updates of bounding boxes lazy/on demand
  • offer a fast and an accurate boundingbox(obj, mode = :fast/:accurate)
  • deprecate data_limits for fast bounding boxes
  • figure out how to handle spaces, maybe:
    • have boundingbox(plot, space) transform to the given space
    • have boundingbox(scene, space) only regard plots in that space
    • or have boudningbox(scene, space) transform plot bboxes to the given space, but only for :data, :transformed and :world space plots (ignoring :pixel, :relative and :clip space plots and returning invalid bbox if the passed space is one of those)
  • offer bounding boxes for markers (w/o position applied), maybe just internally
  • offer bounding boxes of specific elements boundingbox(obj, index)
  • maybe offer some utility for reacting to changes of the bounding box

Type of change

Delete options that do not apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • Added an entry in NEWS.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 Jan 8, 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 3.47s (3.42, 3.55) 0.06+- 434.07ms (406.73, 477.70) 25.53+- 479.85ms (466.24, 524.44) 20.29+- 7.70ms (7.44, 7.93) 0.22+- 25.68ms (25.32, 26.75) 0.54+-
master 3.51s (3.41, 3.73) 0.14+- 414.86ms (397.26, 455.91) 24.39+- 480.95ms (467.28, 518.16) 20.94+- 7.31ms (7.15, 7.55) 0.16+- 25.91ms (25.28, 26.57) 0.52+-
evaluation 1.01x invariant, -0.04s (-0.33d, 0.55p, 0.10std) 0.96x invariant, 19.2ms (0.77d, 0.18p, 24.96std) 1.00x invariant, -1.09ms (-0.05d, 0.92p, 20.61std) 0.95x slower❌, 0.39ms (2.02d, 0.00p, 0.19std) 1.01x invariant, -0.23ms (-0.43d, 0.44p, 0.53std)
CairoMakie 3.12s (3.04, 3.21) 0.06+- 356.02ms (342.86, 372.12) 11.62+- 144.79ms (138.40, 152.00) 5.41+- 7.86ms (7.62, 8.10) 0.20+- 605.44μs (599.88, 615.30) 6.14+-
master 3.12s (3.04, 3.20) 0.06+- 333.67ms (318.22, 347.76) 13.39+- 145.17ms (139.00, 150.53) 4.89+- 7.50ms (7.25, 7.74) 0.22+- 605.24μs (596.76, 612.85) 6.25+-
evaluation 1.00x invariant, 0.0s (0.00d, 0.99p, 0.06std) 0.94x slower❌, 22.35ms (1.78d, 0.01p, 12.50std) 1.00x invariant, -0.38ms (-0.07d, 0.89p, 5.15std) 0.95x slower X, 0.37ms (1.77d, 0.01p, 0.21std) 1.00x invariant, 0.19μs (0.03d, 0.95p, 6.19std)
WGLMakie 3.85s (3.80, 3.97) 0.06+- 367.09ms (353.54, 393.09) 12.47+- 9.42s (9.24, 9.66) 0.16+- 10.19ms (10.04, 10.47) 0.16+- 71.27ms (68.12, 78.31) 3.91+-
master 3.86s (3.81, 3.90) 0.03+- 343.76ms (333.73, 353.84) 6.80+- 9.37s (9.23, 9.57) 0.11+- 9.98ms (9.43, 11.81) 0.82+- 72.02ms (68.74, 79.66) 4.07+-
evaluation 1.00x invariant, -0.01s (-0.21d, 0.70p, 0.04std) 0.94x slower❌, 23.33ms (2.32d, 0.00p, 9.64std) 0.99x invariant, 0.05s (0.36d, 0.52p, 0.14std) 0.98x invariant, 0.21ms (0.36d, 0.52p, 0.49std) 1.01x invariant, -0.75ms (-0.19d, 0.73p, 3.99std)

@ffreyer ffreyer closed this Mar 18, 2024
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.

2 participants