Summary: | Query EGL_BIND_TO_TEXTURE_TARGET_ANGLE to determine 2D/rectangular texture usage | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kenneth Russell <kbr> | ||||||||||
Component: | WebGL | Assignee: | Kyle Piddington <kpiddington> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dino, ews-watchlist, gman, kbr, kkinnunen, kondapallykalyan, kpiddington, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Kenneth Russell
2021-10-08 18:21:12 PDT
Taking this one into my queue :) Created attachment 441265 [details]
Patch
Comment on attachment 441265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441265&action=review Good work overall. Please rebase and let's see green EWS runs before approving. > Source/WebCore/ChangeLog:6 > + Refactor GrraphicsContextGLCocoa to hold a reference to Grr -> Gr :) But: GraphicsContextGLOpenGLCocoa Also, please indent to match. > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:473 > + EGL_GetConfigAttrib(m_displayObj, m_configObj, EGL_BIND_TO_TEXTURE_TARGET_ANGLE, &m_drawingBufferTextureTarget); Are we guaranteed to only call this when m_displayObj and m_configObj are valid? If there's any confusion then let's consider doing this upon display initialization and have any asserts possible about not calling this on a destroyed GraphicsContextGLOpenGL. Created attachment 441412 [details]
Patch
Comment on attachment 441412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441412&action=review Looks good to me. Couple of small comments about the asserts. r+ > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:471 > + ASSERT(!"Invalid enum returned from EGL_GetConfigAttrib"); This is the first I've seen this trick used to incorporate a message. Consider ASSERT_WITH_MESSAGE(false, ...). > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:484 > + ASSERT(!"Invalid drawing target"); Same here. > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:497 > + assert(!"Invalid drawing target"); Please use WebKit's ASSERT macro. Created attachment 441660 [details]
Patch
Comment on attachment 441660 [details]
Patch
Still looks good. r+ again
Created attachment 441693 [details]
Patch for landing
Committed r284438 (243200@main): <https://commits.webkit.org/243200@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441693 [details]. |