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

WebGL resource creation is not robust against context loss #16923

Conversation

kkinnunen-apple
Copy link
Contributor

@kkinnunen-apple kkinnunen-apple commented Aug 22, 2023

6a55c5e

WebGL resource creation is not robust against context loss
https://bugs.webkit.org/show_bug.cgi?id=260513
rdar://problem/114245538

Reviewed by Dan Glastonbury.

At the moment, the various GraphicsContextGL::create{Resource}() calls
might return 0 if the context is lost during that call. Handle this in
all the WebGL resource constructors. Return RefPtr and handle the
nullptr in the caller.

* Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:
(WebCore::WebXROpaqueFramebuffer::create):
* Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:
(WebCore::WebGL2RenderingContext::fenceSync):
* Source/WebCore/html/canvas/WebGLBuffer.cpp:
(WebCore::WebGLBuffer::create):
(WebCore::WebGLBuffer::WebGLBuffer):
* Source/WebCore/html/canvas/WebGLBuffer.h:
* Source/WebCore/html/canvas/WebGLFramebuffer.cpp:
(WebCore::WebGLFramebuffer::create):
(WebCore::WebGLFramebuffer::createOpaque):
(WebCore::WebGLFramebuffer::WebGLFramebuffer):
* Source/WebCore/html/canvas/WebGLFramebuffer.h:
* Source/WebCore/html/canvas/WebGLObject.cpp:
(WebCore::WebGLObject::WebGLObject):
(WebCore::WebGLObject::setObject): Deleted.
* Source/WebCore/html/canvas/WebGLObject.h:
* Source/WebCore/html/canvas/WebGLProgram.cpp:
(WebCore::WebGLProgram::create):
(WebCore::WebGLProgram::WebGLProgram):
* Source/WebCore/html/canvas/WebGLProgram.h:
* Source/WebCore/html/canvas/WebGLQuery.cpp:
(WebCore::WebGLQuery::create):
(WebCore::WebGLQuery::WebGLQuery):
* Source/WebCore/html/canvas/WebGLQuery.h:
* Source/WebCore/html/canvas/WebGLRenderbuffer.cpp:
(WebCore::WebGLRenderbuffer::create):
(WebCore::WebGLRenderbuffer::WebGLRenderbuffer):
* Source/WebCore/html/canvas/WebGLRenderbuffer.h:
* Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::createProgram):
* Source/WebCore/html/canvas/WebGLSampler.cpp:
(WebCore::WebGLSampler::create):
(WebCore::WebGLSampler::WebGLSampler):
* Source/WebCore/html/canvas/WebGLSampler.h:
* Source/WebCore/html/canvas/WebGLShader.cpp:
(WebCore::WebGLShader::create):
(WebCore::WebGLShader::WebGLShader):
* Source/WebCore/html/canvas/WebGLShader.h:
* Source/WebCore/html/canvas/WebGLSync.cpp:
(WebCore::WebGLSync::create):
(WebCore::WebGLSync::WebGLSync):
(WebCore::m_sync):
(WebCore::WebGLSync::deleteObjectImpl):
* Source/WebCore/html/canvas/WebGLSync.h:
* Source/WebCore/html/canvas/WebGLTexture.cpp:
(WebCore::WebGLTexture::create):
(WebCore::WebGLTexture::WebGLTexture):
* Source/WebCore/html/canvas/WebGLTexture.h:
* Source/WebCore/html/canvas/WebGLTimerQueryEXT.cpp:
(WebCore::WebGLTimerQueryEXT::create):
(WebCore::WebGLTimerQueryEXT::WebGLTimerQueryEXT):
* Source/WebCore/html/canvas/WebGLTimerQueryEXT.h:
* Source/WebCore/html/canvas/WebGLTransformFeedback.cpp:
(WebCore::WebGLTransformFeedback::create):
(WebCore::WebGLTransformFeedback::WebGLTransformFeedback):
* Source/WebCore/html/canvas/WebGLTransformFeedback.h:
* Source/WebCore/html/canvas/WebGLVertexArrayObject.cpp:
(WebCore::WebGLVertexArrayObject::create):
(WebCore::WebGLVertexArrayObject::WebGLVertexArrayObject):
* Source/WebCore/html/canvas/WebGLVertexArrayObject.h:
* Source/WebCore/html/canvas/WebGLVertexArrayObjectBase.cpp:
(WebCore::WebGLVertexArrayObjectBase::WebGLVertexArrayObjectBase):
* Source/WebCore/html/canvas/WebGLVertexArrayObjectBase.h:
* Source/WebCore/html/canvas/WebGLVertexArrayObjectOES.cpp:
(WebCore::WebGLVertexArrayObjectOES::createDefault):
(WebCore::WebGLVertexArrayObjectOES::createUser):
(WebCore::WebGLVertexArrayObjectOES::WebGLVertexArrayObjectOES):
* Source/WebCore/html/canvas/WebGLVertexArrayObjectOES.h:

Canonical link: https://commits.webkit.org/267183@main

13c496d

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@kkinnunen-apple kkinnunen-apple self-assigned this Aug 22, 2023
@kkinnunen-apple kkinnunen-apple added the WebGL Bugs in WebKit’s implementation of the WebGL standard. label Aug 22, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 22, 2023
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Aug 22, 2023
@kkinnunen-apple kkinnunen-apple requested review from djg and removed request for rniwa and cdumez August 22, 2023 18:14
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 22, 2023
Copy link
Contributor

@djg djg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

