Clean-up some things in GraphicsContext3D
<rdar://problem/33281257>
Created attachment 315326 [details] Patch
Requires 174345 to apply cleanly.
Comment on attachment 315326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315326&action=review > Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:-576 > -#if USE(COORDINATED_GRAPHICS_THREADED) > - ::glDeleteTextures(1, &m_compositorTexture); > -#endif Why can this go away? > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:315 > + ASSERT(width >= 0 && height >= 0); Can this assert fire if JS passes bad values from script? If so, don't assert.
(In reply to Simon Fraser (smfr) from comment #4) > Comment on attachment 315326 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=315326&action=review > > > Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:-576 > > -#if USE(COORDINATED_GRAPHICS_THREADED) > > - ::glDeleteTextures(1, &m_compositorTexture); > > -#endif > > Why can this go away? Mentioned in the ChangeLog :) We don't use COORDINATED_GRAPHICS_THREADED on Apple platforms. > > > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:315 > > + ASSERT(width >= 0 && height >= 0); > > Can this assert fire if JS passes bad values from script? If so, don't > assert. I don't really understand why these were int to begin with. You can't have a canvas with negative dimensions. I expect that we catch it in the bindings.
Committed r219473: <http://trac.webkit.org/changeset/219473>
Comment on attachment 315326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315326&action=review >>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:315 >>> + ASSERT(width >= 0 && height >= 0); >> >> Can this assert fire if JS passes bad values from script? If so, don't assert. > > I don't really understand why these were int to begin with. You can't have a canvas with negative dimensions. I expect that we catch it in the bindings. General WebKit style note: We try not to write a single assertion with &&. Instead we write two assertions on separate lines, so we can tell which one failed.
Comment on attachment 315326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315326&action=review >>>> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:315 >>>> + ASSERT(width >= 0 && height >= 0); >>> >>> Can this assert fire if JS passes bad values from script? If so, don't assert. >> >> I don't really understand why these were int to begin with. You can't have a canvas with negative dimensions. I expect that we catch it in the bindings. > > General WebKit style note: We try not to write a single assertion with &&. Instead we write two assertions on separate lines, so we can tell which one failed. Thanks Darin.