[webkit-reviews] review granted: [Bug 178254] Performance: Skip texture upload if source image and destination texture haven't changed : [Attachment 323720] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 13 12:17:12 PDT 2017
Dean Jackson <dino at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 178254: Performance: Skip texture upload if source image and destination
texture haven't changed
https://bugs.webkit.org/show_bug.cgi?id=178254
Attachment 323720: Patch
https://bugs.webkit.org/attachment.cgi?id=323720&action=review
--- Comment #7 from Dean Jackson <dino at apple.com> ---
Comment on attachment 323720
--> https://bugs.webkit.org/attachment.cgi?id=323720
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=323720&action=review
I'm not sure I like the "seed" terminology, but this is ok for now. I'd prefer
it if all this state could be tracked directly inside VideoTextureCopierCV
using a separate context, but we can do that later.
>> Source/WTF/wtf/UnsafePointer.h:34
>> +template<typename T>
>
> Couldn't this just be a void* wrapper, and not even templatized? Or do you
care about pointer type?
Yeah, I'm not sure this is needed in this bug. Maybe it could be a separate
patch and just use IOSurfaceRef for now.
> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.h:113
> + UnsafePointer<IOSurfaceRef> m_lastSurface;
I think we can just use a raw IOSurfaceRef here.
> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:264
> + ::glBindTexture(GL_TEXTURE_2D, m_state.boundTarget(GL_TEXTURE0) ==
GL_TEXTURE_2D ? m_state.boundTexture(GL_TEXTURE0) : 0);
Since boundTexture() will return 0 if the value doesn't exist, I don't think
you need the conditional.
>
Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:1937
> + m_state.textureSeedCount.add(o);
Has it been seeded at this point? I don't think so.
More information about the webkit-reviews
mailing list