auto object = context.graphicsContextGL()->createSampler();
if (!object)
return nullptr;
return adoptRef(*new WebGLSampler { context, object });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern feels like something that should be automated.

@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Aug 23, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 23, 2023
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Aug 23, 2023
@kkinnunen-apple kkinnunen-apple added the merge-queue Applied to send a pull request to merge-queue label Aug 23, 2023
https://bugs.webkit.org/show_bug.cgi?id=260513
rdar://problem/114245538

Reviewed by Dan Glastonbury.

At the moment, the various GraphicsContextGL::create{Resource}() calls
might return 0 if the context is lost during that call. Handle this in
all the WebGL resource constructors. Return RefPtr and handle the
nullptr in the caller.

* Source/WebCore/Modules/webxr/WebXROpaqueFramebuffer.cpp:
(WebCore::WebXROpaqueFramebuffer::create):
* Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:
(WebCore::WebGL2RenderingContext::fenceSync):
* Source/WebCore/html/canvas/WebGLBuffer.cpp:
(WebCore::WebGLBuffer::create):
(WebCore::WebGLBuffer::WebGLBuffer):
* Source/WebCore/html/canvas/WebGLBuffer.h:
* Source/WebCore/html/canvas/WebGLFramebuffer.cpp:
(WebCore::WebGLFramebuffer::create):
(WebCore::WebGLFramebuffer::createOpaque):
(WebCore::WebGLFramebuffer::WebGLFramebuffer):
* Source/WebCore/html/canvas/WebGLFramebuffer.h:
* Source/WebCore/html/canvas/WebGLObject.cpp:
(WebCore::WebGLObject::WebGLObject):
(WebCore::WebGLObject::setObject): Deleted.
* Source/WebCore/html/canvas/WebGLObject.h:
* Source/WebCore/html/canvas/WebGLProgram.cpp:
(WebCore::WebGLProgram::create):
(WebCore::WebGLProgram::WebGLProgram):
* Source/WebCore/html/canvas/WebGLProgram.h:
* Source/WebCore/html/canvas/WebGLQuery.cpp:
(WebCore::WebGLQuery::create):
(WebCore::WebGLQuery::WebGLQuery):
* Source/WebCore/html/canvas/WebGLQuery.h:
* Source/WebCore/html/canvas/WebGLRenderbuffer.cpp:
(WebCore::WebGLRenderbuffer::create):
(WebCore::WebGLRenderbuffer::WebGLRenderbuffer):
* Source/WebCore/html/canvas/WebGLRenderbuffer.h:
* Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::createProgram):
* Source/WebCore/html/canvas/WebGLSampler.cpp:
(WebCore::WebGLSampler::create):
(WebCore::WebGLSampler::WebGLSampler):
* Source/WebCore/html/canvas/WebGLSampler.h:
* Source/WebCore/html/canvas/WebGLShader.cpp:
(WebCore::WebGLShader::create):
(WebCore::WebGLShader::WebGLShader):
* Source/WebCore/html/canvas/WebGLShader.h:
* Source/WebCore/html/canvas/WebGLSync.cpp:
(WebCore::WebGLSync::create):
(WebCore::WebGLSync::WebGLSync):
(WebCore::m_sync):
(WebCore::WebGLSync::deleteObjectImpl):
* Source/WebCore/html/canvas/WebGLSync.h:
* Source/WebCore/html/canvas/WebGLTexture.cpp:
(WebCore::WebGLTexture::create):
(WebCore::WebGLTexture::WebGLTexture):
* Source/WebCore/html/canvas/WebGLTexture.h:
* Source/WebCore/html/canvas/WebGLTimerQueryEXT.cpp:
(WebCore::WebGLTimerQueryEXT::create):
(WebCore::WebGLTimerQueryEXT::WebGLTimerQueryEXT):
* Source/WebCore/html/canvas/WebGLTimerQueryEXT.h:
* Source/WebCore/html/canvas/WebGLTransformFeedback.cpp:
(WebCore::WebGLTransformFeedback::create):
(WebCore::WebGLTransformFeedback::WebGLTransformFeedback):
* Source/WebCore/html/canvas/WebGLTransformFeedback.h:
* Source/WebCore/html/canvas/WebGLVertexArrayObject.cpp:
(WebCore::WebGLVertexArrayObject::create):
(WebCore::WebGLVertexArrayObject::WebGLVertexArrayObject):
* Source/WebCore/html/canvas/WebGLVertexArrayObject.h:
* Source/WebCore/html/canvas/WebGLVertexArrayObjectBase.cpp:
(WebCore::WebGLVertexArrayObjectBase::WebGLVertexArrayObjectBase):
* Source/WebCore/html/canvas/WebGLVertexArrayObjectBase.h:
* Source/WebCore/html/canvas/WebGLVertexArrayObjectOES.cpp:
(WebCore::WebGLVertexArrayObjectOES::createDefault):
(WebCore::WebGLVertexArrayObjectOES::createUser):
(WebCore::WebGLVertexArrayObjectOES::WebGLVertexArrayObjectOES):
* Source/WebCore/html/canvas/WebGLVertexArrayObjectOES.h:

Canonical link: https://commits.webkit.org/267183@main
@webkit-commit-queue
Copy link
Collaborator

Committed 267183@main (6a55c5e): https://commits.webkit.org/267183@main

Reviewed commits have been landed. Closing PR #16923 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 23, 2023
@webkit-commit-queue webkit-commit-queue merged commit 6a55c5e into WebKit:main Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebGL Bugs in WebKit’s implementation of the WebGL standard.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants