[webkit-reviews] review granted: [Bug 205823] [WebGL 2] Implement transform feedback and pass transform feedback conformance tests : [Attachment 387307] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jan 10 12:37:08 PST 2020
Dean Jackson <dino at apple.com> has granted Justin Fan <justin_fan at apple.com>'s
request for review:
Bug 205823: [WebGL 2] Implement transform feedback and pass transform feedback
conformance tests
https://bugs.webkit.org/show_bug.cgi?id=205823
Attachment 387307: Patch
https://bugs.webkit.org/attachment.cgi?id=387307&action=review
--- Comment #6 from Dean Jackson <dino at apple.com> ---
Comment on attachment 387307
--> https://bugs.webkit.org/attachment.cgi?id=387307
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=387307&action=review
You're going to have to rebase, sorry.
> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:105
> + // Remove all references to WebGLObjects so if they are the last
reference
> + // they will be freed before the last context is removed from the
context group.
I don't think we need this comment.
> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:331
> - memcpy(static_cast<char*>(dstData->baseAddress()) +
dstData->byteOffset() + dstOffset * elementSize, ptr, copyLength *
elementSize);
> + if (ptr)
> + memcpy(static_cast<char*>(dstData->baseAddress()) +
dstData->byteOffset() + dstOffset * elementSize, ptr, copyLength *
elementSize);
> +
If ptr is null, wouldn't we want to throw an error?
> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1100
> + auto addResult = m_activeQueries.add(targetKey, makeRefPtr(&query));
> +
> + if (!addResult.isNewEntry) {
You can do this in one line - no need for addResult.
> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1268
> + synthesizeGLError(GraphicsContextGL::INVALID_OPERATION,
"bindTransformFeedback", "cannot bind a deleted Transform Feedback object");
Nit: Transform Feedback should probably be one word.
> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:2541
> + size_t index = m_boundTransformFeedbackBuffers.find(buffer);
> + if (index < m_boundTransformFeedbackBuffers.size())
> + m_boundTransformFeedbackBuffers[index] = nullptr;
Why can't you call m_boundTransformFeedbackBuffers.removeAll(buffer) ?
> Source/WebCore/platform/graphics/GraphicsContextGL.h:1068
> + // WebGL2 Transform Feedback calls
> + virtual PlatformGLObject createTransformFeedback() = 0;
> + virtual void deleteTransformFeedback(PlatformGLObject) = 0;
> + virtual GCGLboolean isTransformFeedback(PlatformGLObject) = 0;
> + virtual void bindTransformFeedback(GCGLenum, PlatformGLObject) = 0;
> + virtual void beginTransformFeedback(GCGLenum) = 0;
> + virtual void endTransformFeedback() = 0;
> + virtual void transformFeedbackVaryings(PlatformGLObject, const
Vector<String>&, GCGLenum) = 0;
> + virtual void getTransformFeedbackVarying(PlatformGLObject, GCGLuint,
ActiveInfo&) = 0;
> +
> + virtual void bindBufferBase(GCGLenum, GCGLuint, PlatformGLObject) = 0;
> +
I think I landed all these just moments ago.
More information about the webkit-reviews
mailing list