Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ in the README).
`memory_report`), and `Chart.figure()` remains as an advanced escape hatch
to the internal engine object.

### Fixed
- **Streaming appends no longer draw a stroke-width step after browser zoom.**
The tail-only GPU upload path bakes `stroke_width` in device pixels, and a
dpr change (browser zoom, monitor swap) updates `dpr` without rebuilding
traces — so points appended after the change rendered their outline at a
different scale than the points already on screen. Such an append now falls
back to the full rebuild, which renormalizes every row.

### Changed
- **Colored huge-scatter builds are peak-memory-bounded (LOD doc §4.4).**
The mean-color feature's one-time costs no longer scale peak RSS with N ×
Expand Down
4 changes: 4 additions & 0 deletions js/src/50_chartview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2200,6 +2200,10 @@ export class ChartView {
copy(widthName, 2, this.dpr);
copy("symbol", 3);
g.styleBuf = this._upload(values);
// Width rows are baked at the dpr in force right now. Record it so the
// streaming-append fast path can tell whether a later tail upload would
// write rows at a different scale than the prefix already holds (§4).
g._styleDpr = this.dpr;
}
const radius = channel("corner_radius");
if (radius) {
Expand Down
25 changes: 25 additions & 0 deletions js/src/54_kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,23 @@ Object.assign(ChartView.prototype, {
const style = t.style || {};
if (style.curve === "smooth" || style.step) return false; // expanded vertices
if (JSON.stringify(oldT.style || {}) !== JSON.stringify(style)) return false;
// Only the scatter branch below knows how to extend per-point buffers, so
// any other mark carrying one must rebuild. Today a line has none; this
// keeps that a checked fact rather than an assumption a future per-point
// line channel would silently break.
if (
t.kind !== "scatter" &&
(g.cBuf || g.rgbaBuf || g.sBuf || g.styleBuf || g.strokeBuf || g.radiusBuf)
) {
return false;
}
// stroke_width rows are baked with the dpr in force when they were written
// (`_buildInstanceStyleChannels`). `_resize` updates `this.dpr` on browser
// zoom or a monitor swap WITHOUT rebuilding traces, so a tail upload after
// such a change would scale appended rows differently from the prefix — a
// visible outline-width step inside one trace. Rebuild instead, which
// renormalizes every row at the current dpr.
if (g.styleBuf && g._styleDpr !== this.dpr) return false;
const oldCols = oldSpec.columns;
const newCols = this.spec.columns;

Expand Down Expand Up @@ -708,13 +725,21 @@ Object.assign(ChartView.prototype, {
gl.bufferData(gl.ARRAY_BUFFER, st ? st.x : src.x, gl.STATIC_DRAW);
gl.bindBuffer(gl.ARRAY_BUFFER, g.yBuf);
gl.bufferData(gl.ARRAY_BUFFER, st ? st.y : src.y, gl.STATIC_DRAW);
// These reallocate the data store, so the append fast path's capacity
// bookkeeping no longer describes it. Unreachable for a direct trace
// today (`decimate_view` skips exactly the traces the fast path
// accepts), but leaving a stale cap here would mean a tail
// bufferSubData past the end of the store if that ever drifts.
g.xBuf._fcCapBytes = 0;
g.yBuf._fcCapBytes = 0;
g.xMeta = { ...g.xMeta, offset: upd.x.offset, scale: upd.x.scale };
g.yMeta = { ...g.yMeta, offset: upd.y.offset, scale: upd.y.scale };
g._dashX = st ? st.x : src.x;
g._dashY = st ? st.y : src.y;
if (bArr) {
gl.bindBuffer(gl.ARRAY_BUFFER, g.baseBuf);
gl.bufferData(gl.ARRAY_BUFFER, sm ? sm.extra : bArr, gl.STATIC_DRAW);
g.baseBuf._fcCapBytes = 0;
g.baseMeta = { ...g.baseMeta, offset: upd.base.offset, scale: upd.base.scale };
}
g.n = st ? st.n : src.n;
Expand Down
2 changes: 1 addition & 1 deletion python/xy/_payload.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def ship(self, values: np.ndarray, col: "Column", *, scale: str | None = None) -
# shipped offset and re-center only when it leaves the safe span.
offset = (
lod.geometry_offset(scale, col.min, col.max)
if scale in ("log", "symlog")
if lod.pins_offset_to_zero(scale)
else col.suggest_offset()
)
encoded = lod.encode_f32_values(
Expand Down
13 changes: 12 additions & 1 deletion python/xy/lod.py
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,17 @@ def encode_f32_values(
return EncodedColumn(meta=meta, values=enc)


#: Axis scales whose geometry must be encoded around a zero origin. Callers
#: that choose an offset themselves (the sticky append offset in `_payload`)
#: must branch on this, not on their own copy of the scale names.
LOG_FAMILY_SCALES = ("log", "symlog")


def pins_offset_to_zero(scale: str | None) -> bool:
"""Whether `scale` requires the zero origin `geometry_offset` gives it."""
return scale in LOG_FAMILY_SCALES


def geometry_offset(scale: str | None, lo: float, hi: float) -> float:
"""Precision center for offset-encoded geometry (§4/§16).

Expand All @@ -882,7 +893,7 @@ def geometry_offset(scale: str | None, lo: float, hi: float) -> float:
decades). With offset 0 the encode error is a ~2⁻²⁴ *relative* error,
which the log-family transform maps to a bounded sub-pixel coordinate
error at every magnitude."""
if scale in ("log", "symlog") or not (np.isfinite(lo) and np.isfinite(hi)):
if pins_offset_to_zero(scale) or not (np.isfinite(lo) and np.isfinite(hi)):
return 0.0
return (lo + hi) / 2.0

Expand Down
88 changes: 83 additions & 5 deletions scripts/append_stream_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@
M = 200 # rows appended per tick
TICKS = 5
DECIMATED_SHIPPED = 500
DECIMATED_PX = 2048
# Just above the ~620 px plot this spec produces (700 wide minus margins), so
# the at-home skip predicate `decimation_px >= plot.w` is actually load-bearing:
# a unit error or a dpr-scaled comparison on either side flips it and the
# viewSendsHome assertion fails. A generous 2048 would mask all of those.
DECIMATED_PX = 640


def find_chromium() -> str:
Expand Down Expand Up @@ -104,10 +108,15 @@ def series(n: int):
return xs, ys, cs, ss


def build_payload(n: int, x_off: float, y_off: float):
def build_payload(n: int, x_off: float, y_off: float, *, stroke_width: bool = False):
"""One full payload for `n` rows per direct trace. Offsets are the
caller's (sticky across ticks, like `Column.suggest_offset`), so every
payload's columns are byte-prefixes of the next one's."""
payload's columns are byte-prefixes of the next one's.

`stroke_width=True` adds a per-point stroke-width channel to the scatter,
which is what makes the client build an interleaved `styleBuf` — the one
buffer whose rows are baked with the dpr in force when they were written.
"""
xs, ys, cs, ss = series(n)
w = Writer()
hi_x = float(n) # home range follows the data
Expand All @@ -134,6 +143,11 @@ def build_payload(n: int, x_off: float, y_off: float):
"domain": [0.0, 1.0],
"buf": w.ship_scalar(ss),
},
**(
{"channels": {"stroke_width": {"buf": w.ship_scalar([1.0] * n)}}}
if stroke_width
else {}
),
},
{
"id": 1,
Expand Down Expand Up @@ -193,18 +207,35 @@ def main() -> None:
spec, blob = build_payload(N0 + k * M, x_off, y_off)
ticks.append({"spec": spec, "blob": base64.b64encode(blob).decode()})

# Phase B continues the same monotonically growing stream. Replaying the
# phase-A payloads instead would ship *fewer* rows than are already
# resident, which fails the `len >= old_len` prefix check and silently
# measures rebuild-path coalescing rather than fast-path coalescing.
zoom_ticks = []
for k in range(TICKS + 1, TICKS + 7):
spec, blob = build_payload(N0 + k * M, x_off, y_off)
zoom_ticks.append({"spec": spec, "blob": base64.b64encode(blob).decode()})

# Phase C runs a small separate chart carrying a per-point stroke_width
# channel, so the client builds an interleaved styleBuf for it.
dpr_ticks = []
for k in range(3):
spec, blob = build_payload(2_000 + k * M, 1_000.0, 0.0, stroke_width=True)
dpr_ticks.append({"spec": spec, "blob": base64.b64encode(blob).decode()})

# Expected steady-state tail bytes per tick: 6 f32 columns × M rows
# (scatter x/y/color/size + line x/y); the decimated trace is unaffected.
tail_bytes = 6 * M * 4
full_bytes = 6 * (N0 + TICKS * M) * 4 # what a rebuild re-uploads at the end

page = f"""<!doctype html><html><head><meta charset=utf-8><title>pending</title></head>
<body><div id=chart></div><div id=fresh></div>
<body><div id=chart></div><div id=fresh></div><div id=dpr></div>
<script>{standalone}</script>
<script>
const spec0={json.dumps(spec0)};
const blob0=Uint8Array.from(atob("{base64.b64encode(blob0).decode()}"),c=>c.charCodeAt(0));
const ticks={json.dumps(ticks)};
const zoomTicks={json.dumps(zoom_ticks)};
const b64=(s)=>Uint8Array.from(atob(s),c=>c.charCodeAt(0));
const gl2=WebGL2RenderingContext.prototype;
const counters={{data:0,dataBytes:0,sub:0,subBytes:0}};
Expand Down Expand Up @@ -268,17 +299,46 @@ def main() -> None:
v.view=v._viewFrom({{x0:100,x1:900}}); // strictly inside: hold, not follow
comm.sent.length=0;
for(let i=0;i<6;i++){{
const t=ticks[i%ticks.length];
const t=zoomTicks[i];
v._onKernelMsg({{type:"append",affected:[0,1],spec:t.spec}},[b64(t.blob)]);
await wait(60);
}}
await wait(1000);
const viewSendsZoomed=comm.sent.filter(m=>m.type==="view").length;

// --- phase C: a dpr change between build and append must NOT tail-append ---
// styleBuf rows bake stroke_width at the dpr in force when written, and
// _resize updates dpr without rebuilding traces. Appending in place across
// that change would leave the prefix at the old scale and the appended tail
// at the new one — a width step inside a single trace. The fast path must
// detect it and fall back to the rebuild, which renormalizes every row.
const dprTicks={json.dumps(dpr_ticks)};
const vd=xy.renderStandalone(document.getElementById("dpr"),
dprTicks[0].spec,b64(dprTicks[0].blob).buffer);
vd._sampleRebinDisabled=true;
vd._drawNow();
const gd0=vd.gpuTraces[0];
const hasStyleBuf=gd0.styleBuf?1:0;
// Same dpr: the fast path is expected to take it (trace object retained).
vd._onKernelMsg({{type:"append",affected:[0],spec:dprTicks[1].spec}},[b64(dprTicks[1].blob)]);
const dprSameKeeps=(vd.gpuTraces[0]===gd0)?1:0;
const gd1=vd.gpuTraces[0];
// Now move the dpr the way browser zoom does, then append again.
const dpr0=vd.dpr;
Object.defineProperty(window,"devicePixelRatio",{{value:dpr0*2,configurable:true}});
vd._resize();
const dprMoved=(vd.dpr===dpr0*2)?1:0;
vd._onKernelMsg({{type:"append",affected:[0],spec:dprTicks[2].spec}},[b64(dprTicks[2].blob)]);
const dprChangeRebuilds=(vd.gpuTraces[0]!==gd1)?1:0;
vd._drawNow();
Object.defineProperty(window,"devicePixelRatio",{{value:dpr0,configurable:true}});
vd.destroy();

document.title="XY_OK "+JSON.stringify({{
warmData:perTick[0].data,warmBytes:perTick[0].dataBytes,
steadyData,steadySubBytes:Math.round(steadySub),
inPlace,lit,mismatch,viewSendsHome,viewSendsZoomed,
hasStyleBuf,dprSameKeeps,dprMoved,dprChangeRebuilds,
}});
}}catch(e){{document.title="XY_ERROR "+(e.stack||e.message)}}}})();
</script></body></html>"""
Expand Down Expand Up @@ -345,6 +405,24 @@ def main() -> None:
f"zoomed 6-tick burst sent {stats['viewSendsZoomed']} view re-requests; "
"expected a single coalesced round-trip (maxWait)"
)
if not stats["hasStyleBuf"]:
raise SystemExit(
"the dpr probe's trace built no interleaved styleBuf, so it cannot "
"test the dpr guard (stroke_width channel wiring changed?)"
)
if not stats["dprSameKeeps"]:
raise SystemExit(
"an append at an unchanged dpr rebuilt the trace instead of "
"extending it in place; the dpr guard is too strict"
)
if not stats["dprMoved"]:
raise SystemExit("the dpr probe failed to move devicePixelRatio; probe is inert")
if not stats["dprChangeRebuilds"]:
raise SystemExit(
"an append after a dpr change extended GPU buffers in place; the "
"appended stroke_width rows are scaled at the new dpr while the "
"prefix holds the old one (visible width step inside one trace)"
)
print("append stream smoke OK: tail-only uploads + pixel-identical + coalesced refines")


Expand Down
6 changes: 5 additions & 1 deletion spec/design/wire-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,11 @@ rebuilt otherwise:
domain that expanded fails this on purpose: its shipped values are
normalized over the domain), same style, no transition keys, no
`curve: "smooth"`/`step` vertex expansion — extends its existing GPU
buffers with a tail-only `bufferSubData`. Buffer *objects* are retained
buffers with a tail-only `bufferSubData`. The device pixel ratio must also
be unchanged since the trace was built: the interleaved style buffer bakes
`stroke_width` in device pixels, and `_resize` moves `dpr` on browser zoom
*without* rebuilding traces, so appending across that change would leave
the prefix at one scale and the tail at another. Buffer *objects* are retained
(VAO attachments stay valid); data stores grow with doubling capacity,
mirroring `Column.append` kernel-side, so a steady stream costs O(rows
appended) GPU upload per tick instead of O(N). The client derives
Expand Down
15 changes: 15 additions & 0 deletions tests/test_lod.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,3 +612,18 @@ def test_sample_rows_for_target_rejects_bad_options(kwargs, message: str) -> Non

with pytest.raises(ValueError, match=message):
lod.sample_rows_for_target(np.arange(3, dtype=np.int64), target, **options)


def test_pins_offset_to_zero_agrees_with_geometry_offset() -> None:
"""The sticky-offset path in `_payload.ship` branches on
`pins_offset_to_zero` to decide whether it may choose an offset at all.
That predicate and `geometry_offset`'s own zero-origin rule must never
drift apart: if they did, `ship` would pin a sticky midpoint on an axis
whose shader transform requires the zero origin (§4)."""
for scale in ("log", "symlog"):
assert lod.pins_offset_to_zero(scale)
assert lod.geometry_offset(scale, 10.0, 20.0) == 0.0

for scale in (None, "linear"):
assert not lod.pins_offset_to_zero(scale)
assert lod.geometry_offset(scale, 10.0, 20.0) == 15.0
Loading