1805626 - WebGL loses VAO buffer bindings due to incorrect save/restore in GLBlitHelper.cpp

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Hoshino Lina]

Steps to reproduce:

GLBlitHelper fails to correctly save and restore the buffer for vertex attribute bindings. In line 580, it saves the VBO currently bound into GL_ARRAY_BUFFER. Then, in line 596, it binds the saved vertex attribute to that VBO. This is wrong, since the VBO associated with the vertex attribute is a property of the VAO, and distinct from the current GL_ARRAY_BUFFER binding.

What it needs to do is use GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING to get the actual VBO bound to the VAO, like it does for all the other VAO properties.

On an Apple M1 Mac running Asahi Linux with the mesa-asahi-edge drivers:

1. apitrace trace firefox https://app.plex.tv/desktop/
2. apitrace dump firefox.trace

Actual results:

The Plex website background is magenta, the system stutters, and the kernel reports GPU timeouts.

This happens because the incorrect save/restore in GLBlitHelper ends up binding the VAO to a 0/NULL VBO, and the GPU then cannot fetch vertex attribute data and faults.

Expected results:

The website renders properly and no GPU timeouts occur.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Yep, thanks!

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1805626 - Query vert attrib buffer binding directly.

Previously we were querying the current ARRAY_BUFFER_BINDING, which
would fail if you did:

BindBuffer(ARRAY_BUFFER, a)
VertexAttribPointer(...)
BindBuffer(ARRAY_BUFFER, b)

We would assume we should call VeretxAttribPointer again with b bound,
when we need to bind a instead.

Unfortunately, this is hard to test, and we only hit this on drivers
where we don't use VAOs, which are rare now.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/972b14b3582f
Query vert attrib buffer binding directly. r=gfx-reviewers,lsalzman

Comment 4

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/972b14b3582f