AutoAdaptLabel.isFirstRender is never set to false, so getLabelElementsInView() has been unreachable since #6465

Searched open and closed issues, PRs and commits for isFirstRender before filing — no existing report found.

Summary

In AutoAdaptLabel, the field isFirstRender is initialised to true and never assigned again, so the branch that filters label candidates by viewport is unreachable. Before #6465 that filtered path was the only path; the PR added the flag alongside it, so the effect is that viewport filtering has been switched off since then rather than a new option not taking effect.

Observed in @antv/[email protected]; the file on the current v5 default branch is byte-for-byte identical to the published src (I diffed them), and the shipped build carries the same code (lib/behaviors/auto-adapt-label.js:86 and :99). Line numbers below refer to packages/g6/src/behaviors/auto-adapt-label.ts on v5.

The code

195:  private isFirstRender = true;
209:    const labelElementsInView = this.isFirstRender ? this.getLabelElements() : this.getLabelElementsInView();

These are the only two occurrences of the field in packages/g6/src. Nothing assigns false, therefore:

• getLabelElementsInView() (:143) has exactly one call site — the ternary on :209 — and is never executed;
• every recompute bound in bindEvents() (:257-264) — AFTER_RENDER, AFTER_DRAW, AFTER_LAYOUT, AFTER_TRANSFORM — walks the whole elementMap through getLabelElements() (:128), sorts that whole list (sortLabelElementsInView, :169) and runs the collision pass over it (detectLabelCollision, :111), regardless of how much of the graph is on screen.

