<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - Prefer eglGetPlatformDisplay to eglGetDisplay"
href="https://bugs.webkit.org/show_bug.cgi?id=163333">bug 163333</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #291358 Flags</td>
<td>
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Prefer eglGetPlatformDisplay to eglGetDisplay"
href="https://bugs.webkit.org/show_bug.cgi?id=163333#c1">Comment # 1</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Prefer eglGetPlatformDisplay to eglGetDisplay"
href="https://bugs.webkit.org/show_bug.cgi?id=163333">bug 163333</a>
from <span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=291358&action=diff" name="attach_291358" title="eglGetPlatformDisplay.patch">attachment 291358</a> <a href="attachment.cgi?id=291358&action=edit" title="eglGetPlatformDisplay.patch">[details]</a></span>
eglGetPlatformDisplay.patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=291358&action=review">https://bugs.webkit.org/attachment.cgi?id=291358&action=review</a>
Hi, thanks for the patch. A couple of comments on submitting patches to WebKit in general:
* Be sure to select the WebKit Gtk Bugzilla component, or us Linux folks may not notice your patch for a very long time. I found this by luck, because I happened to check the Fedora git repo to see if Tom had packaged 2.14.1 yet. :)
* Also be sure to set the r? and cq? Bugzilla flags for patches if you want them to be reviewed or committed.
* The patch requires a changelog entry ('Tools/Scripts/PrepareChangeLog -b 291358').
* The patch must be prepared against a git checkout rather than a release tarball.
* Easiest way to submit the patch is to use 'Tools/Scripts/webkit-patch upload --request-commit', it will pick the bug number automatically from your changelog entry
'webkit-patch upload' would have warned you about dozens of code style errors, including:
* The pointer star always goes on the type, not the variable name. "const char* client_extensions" rather than "const char *client_extensions"
* No space before opening parentheses in function calls. "eglQueryString(" not "eglQueryString ("
* Use nullptr rather than NULL
* Use C++ casts, never C style casts. Prefer static_cast and use reinterpret_cast when that fails.
* Always indent four spaces, no tabs.
As for how to deduplicate the code, I think the best option would be to use a protected static function in PlatformDisplay. The child class would pass either EGL_PLATFORM_WAYLAND_KHR or EGL_PLATFORM_X11_KHR and get back a value that can be assigned to m_eglDisplay.
Lastly, we have another open <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Avoid strstr() when checking (E)GL extensions"
href="show_bug.cgi?id=161958">bug #161958</a> about how strstr is not the correct way to check for EGL extensions; seems like now would be a good time to tighten that up.
<span class="quote">> webkitgtk-2.14.0/Source/WebCore/platform/graphics/egl/GLContextEGL.h:27
> +#include <EGL/eglext.h></span >
Is this an accident? You don't change anything else in this file, so I presume this include was meant to be added to PlatformDisplayWayland.cpp instead. It's important to minimize the number of include statements in header files since they can dramatically increase build time.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>