Closed Bug 844698 Opened 9 years ago Closed 9 years ago

Make Qt EGL port working on X11 Maemo/Meego after bug 716859 landed

Categories

(Core :: Graphics, defect)

ARM
MeeGo
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: romaxa, Assigned: romaxa)

References

Details

Attachments

(1 file)

commit http://hg.mozilla.org/mozilla-central/rev/3a7d4085787e#l12.711 - broke Qt port build
not sure why that line was changed...

Also we need to refresh EGLContext eglContext after aWidget->HasGLContext call
Summary: Fix Qt EGL port build after bug 716859 landed → Make Qt EGL port working on X11 Maemo/Meego after bug 716859 landed
1) Failed to create EGLContext - not detected when sharedContext = null and first attempt failed, later causing GLContext created without internal context, and painful debugging. also added some warnings in other places.
2) Fix back EGL context initialization around QGL context (using depth and simple convert from QGLFormat to SurfaceCaps)
3) Fix arguments API for GLContextEGL::CreateGLContext, 
http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContextProviderEGL.cpp?mark=2293,2146#2146
Assignee: nobody → romaxa
Attachment #718847 - Flags: review?(jgilbert)
Part 3) fixing bug 844715 as well
Blocks: 844715
Comment on attachment 718847 [details] [diff] [review]
Fix EGL X11 build Qt Platform

Review of attachment 718847 [details] [diff] [review]:
-----------------------------------------------------------------

A bunch of nits, but otherwise good. Please fix before pushing.

::: gfx/gl/GLContextProviderEGL.cpp
@@ +2038,5 @@
>  
>      EGLContext eglContext = sEGLLibrary.fGetCurrentContext();
>      if (aWidget->HasGLContext() && eglContext) {
> +        // HasGLContext might change current context to real one, so get it here again
> +        eglContext = sEGLLibrary.fGetCurrentContext();

We should change this so we don't have to call this twice. Something like:
foo = HasGLContext()
bar = GetCurrentContext()
if (foo && bar)...

@@ +2053,2 @@
>          platformContext = context;
> +        caps.bpp16 = depth == 16 ? true : false;

Parens around `(depth == 16)` is preferable to the redundant `? true : false`.

@@ +2223,5 @@
>  
>      if (!sEGLLibrary.fChooseConfig(EGL_DISPLAY(),
>                                     sEGLLibrary.HasKHRLockSurface() ?
>                                         pixmap_lock_config : pixmap_config,
> +                                   configs, numConfigs, &numConfigs) || numConfigs == 0) {

Prefer '{' on the next line if the conditional is multi-line.

@@ +2295,5 @@
>  
>      GLContextEGL* shareContext = GetGlobalContextEGL();
>      SurfaceCaps dummyCaps = SurfaceCaps::Any();
>      nsRefPtr<GLContextEGL> glContext =
>      GLContextEGL::CreateGLContext(dummyCaps,

Indent these RHS expressions, since we're here.

@@ +2297,5 @@
>      SurfaceCaps dummyCaps = SurfaceCaps::Any();
>      nsRefPtr<GLContextEGL> glContext =
>      GLContextEGL::CreateGLContext(dummyCaps,
>                                    shareContext, true,
> +                                  config, surface);

Hah, yes indeed.
Attachment #718847 - Flags: review?(jgilbert) → review+
backed out for bustage https://hg.mozilla.org/integration/mozilla-inbound/rev/0a6610b9c2fb this some how broke push log too!
/tools/android-ndk-r8c/toolchains/x86-4.6/prebuilt/linux-x86/bin/../lib/gcc/i686-linux-android/4.6/../../../../i686-linux-android/bin/as: /lib/libz.so.1: no version information available (required by /tools/android-ndk-r8c/toolchains/x86-4.6/prebuilt/linux-x86/bin/../lib/gcc/i686-linux-android/4.6/../../../../i686-linux-android/bin/as)
../../../gfx/gl/GLContextProviderEGL.cpp: In static member function 'static already_AddRefed<mozilla::gl::GLContext> mozilla::gl::GLContextProviderEGL::CreateForWindow(nsIWidget*)':
../../../gfx/gl/GLContextProviderEGL.cpp:2040:13: error: unused variable 'depth' [-Werror=unused-variable]
cc1plus: all warnings being treated as errors
https://hg.mozilla.org/mozilla-central/rev/249ff1a50e40
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.