Introduced in 49ba7ff (#6465). The relevant hunk changed an unconditional call into the ternary:

-    const labelElementsInView = this.getLabelElementsInView();
+    const labelElementsInView = this.isFirstRender ? this.getLabelElements() : this.getLabelElementsInView();

Two things make it easy to miss: the output is unaffected (detectLabelCollision re-checks viewport.isInViewport(labelBounds, true) per element at :118, so off-screen labels still end up hidden), and only AFTER_TRANSFORM is throttled — the other three channels call through directly.

A sibling behaviour in the same PR does clear its first-render state

#6465 also added a first-render payload to AFTER_DRAW — firstRender: this.context.graph.rendered === false (packages/g6/src/runtime/element.ts:360, with graph.rendered flipped in runtime/graph.ts:1212) — and fix-element-size.ts, changed in the same commit, consumes it:

299:  private resetTransform = async (event: IGraphLifeCycleEvent) => {
300:    // 首屏渲染时跳过 | Skip when rendering the first screen
301:    if (event.data?.firstRender) return;

Across packages/g6/src that payload has exactly one producer and one consumer. So two behaviours in the same PR express a "first screen" condition two different ways; one reads the event payload, the other uses an instance field that is never cleared. I'm not going to guess which shape was intended here — that's the part I'd like your call on.

What clearing the flag changes, and what it doesn't

I patched the two private methods on the prototype to count calls (they are TS-private but ordinary prototype methods at runtime), and compared stock 5.1.1 against an arm that sets isFirstRender = false after the first full scan. 1000 labelled nodes on a 40×25 grid, 900×700 page, autoFit: 'view', sequence render() → zoomTo(k) → updateNodeData + draw(), with pauses to drain the throttle window. Headless Chromium 149, two runs, identical numbers.

Stock:

• getLabelElementsInView called 0 times across all 8 recomputes;
• candidate set is 1000 every time, at both the fit zoom (0.338) and at zoom 4;
• isFirstRender still true at the end.

With the flag cleared after the first scan:

• candidate set becomes 12 at zoom 4 and 2 at zoom 8 — i.e. proportional to what is on screen, not a fixed small number;
• the set of visible labels is identical at all three checkpoints — element-wise identical id sets, not merely equal counts (994 and 995 hidden respectively in both arms).

Two limits on reading anything performance-related into this:

• At a fit camera the two paths necessarily agree, since every key bbox intersects the viewport; the run confirms it (1000 candidates either way). So this only differs once you are zoomed in.
• getLabelElementsInView() (:145) itself calls getLabelElements() and filters, so the full elementMap walk happens per recompute either way. What shrinks is the work downstream of it — the centrality sort and the per-element getShape('label').getRenderBounds() plus occupiedBounds intersection tests — at the cost of one key-shape getRenderBounds() per element. Whether that translates into measurable frame time in a real application is something I have not measured and am not claiming.

<!doctype html>
<div id="c" style="width:800px;height:600px"></div>
<script src="https://unpkg.com/@antv/[email protected]/dist/g6.min.js"></script>
<script>
  const { Graph, AutoAdaptLabel, register } = window.G6;
  const N = 1000;
  const CLEAR_FLAG = false; // flip to true for the second arm

  const scans = [];
  const origAll = AutoAdaptLabel.prototype.getLabelElements;
  AutoAdaptLabel.prototype.getLabelElements = function (...a) {
    const r = origAll.apply(this, a);
    scans.push({ path: 'all', size: r.length });
    if (CLEAR_FLAG) this.isFirstRender = false;
    return r;
  };
  const origInView = AutoAdaptLabel.prototype.getLabelElementsInView;
  AutoAdaptLabel.prototype.getLabelElementsInView = function (...a) {
    const r = origInView.apply(this, a);
    scans.push({ path: 'inView', size: r.length });
    return r;
  };

  const cols = 40;
  const nodes = Array.from({ length: N }, (_, i) => ({
    id: `n${i}`,
    style: { x: (i % cols) * 60 + 40, y: Math.floor(i / cols) * 60 + 40 },
  }));
  const graph = new Graph({
    container: 'c', data: { nodes }, animation: false, autoFit: 'view',
    node: { style: { size: 12, labelText: (d) => d.id } },
    behaviors: ['auto-adapt-label'],
  });

  (async () => {
    await graph.render();
    await new Promise((r) => setTimeout(r, 300));
    await graph.zoomTo(4);
    await new Promise((r) => setTimeout(r, 300));
    graph.updateNodeData([{ id: 'n500', style: { x: 120, y: 120 } }]);
    await graph.draw();
    console.log(JSON.stringify({ CLEAR_FLAG, scans,
      inViewCalls: scans.filter((s) => s.path === 'inView').length }, null, 2));
  })();
</script>

The static evidence above stands on its own — two occurrences of the field, no assignment — so this script is only here if you want to see the counts move.

Two things I could not settle, and would rather leave to you

Are the two paths meant to be equivalent? They do not use the same predicate. isInViewport(target, complete = false, tolerance = 0) (runtime/viewport.ts:302) tests intersection when complete is false and containment when true. The filter at :145 passes the key shape's bounds with the default (intersection); the visibility decision at :118 passes the label shape's bounds with complete = true (containment). So I cannot argue the two are equivalent by construction. I also could not construct a case where they diverge: sweeping a default-placed node across the viewport edge in 2px steps over 201 positions, the precondition ("key bbox does not intersect the viewport while the label bbox is fully inside it") held 0 times and observed label visibility differed 0 times between the arms — with the default placement the label sits against the key, so once the key leaves the edge the label is no longer fully inside either. A positive label offset might open that window; I have not tested it.

The existing tests may need a look either way. __tests__/unit/behaviors/auto-adapt-label.spec.ts is four snapshot cases with no assertion pinning first-render behaviour, and zoom-3 goes through graph.zoomTo — the path this affects. #6465 also rewrote snapshots/.../zoom-3.svg and padding-60.svg (many labels flipping visible → hidden), though that commit touched runtime/element.ts, utils/transform.ts and elements/shapes/base-shape.ts as well, so I can't attribute the snapshot change to this hunk specifically. I mention it only so that "just clear the flag" doesn't look free: whichever way you resolve the semantics, those snapshots are the place it will show up.

Possibly related, though with no root-cause analysis in it: #6658 reports rendering fuzziness with this behaviour after addData. I have not verified any connection.

Happy to send a PR once you've decided which of the two shapes is the intended one — reading the payload like fix-element-size does, or clearing the field — since that choice also decides what the snapshots should say.

Unrelated nit while in this file, say the word and I'll open it separately: the JSDoc at :63 says @defaultValue 32 for throttle, while :80 and both documentation pages say 100, so the JSDoc looks like the odd one out.