Skip to content

refactor(log-viewer): share the pixi app and mesh setup - #1090

Merged
lcottercertinia merged 3 commits into
certinia:mainfrom
lukecotter:refactor-share-pixi-rendering
Sep 29, 2026
Merged

lcottercertinia merged 3 commits into
certinia:mainfrom
lukecotter:refactor-share-pixi-rendering

Conversation

@lukecotter

Copy link
Copy Markdown
Collaborator

📝 PR Overview

The timeline built three Pixi apps and four rectangle meshes from copies of the same
setup. Each now has one home in optimised/rendering/. No behaviour change.

Draft until the dev host check below is done. A missing renderer here fails silently —
the canvas simply comes up empty — and neither jest nor pnpm measure can see it.

🛠️ Changes made

  • share the pixi app setup — FlameChart, MetricStripOrchestrator and
    MinimapOrchestrator each held the same ten lines, differing only in height and
    antialias. They now call createTimelineApp, beside the destroyTimelineApp they
    already shared. antialias defaults off; only the metric strip draws lines that
    need it.
  • share the pixi mesh construction — MeshRectangleRenderer,
    MeshMarkerRenderer, MeshSearchStyleRenderer and MeshAxisRenderer each built the
    same geometry, shader, mesh, label and addChild sequence. They now call
    createRectangleMesh.

Two differences worth stating

  • this.app is assigned after init() resolves, where before
    this.app = new PIXI.Application() set a not-yet-initialised app ahead of the
    await. Nothing reads it in that window: FlameChart wires its ResizeObserver
    only after the first render, and TimelineFlameChart guards teardown with an init
    epoch rather than racing it. Where it would matter it is safer — resize() guards on
    !this.app, and the old code could reach an app that had not been initialised.
  • The four private shader fields are gone, since the helper owns the shader. Each
    was assigned once, never read again, and no destroy() released it. Holding it was a
    small retention: Mesh.destroy() nulls its own reference, so the field kept the
    Shader alive after the mesh was already gone.

pixiApp.ts imports pixi as a value now rather than as a type. Its only three
importers already did, and no bundler config makes pixi a lazy chunk.

Left out on purpose

  • minimap/MinimapRenderer.ts builds a fifth mesh and is untouched. It uses
    MinimapBarGeometry, takes no label, and adds itself to a container later.
  • The BUCKET_BLOCK derivation is still written twice. It is five lines of
    straight-line arithmetic over two exported constants, with nothing that can drift
    between the sites, and two of its four original sites are deleted by refactor: remove dead code and share test setup #1066. Better
    revisited once that lands and the surviving count is settled.

🧩 Type of change (check all applicable)

  • 🐛 Bug fix - something not working as expected
  • ✨ New feature – adds new functionality
  • ♻️ Refactor - internal changes with no user impact
  • ⚡ Performance Improvement
  • 📝 Documentation - README or documentation site changes
  • 🔧 Chore - dev tooling, CI, config
  • 💥 Breaking change

📷 Screenshots / gifs / video [optional]

None yet. The dev host pass below is what would produce them.

🔗 Related Issues

None. Touches none of the files #1066 deletes, so the two are independent.

✅ Tests added?

  • 👍 yes
  • 🙅 no, not needed
  • 🙋 no, I need help

Both changes are construction-time Pixi code with no testable behaviour of their own,
and the existing suites cover the renderers that use them. pnpm test is green at
2,625 tests across 191 suites, with pnpm exec tsc -b --force and pnpm exec eslint
clean.

There is no automated check that can cover this. scripts/measure/measure.ts:8 says
its harness is "free of the DOM and of PixiJS, so they run under Node", and jest runs
under jsdom with no WebGL. Hence the manual pass.

📚 Docs updated?

  • 🔖 README.md
  • 🔖 CHANGELOG.md
  • 📖 help site
  • 🧪 Marked any pre-release-only features
  • 🙅 not needed

Nothing reachable by a user changes.

Anything else we need to know? [optional]

Dev host check, required before this leaves draft. Open a sample-app/ log and
confirm on the timeline tab that all six draw:

  • frames
  • markers
  • the time axis
  • find highlights
  • the minimap
  • the metric strip, expanded — the one app built with antialias: true

FlameChart, MetricStripOrchestrator and MinimapOrchestrator each held
the same ten lines of Pixi app setup, differing only in height and
antialias. They now call createTimelineApp, beside the
destroyTimelineApp they already shared.

`this.app` is assigned after init() resolves rather than before the
await. Nothing reads it during that window: FlameChart wires its
ResizeObserver only after the first render, and TimelineFlameChart
guards teardown with an init epoch rather than racing it. Where it
would matter it is safer — resize() guards on `!this.app`, and the old
code could reach a Pixi app that had not been initialised.

pixiApp.ts imports pixi as a value now. Its only three importers
already did, and no bundler config makes pixi a lazy chunk.
MeshRectangleRenderer, MeshMarkerRenderer, MeshSearchStyleRenderer and
MeshAxisRenderer each built the same geometry, shader, mesh, label and
addChild sequence. They now call createRectangleMesh.

The helper owns the shader, so the four `private shader` fields go.
Each was assigned once and never read again, and no destroy() released
it. Holding it was a small leak: Mesh.destroy() nulls its own
reference, so the field kept the shader alive after the mesh was gone.

MinimapRenderer builds a fifth mesh and is left alone. It uses
MinimapBarGeometry, takes no label and adds itself to a container
later.
@lcottercertinia
lcottercertinia marked this pull request as ready for review September 29, 2026 09:35
@lcottercertinia
lcottercertinia merged commit 3c15eba into certinia:main Sep 29, 2026
8 checks passed
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