Skip to content

Commit

Permalink
WebGL "is resource" checking and object validation contains repeated …
Browse files Browse the repository at this point in the history
…code

https://bugs.webkit.org/show_bug.cgi?id=260475
rdar://114207432

Reviewed by Dan Glastonbury.

Checks of "isTexture()" and others, and webgl object validation has
following problems:
 - Uses inheritance for customizing behavior, forcing the base class
   have state which is not common to all subclasses
 - Has custom but similar code for each resource, with varying level
   of consistency (slight checking differences)
 - Has duplicated isContextLost() checks inside validation functions

Fix these by:
 - Using templates to implement the common logic for validation
   instead of using WebGLObject base class. This allows future changes
   to remove the virtualization, further simplifying the code.
 - Expose the differences in resource behavior in the `isUsable()`,
   `isInitialized()` methods in the resources.
 - Implement WebGLBindingPoint<T> template to implement "this object
   was bound" marker in a consistent, "provably" not missing any cases
   kind of way.
 - Removes unused WebGLRenderbuffer::isInitialized() and related
   WebGLFramebuffer::WebGLAttachment::isInitialized() code.
   The code was unused and appropriate to remove here due to the name
   clash with the added isInitialized().
 - Change validateWebGLObject object argument from pointer to object
   since it was always called with an existing object.

