[TheTomBritton]

Fixes #27945.

Real GL reads client-side vertex arrays when the draw call executes, so an application may keep the same arrays bound via gl*Pointer and refill them in place between draws. GLImmediate cached the restrided copy of planar client arrays and skipped the re-copy unless a gl*Pointer/client-state call had set modifiedClientAttributes since the last draw — so every such draw re-drew the first draw's vertex data.

Two changes, both needed:

1. Drop the modifiedClientAttributes early-out from prepareClientAttributes (and the now-unread flag everywhere), as proposed by @juj in the issue, so client arrays are examined at every draw.
2. Stop overwriting attr.pointer with the restrided staging location in the planar slow path. With the early-out gone, this overwrite made the next prepareClientAttributes re-read the staging buffer's interleaved output using the original planar strides — reading its own previous output with the wrong layout — instead of the application's arrays. Nothing needs the overwritten value: rendering works from attr.offset/stride/vertexPointer, the interleaved fast path needs the real pointer to compute offsets, and glGetPointerv now answers with the pointer the application actually set.

Verification that (1) alone is not sufficient and (1)+(2) fixes the real-world app this was found in, with captures: #27945 (comment)

Test: browser.test_gl_client_arrays_refill draws planar client arrays, refills them in place without touching any GL state, draws again, and asserts via glReadPixels that each draw showed its own data. It fails before this change (second draw re-draws the first draw's triangle; the asserted pixel stays at the clear color) and passes after it.

Perf note: planar client arrays are now re-copied on every draw. That is slower, but they were already the documented very-slow path (GL_ASSERTIONS warns about it), correctness wins per the issue discussion, and interleaved arrays remain the fast path.

🤖 Generated with Claude Code

[juj]

Suggested change

	// Leave attr.pointer at the client's own array: client-side vertex
	// arrays are read at draw time, so the next draw call must re-read
	// (and re-restride) the array's contents from the application's
	// memory, not from this staging copy of the previous draw's data.
	// n.b. The above restrided data cannot be cached, since the user might change the
	// vertex data between draws, and there is no way to detect that. So later draws must
	// re-do the above work.

[juj]

Suggested change

	- `LEGACY_GL_EMULATION` now re-reads client-side vertex arrays at every draw
	call, as real GL does. Previously, planar (non-interleaved) client arrays
	that an application refilled in place between draws kept being drawn with
	the first draw's data. Planar client arrays are consequently re-copied on
	every draw, which can be slower; they were already the documented slow path,
	and interleaved arrays remain fast. (#27945)
	- Removed broken caching of client-side vertex arrays with `LEGACY_GL_EMULATION`,
	since there is no sound way to detect if the cached data might have gotten
	modified by the user between draws. (#27945)

[juj]

LGTM, a couple of comment suggestions to avoid LLMism creep.

This change can have a performance impact to FFP GL code, but given that the caching logic was not sound, we cannot unfortunately keep that.

[TheTomBritton]

Applied both suggestions verbatim in 2b0589f — thanks for the review!

[sbc100]

Is this comment not out of date (since it refers to defining attr.pointer)?

[TheTomBritton]

Good catch, that clause was out of date now that the loop no longer assigns attr.pointer. Dropped it in ced9b58.

[sbc100]

minor nit: Drop the static here. See #27873. I'm trying to keep the tests consistent in this regard.

[TheTomBritton]

Done in ced9b58.

[sbc100]

lgtm.. will land once the tests are passing.

[sbc100]

I think a rebase (or merge) will fix of the current failures.