Skip to content

VertexArrayState: dirty() forces every active array to be dispatched again - #51

Open
patricktodd wants to merge 1 commit into
OpenMW:3.6from
patricktodd:fix/vertexarraystate-offset
Open

VertexArrayState: dirty() forces every active array to be dispatched again#51
patricktodd wants to merge 1 commit into
OpenMW:3.6from
patricktodd:fix/vertexarraystate-offset

Conversation

@patricktodd

Copy link
Copy Markdown

A vertex array object records the attribute pointer (buffer and offset) at dispatch time. When another array sharing the same VertexBufferObject is replaced or resized, the buffer is re-laid out and unchanged arrays move, but setArray() only re-dispatched on a new array pointer or modified count, so the vertex array object kept a stale offset. Track the buffer object and offset each array was last dispatched with and re-dispatch when they differ; reset them in dirty(). With a vertex array object in use, arrays that were never assigned a buffer object get one, as client-side pointers are not allowed there.

Observed while working on OpenMW on a GL 4.1 core profile context on macOS, alongside PR #50: OpenMW's cloud layer read its colour array from the wrong offset after the sky geometry's other arrays were regenerated, so the clouds drew with the wrong data.

Two things for review: the array gets its buffer object through a const_cast (osg::Geometry does the same internally), and the last-dispatched buffer object is compared by address. The check is gated on a VAO being in use, so the client-array path is untouched.

Mainline OpenSceneGraph master still has the offset-only check.

@AnyOldName3

Copy link
Copy Markdown
Member

There are plenty of other things that have been dirtied along the way before we get to the point where the offset's changed and the dispatch needs redoing as a result, and I expect that at least one of them will always need a re-dispatch when it's changed, so there should be a simpler way to fix this problem by cleverly reusing something that already exists rather than picking out the offset as a new thing that needs special tracking.

…again

A vertex array object records each attribute pointer (buffer and offset) at
dispatch time. When one of a drawable's arrays is replaced, Drawable::
dirtyGLObjects() already reaches the per-context VertexArrayState through
dirty(), but that only set _requiresSetArrays, and setArray() still skipped
arrays whose pointer and modified count were unchanged. Those arrays share a
VertexBufferObject with the replaced one and move inside it when the buffer
is recompiled, so the vertex array object kept their old offsets. Have dirty()
invalidate the recorded modified count of every active array so the next
setArrays() dispatches them all.
@patricktodd
patricktodd force-pushed the fix/vertexarraystate-offset branch from ca21a1e to 03aac0d Compare September 8, 2026 00:29
@patricktodd

patricktodd commented Sep 8, 2026

Copy link
Copy Markdown
Author

OK yeah, I rebuilt around dirty() since that's what Drawable::dirtyGLObjects() already calls when an array is replaced. It now invalidates the recorded modified count of every active array, so the next pass resends all of them; nothing new to track. It does force every array of that drawable to be resent once.

Dropping the other half (giving buffer-less arrays a VBO) turned up OpenMW's NifOsg::ParticleSystem sending a one-element overall-bound normal through the vertex array path under a VAO, so it errored constantly. Fixed that on the OpenMW side instead, so my test stations render clouds correctly again.

Let me know if that fits, thanks!

@patricktodd patricktodd changed the title VertexArrayState: re-dispatch an array when its offset in the buffer object changes VertexArrayState: dirty() forces every active array to be dispatched again Sep 8, 2026
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