* Source/WebCore/html/canvas/EXTDisjointTimerQuery.cpp:
(WebCore::EXTDisjointTimerQuery::isQueryEXT):
(WebCore::EXTDisjointTimerQuery::beginQueryEXT):
(WebCore::EXTDisjointTimerQuery::queryCounterEXT):
(WebCore::EXTDisjointTimerQuery::getQueryObjectEXT):
* Source/WebCore/html/canvas/EXTDisjointTimerQueryWebGL2.cpp:
(WebCore::EXTDisjointTimerQueryWebGL2::queryCounterEXT):
* Source/WebCore/html/canvas/OESVertexArrayObject.cpp:
(WebCore::OESVertexArrayObject::isVertexArrayOES):
(WebCore::OESVertexArrayObject::bindVertexArrayOES):
* Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:
(WebCore::WebGL2RenderingContext::validateAndCacheBufferBinding):
(WebCore::WebGL2RenderingContext::bindFramebuffer):
(WebCore::WebGL2RenderingContext::framebufferTextureLayer):
(WebCore::WebGL2RenderingContext::getFragDataLocation):
(WebCore::WebGL2RenderingContext::deleteQuery):
(WebCore::WebGL2RenderingContext::isQuery):
(WebCore::WebGL2RenderingContext::beginQuery):
(WebCore::WebGL2RenderingContext::getQueryParameter):
(WebCore::WebGL2RenderingContext::isSampler):
(WebCore::WebGL2RenderingContext::bindSampler):
(WebCore::WebGL2RenderingContext::samplerParameteri):
(WebCore::WebGL2RenderingContext::samplerParameterf):
(WebCore::WebGL2RenderingContext::getSamplerParameter):
(WebCore::WebGL2RenderingContext::isSync):
(WebCore::WebGL2RenderingContext::clientWaitSync):
(WebCore::WebGL2RenderingContext::waitSync):
(WebCore::WebGL2RenderingContext::getSyncParameter):
(WebCore::WebGL2RenderingContext::isTransformFeedback):
(WebCore::WebGL2RenderingContext::bindTransformFeedback):
(WebCore::WebGL2RenderingContext::transformFeedbackVaryings):
(WebCore::WebGL2RenderingContext::getTransformFeedbackVarying):
(WebCore::WebGL2RenderingContext::setIndexedBufferBinding):
(WebCore::WebGL2RenderingContext::getUniformIndices):
(WebCore::WebGL2RenderingContext::getActiveUniforms):
(WebCore::WebGL2RenderingContext::getUniformBlockIndex):
(WebCore::WebGL2RenderingContext::getActiveUniformBlockParameter):
(WebCore::WebGL2RenderingContext::getActiveUniformBlockName):
(WebCore::WebGL2RenderingContext::uniformBlockBinding):
(WebCore::WebGL2RenderingContext::isVertexArray):
(WebCore::WebGL2RenderingContext::bindVertexArray):
(WebCore::WebGL2RenderingContext::getFramebufferAttachmentParameter):
(WebCore::WebGL2RenderingContext::getParameter):
* Source/WebCore/html/canvas/WebGL2RenderingContext.h:
* Source/WebCore/html/canvas/WebGLBuffer.h:
(WebCore::WebGLBuffer::didBind):
* Source/WebCore/html/canvas/WebGLDebugShaders.cpp:
(WebCore::WebGLDebugShaders::getTranslatedShaderSource):
* Source/WebCore/html/canvas/WebGLFramebuffer.cpp:
(WebCore::WebGLFramebuffer::WebGLFramebuffer):
* Source/WebCore/html/canvas/WebGLFramebuffer.h:
* Source/WebCore/html/canvas/WebGLObject.h:
(WebCore::WebGLBindingPoint::WebGLBindingPoint):
(WebCore::WebGLBindingPoint::operator=):
(WebCore::WebGLBindingPoint::operator== const):
(WebCore::WebGLBindingPoint::operator bool const):
(WebCore::WebGLBindingPoint::get const):
(WebCore::WebGLBindingPoint::operator-> const):
(WebCore::WebGLBindingPoint::operator* const):
(WebCore::WebGLBindingPoint::operator RefPtr<T> const):
(WebCore::WebGLBindingPoint::didBind):
(WebCore::WebGLObject::isDeleted const):
(WebCore::objectOrZero):
(WebCore::WebGLObject::isDeleted): Deleted.
* Source/WebCore/html/canvas/WebGLProgram.h:
* Source/WebCore/html/canvas/WebGLQuery.h:
* Source/WebCore/html/canvas/WebGLRenderbuffer.cpp:
(WebCore::WebGLRenderbuffer::WebGLRenderbuffer):
* Source/WebCore/html/canvas/WebGLRenderbuffer.h:
* Source/WebCore/html/canvas/WebGLRenderingContext.h:
* Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::clearIfComposited):
(WebCore::WebGLRenderingContextBase::reshape):
(WebCore::WebGLRenderingContextBase::attachShader):
(WebCore::WebGLRenderingContextBase::bindAttribLocation):
(WebCore::WebGLRenderingContextBase::validateAndCacheBufferBinding):
(WebCore::WebGLRenderingContextBase::bindBuffer):
(WebCore::WebGLRenderingContextBase::bindFramebuffer):
(WebCore::WebGLRenderingContextBase::bindRenderbuffer):
(WebCore::WebGLRenderingContextBase::bindTexture):
(WebCore::WebGLRenderingContextBase::compileShader):
(WebCore::WebGLRenderingContextBase::detachShader):
(WebCore::WebGLRenderingContextBase::getActiveAttrib):
(WebCore::WebGLRenderingContextBase::getActiveUniform):
(WebCore::WebGLRenderingContextBase::getAttachedShaders):
(WebCore::WebGLRenderingContextBase::getAttribLocation):
(WebCore::WebGLRenderingContextBase::getParameter):
(WebCore::WebGLRenderingContextBase::getProgramParameter):
(WebCore::WebGLRenderingContextBase::getProgramInfoLog):
(WebCore::WebGLRenderingContextBase::getShaderParameter):
(WebCore::WebGLRenderingContextBase::getShaderInfoLog):
(WebCore::WebGLRenderingContextBase::getShaderSource):
(WebCore::WebGLRenderingContextBase::getUniform):
(WebCore::WebGLRenderingContextBase::getUniformLocation):
(WebCore::WebGLRenderingContextBase::isBuffer):
(WebCore::WebGLRenderingContextBase::isFramebuffer):
(WebCore::WebGLRenderingContextBase::isProgram):
(WebCore::WebGLRenderingContextBase::isRenderbuffer):
(WebCore::WebGLRenderingContextBase::isShader):
(WebCore::WebGLRenderingContextBase::isTexture):
(WebCore::WebGLRenderingContextBase::linkProgram):
(WebCore::WebGLRenderingContextBase::linkProgramWithoutInvalidatingAttribLocations):
(WebCore::WebGLRenderingContextBase::shaderSource):
(WebCore::WebGLRenderingContextBase::useProgram):
(WebCore::WebGLRenderingContextBase::validateProgram):
(WebCore::WebGLRenderingContextBase::setFramebuffer):
(WebCore::WebGLRenderingContextBase::validateNullableWebGLObject): Deleted.
(WebCore::WebGLRenderingContextBase::validateWebGLObject): Deleted.
(WebCore::WebGLRenderingContextBase::validateWebGLProgramOrShader): Deleted.
* Source/WebCore/html/canvas/WebGLRenderingContextBase.h:
(WebCore::WebGLRenderingContextBase::validateWebGLObject):
(WebCore::WebGLRenderingContextBase::validateNullableWebGLObject):
(WebCore::WebGLRenderingContextBase::validateIsWebGLObject const):
* Source/WebCore/html/canvas/WebGLSampler.h:
* Source/WebCore/html/canvas/WebGLShader.h:
* Source/WebCore/html/canvas/WebGLSync.h:
* Source/WebCore/html/canvas/WebGLTexture.cpp:
(WebCore::WebGLTexture::WebGLTexture):
(WebCore::WebGLTexture::didBind):
(WebCore::WebGLTexture::setTarget): Deleted.
* Source/WebCore/html/canvas/WebGLTexture.h:
* Source/WebCore/html/canvas/WebGLTimerQueryEXT.h:
* Source/WebCore/html/canvas/WebGLTransformFeedback.h:
* Source/WebCore/html/canvas/WebGLVertexArrayObjectBase.h:
(WebCore::WebGLVertexArrayObjectBase::didBind):
(WebCore::WebGLVertexArrayObjectBase::isUsable const):
(WebCore::WebGLVertexArrayObjectBase::isInitialized const):
(WebCore::WebGLVertexArrayObjectBase::hasEverBeenBound const): Deleted.
(WebCore::WebGLVertexArrayObjectBase::setHasEverBeenBound): Deleted.
* Source/WebCore/inspector/InspectorShaderProgram.cpp:
(WebCore::InspectorShaderProgram::updateShader):

