Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GL] Rebind buffer to another target panic on WebGL #3669

Closed
Gordon-F opened this issue Mar 11, 2021 · 6 comments · Fixed by #3682
Closed

[GL] Rebind buffer to another target panic on WebGL #3669

Gordon-F opened this issue Mar 11, 2021 · 6 comments · Fixed by #3682

Comments

@Gordon-F
Copy link
Contributor

WebGL: INVALID_OPERATION: bindBuffer: buffers bound to non ELEMENT_ARRAY_BUFFER targets can not be bound to ELEMENT_ARRAY_BUFFER target

panicked at 'Error InvalidOperation executing command: BindIndexBuffer(WebBufferKey(2v1))', gfx/src/backend/gl/src/queue.rs:1072:13

From WebGL docs:

Only one target can be bound to a given WebGLBuffer. An attempt to bind the buffer to another target will throw an INVALID_OPERATION error and the current buffer binding will remain the same.

SpectorJS trace:

viewport: 0, 0, 0, 0
depthRange: 0, 1
scissor: 0, 0, 0, 0
bindBuffer: ELEMENT_ARRAY_BUFFER, undefined
viewport: 0, 0, 0, 0
depthRange: 0, 1
scissor: 0, 0, 0, 0
bindBuffer: ELEMENT_ARRAY_BUFFER, undefined
viewport: 0, 0, 0, 0
depthRange: 0, 1
scissor: 0, 0, 0, 0
bindFramebuffer: DRAW_FRAMEBUFFER, WebGLFramebuffer - ID: 2
framebufferRenderbuffer: DRAW_FRAMEBUFFER, COLOR_ATTACHMENT0_WEBGL, RENDERBUFFER, WebGLRenderbuffer - ID: 0
getError
drawBuffers: [..(1)..]
getError
colorMask: true, true, true, true
getError
clearBufferfv: COLOR, 0, [..(4)..]
getError
scissor: 0, 0, 2048, 1536
getError
viewport: 0, 0, 2048, 1536
depthRange: 0, 1
getError
useProgram: WebGLProgram - ID: 0
getError
disable: BLEND
getError
colorMask: true, true, true, true
getError
frontFace: CCW
disable: CULL_FACE
lineWidth: 1
disable: POLYGON_OFFSET_FILL
getError
disable: DEPTH_TEST
getError
depthMask: true
getError
bindBuffer: ELEMENT_ARRAY_BUFFER, WebGLBuffer - ID: 1
getError -> INVALID_OPERATION
bindBuffer: ARRAY_BUFFER, WebGLBuffer - ID: 1
vertexAttribPointer: 0, 3, FLOAT, false, 24, 0
vertexAttribDivisor: 0, 0
enableVertexAttribArray: 0
bindBuffer: ARRAY_BUFFER, undefined
getError
bindBuffer: ARRAY_BUFFER, WebGLBuffer - ID: 1
vertexAttribPointer: 1, 3, FLOAT, false, 24, 12
vertexAttribDivisor: 1, 0
enableVertexAttribArray: 1
bindBuffer: ARRAY_BUFFER, undefined
getError
drawElements: TRIANGLES, 9, UNSIGNED_SHORT, 1024VertexFragment
getError -> INVALID_OPERATION
fenceSync: SYNC_GPU_COMMANDS_COMPLETE, 0 -> WebGLSync - ID: 360
getSyncParameter: WebGLSync - ID: 359, SYNC_STATUS -> SIGNALED
deleteSync: WebGLSync - ID: 359
bindFramebuffer: DRAW_FRAMEBUFFER, undefined
bindFramebuffer: READ_FRAMEBUFFER, WebGLFramebuffer - ID: 1
blitFramebuffer: 0, 0, 2048, 1536, 0, 0, 2048, 1536, COLOR_BUFFER_BIT, NEAREST
bindFramebuffer: READ_FRAMEBUFFER, undefined

Note: Error was found with code from #3668

@kvark
Copy link
Member

kvark commented Mar 11, 2021

bindBuffer: ELEMENT_ARRAY_BUFFER, WebGLBuffer - ID: 1
getError -> INVALID_OPERATION
bindBuffer: ARRAY_BUFFER, WebGLBuffer - ID: 1
vertexAttribPointer: 0, 3, FLOAT, false, 24, 0
vertexAttribDivisor: 0, 0
enableVertexAttribArray: 0

Weird, so it's a vertex buffer? If so, why are we binding it to the ELEMENT_ARRAY_BUFFER point first?
Does the user code try to use the same buffer for both vertices and indices, perhaps?

@Gordon-F
Copy link
Contributor Author

Gordon-F commented Mar 11, 2021

Weird, so it's a vertex buffer? If so, why are we binding it to the ELEMENT_ARRAY_BUFFER point first?

I will try to investigate it. Maybe we should check first if the buffer already bounded to another target and try to create a new one 🤔

Does the user code try to use the same buffer for both vertices and indices, perhaps?

No, I used different buffers. Anyway, you can see the same error with any wgpu-rs example that using an index buffer.

let vertex_buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
    label: Some("VertexBuffer"),
    contents: bytemuck::cast_slice(VERTICES),
    usage: wgpu::BufferUsage::VERTEX,
});

let index_buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
    label: Some("IndexBuffer"),
    contents: bytemuck::cast_slice(INDICES),
    usage: wgpu::BufferUsage::INDEX,
});

@Gordon-F
Copy link
Contributor Author

Also this issue, not an error for OpenGL ES. The same examples work well with it. I don't understand why there is such a limitation for WebGL.

@kvark
Copy link
Member

kvark commented Mar 11, 2021

IIRC, that's because the browser needs to know exactly what the indices are, so that it can prevent out-of-bounds access on vertex fetching. So it's a safety requirement on the Web...

@Gordon-F
Copy link
Contributor Author

@kvark Could buffer rebinding potentially be a problem for OpenGL ES targets? Should we avoid it?
If it's not a problem for OpenGL ES we should properly check buffer binding and create a new clear buffer only on WebGL (because of extra performance costs).

@kvark
Copy link
Member

kvark commented Mar 11, 2021

I don't think it will affect GLES in any way, no. Having a WebGL-specific workaround sounds fine.

@bors bors bot closed this as completed in f3df513 Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants