[webkit-reviews] review granted: [Bug 234293] [GTK] Fix ANGLE initialisation and drawing : [Attachment 447122] Patch

[bugzilla-daemon at webkit.org]

Kimmo Kinnunen <kkinnunen at apple.com> has granted Chris Lord
<clord at igalia.com>'s request for review:
Bug 234293: [GTK] Fix ANGLE initialisation and drawing
https://bugs.webkit.org/show_bug.cgi?id=234293

Attachment 447122: Patch

https://bugs.webkit.org/attachment.cgi?id=447122&action=review




--- Comment #2 from Kimmo Kinnunen <kkinnunen at apple.com> ---
Comment on attachment 447122
  --> https://bugs.webkit.org/attachment.cgi?id=447122
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=447122&action=review

I suppose it's ok to prevent crashes. I think the commit message could be
better and optimally there should be no additional ifdefs

> Source/WebCore/ChangeLog:8
> +	   Fix ANGLE initialisation and drawing buffer binding on GTK.

If it's intended to initialize using EGL specifically, then the commit message
could be more verbose as to explain what's happening.

So below I'll recommend using shared default initialisation, but I don't
exactly know why it doesn't work for GTK ..

> Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.cpp:77
> +	   EGL_PLATFORM_ANGLE_TYPE_ANGLE,
EGL_PLATFORM_ANGLE_TYPE_OPENGLES_ANGLE,

Without any further knowledge, it's hard to understand why GLES is forced. I'd
imagine would be better to use the default one..

Would EGL_PLATFORM_ANGLE_TYPE_ANGLE == EGL_PLATFORM_ANGLE_TYPE_DEFAULT_ANGLE
work?
if so, you can call GetPlatformDisplayExt and provide nullptr
displayAttributes.

> Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.cpp:81
> +    EGLDisplay display = EGL_GetPlatformDisplay(EGL_PLATFORM_ANGLE_ANGLE,
EGL_DEFAULT_DISPLAY, displayAttributes);

Could you call GetPlatformDisplayExt?

> Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.cpp:84
> +#endif

If the above would work, you could try if all ports using this file could share
the initialisation.

EGL_GetDisplay is implemented with same code as  GetPlatformDisplayExt