Canonical link: https://commits.webkit.org/267121@main
  • Loading branch information
kkinnunen-apple committed Aug 22, 2023
1 parent 8d79b38 commit f13b4be
Show file tree
Hide file tree
Showing 26 changed files with 351 additions and 334 deletions.
14 changes: 4 additions & 10 deletions Source/WebCore/html/canvas/EXTDisjointTimerQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,8 @@ GCGLboolean EXTDisjointTimerQuery::isQueryEXT(WebGLTimerQueryEXT* query)
if (isContextLost())
return false;
auto& context = this->context();

if (!query || !query->validate(context))
return false;

if (query->isDeleted())
if (!context.validateIsWebGLObject(query))
return false;

return context.graphicsContextGL()->isQueryEXT(query->object());
}

Expand All @@ -112,7 +107,7 @@ void EXTDisjointTimerQuery::beginQueryEXT(GCGLenum target, WebGLTimerQueryEXT& q

Locker locker { context.objectGraphLock() };

if (!context.validateWebGLObject("beginQueryEXT", &query))
if (!context.validateWebGLObject("beginQueryEXT", query))
return;

// The WebGL extension requires ending time elapsed queries when they are deleted.
Expand All @@ -134,7 +129,6 @@ void EXTDisjointTimerQuery::beginQueryEXT(GCGLenum target, WebGLTimerQueryEXT& q
return;
}

query.setTarget(target);
context.m_activeQuery = &query;

context.graphicsContextGL()->beginQueryEXT(target, query.object());
Expand Down Expand Up @@ -176,7 +170,7 @@ void EXTDisjointTimerQuery::queryCounterEXT(WebGLTimerQueryEXT& query, GCGLenum
if (!context.scriptExecutionContext())
return;

if (!context.validateWebGLObject("queryCounterEXT", &query))
if (!context.validateWebGLObject("queryCounterEXT", query))
return;

if (target != GraphicsContextGL::TIMESTAMP_EXT) {
Expand Down Expand Up @@ -228,7 +222,7 @@ WebGLAny EXTDisjointTimerQuery::getQueryObjectEXT(WebGLTimerQueryEXT& query, GCG
return nullptr;
auto& context = this->context();

if (!context.validateWebGLObject("getQueryObjectEXT", &query))
if (!context.validateWebGLObject("getQueryObjectEXT", query))
return nullptr;

if (!query.target()) {
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/html/canvas/EXTDisjointTimerQueryWebGL2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void EXTDisjointTimerQueryWebGL2::queryCounterEXT(WebGLQuery& query, GCGLenum ta
if (!context.scriptExecutionContext())
return;

if (!context.validateWebGLObject("queryCounterEXT", &query))
if (!context.validateWebGLObject("queryCounterEXT", query))
return;

if (target != GraphicsContextGL::TIMESTAMP_EXT) {
Expand Down
9 changes: 1 addition & 8 deletions Source/WebCore/html/canvas/OESVertexArrayObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,8 @@ GCGLboolean OESVertexArrayObject::isVertexArrayOES(WebGLVertexArrayObjectOES* ar
if (isContextLost())
return false;
auto& context = this->context();
if (!arrayObject || !arrayObject->validate(context))
if (!context.validateIsWebGLObject(arrayObject))
return false;

if (!arrayObject->hasEverBeenBound())
return false;
if (arrayObject->isDeleted())
return false;

return context.graphicsContextGL()->isVertexArray(arrayObject->object());
}

Expand All @@ -112,7 +106,6 @@ void OESVertexArrayObject::bindVertexArrayOES(WebGLVertexArrayObjectOES* arrayOb
auto* contextGL = context.graphicsContextGL();
if (arrayObject && !arrayObject->isDefaultObject() && arrayObject->object()) {
contextGL->bindVertexArray(arrayObject->object());
arrayObject->setHasEverBeenBound();
context.setBoundVertexArrayObject(locker, arrayObject);
} else {
contextGL->bindVertexArray(0);
Expand Down
Loading

0 comments on commit f13b4be

Please sign in to comment.