silx-kit / h5web

React components for data visualization and exploration
https://h5web.panosc.eu/
MIT License
183 stars 18 forks source link

Implement `useGeometry` hook to ease creation/update of geometries #1515

Closed axelboc closed 11 months ago

axelboc commented 11 months ago

This is a big one, even though I've tried to keep a few things out of it.

Goal

The goal is to make it more straightforward for us internally but also for consumers of @h5web/lib to implement and use custom buffer geometries (like maybe thick/dashed lines that don't change while zooming). Attn @PeterC-DLS, as this might be of interest to you for Davidia.

Custom buffer geometries are useful to avoid blowing up the bundle and increasing maintenance cost by adding three-stdlib and drei as (peer) dependencies to @h5web/lib and consumer applications. Moreover, Three's/drei's built-in geometry code can be:

Implementation

The main entry point is a new hook called useGeometry, which accepts:

  1. a buffer geometry class that inherits H5WebGeometry (see below);
  2. the number of items to iterate over when updating the geometry (called dataLength);
  3. a bunch of parameters that are required to prepare/update the various buffer attributes in the geometry (abscissas, values, scales, interpolators, etc.

H5WebGeometry is an abstract class that inherits BufferGeometry and specifies two methods: prepare and update.

Justification

With this approach geometries are fully independent from one another. We no longer try to over-optimise things by sharing buffer attributes across geometries or by coupling lots of unrelated computations inside the same loop, which led to code that was difficult to read and impossible to abstract for re-use.

Sure, we end up with more buffers in memory in some cases (like when displaying both the line and points with DataCurve/LineVis), but we gain in code readability and make the geometry code much easier to refactor in the future (for instance to fix the subtle glitches when drawing lines that include points with non-finite coordinates, which are moved to (0, 0, CAMERA_FAR)).

My initial idea was to implement a useGeometries hook that allowed updating multiple geometries within a single loop. However, this required having a small loop (to iterate over the geometries) inside a big loop (to iterate over the data), and I realised that this had no theoretical performance benefit over doing the big loop multiple times in a row (i.e. via multiple, consecutive calls to useGeometry).

Moreover, useGeometry (rather than useGeometries) allows having re-usable components like Line, Glyphs and ErrorBars that each create/update their own geometries independently from one another. As a result, DataCurve is now just a dumb component that helps reduce duplication in LineVis.

In this PR

Naming decisions

I wasn't sure about the naming of the geometries and the components in which they are used, because:

As a result, I decided to use the names of the components in which they are used - i.e. SurfaceMesh, ScatterPoints, Line, etc. Was this a good decision?

If it was, are the names of the new components, Line, Glyphs, ErrorBars, appropriate?

I wondered about using the names LineCurve/LineCurveGeometry and GlyphCurve/GlyphCurveGeometry, for instance.