Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy egl diagnostics from GLContextEGL.cpp #14561

Closed
6 tasks
boblun opened this issue Oct 10, 2018 · 8 comments
Closed
6 tasks

Lazy egl diagnostics from GLContextEGL.cpp #14561

boblun opened this issue Oct 10, 2018 · 8 comments
Labels

Comments

@boblun
Copy link

boblun commented Oct 10, 2018

Bug report

Describe the bug

Here is a clear and concise description of what the problem is:

The Diagnostics in GLContext.cpp rarely include the return from eglGetError and when they do return an opaque numeric value.

GLContextEGL:~157 emits the exact same diagnostic as line 163

Expected Behavior

Here is a clear and concise description of what was expected to happen:

Debug output should include the egl error on failure

Actual Behavior

EGL errors do not show textual reasons why egl call fails.
EGL Errors should clearly localise where the failure happens and the proper context

Possible Fix

I added the below private method to class CGLContextEGL

----- Added method ======>

std::string CGLContextEGL::GetEGLErrorString()
{
int error= eglGetError();
switch(error)
{
case EGL_SUCCESS: return "No erro";
case EGL_NOT_INITIALIZED: return "EGL not initialized or failed to initialize";
case EGL_BAD_ACCESS: return "Resource inaccessible";
case EGL_BAD_ALLOC: return "Cannot allocate resources";
case EGL_BAD_ATTRIBUTE: return "Unrecognized attribute or attribute value";
case EGL_BAD_CONTEXT: return "Invalid EGL context";
case EGL_BAD_CONFIG: return "Invalid EGL frame buffer configuration";
case EGL_BAD_CURRENT_SURFACE: return "Current surface is no longer valid";
case EGL_BAD_DISPLAY: return "Invalid EGL display";
case EGL_BAD_SURFACE: return "Invalid surface";
case EGL_BAD_MATCH: return "Inconsistent arguments";
case EGL_BAD_PARAMETER: return "Invalid argument";
case EGL_BAD_NATIVE_PIXMAP: return "Invalid native pixmap";
case EGL_BAD_NATIVE_WINDOW: return "Invalid native window";
case EGL_CONTEXT_LOST: return "Context lost";
}
std::string s = std::to_string(error);
return "Unknown error "+s;
}
<========

Changed duplicate diagnostic message at GLContextEGL:~163 - Note the addition of textual errors from GetEGLErrorString. All other EGL Errors should attach a similar error description.

CLog::Log(LOGERROR, "failed to bind egl API %s",this->GetEGLErrorString())

To Reproduce

Steps to reproduce the behavior:

  1. generate an EGL error and look at debug log

Debuglog

The debuglog can be found here:
...
10:35:34.813 T:548446380048 NOTICE: Using visual 0x21
10:35:34.813 T:548446380048 ERROR: failed to initialize egl <== actually refers to failure to bind API - No Indication of WHY
10:35:34.814 T:548446380048 WARNING: Visual 0x21 of the window is not suitable, looking for another one...
10:35:34.814 T:548446380048 ERROR: GLX Error: vInfo is NULL!
...

Screenshots

Here are some links or screenshots to help explain the problem:

Additional context or screenshots (if appropriate)

Here is some additional context or explanation that might help:

Your Environment

Used Operating system:

  • Android

  • iOS

  • [X ] Linux

  • OSX

  • Raspberri-Pi

  • Windows

  • Windows UWP

  • Operating system version/name:
    ubuntu 18.04 /aarch64

  • Kodi version:
    Leia

note: Once the issue is made we require you to update it with new information or Kodi versions should that be required.
Team Kodi will consider your problem report however, we will not make any promises the problem will be solved.

@lrusak
Copy link
Contributor

lrusak commented Oct 10, 2018

STOP! You do not need to open a new issue for three of the same problems. I already told you that you're configuration is unsupported.

see, #14559 (comment)

@lrusak lrusak closed this as completed Oct 10, 2018
@lrusak lrusak added the Resolution: Not applicable issue is not relevant to code in this repo and is not an external issue label Oct 10, 2018
@boblun
Copy link
Author

boblun commented Oct 10, 2018

They are not the same problem related but not the same - and are you really trying to tell me that despite people like me trying to help get Kodi Running under X11 on embedded platforms that now support some sort of acceleration under X11 you are not prepared to help?

@lrusak
Copy link
Contributor

lrusak commented Oct 10, 2018

We do not support OpenGLES under x11. It was removed a while ago.

If you want to use OpenGLES you can use Wayland or GBM.

@yol
Copy link
Member

yol commented Oct 10, 2018

@boblun Although it is not a bug, we would be happy to include EGL error strings in the log. It would be cool if you could do a pull request following this outline:

  • Add error code resolution to CEGLUtils::LogError - we probably don't need it anywhere else; please also keep the error code (in hexadecimal notation preferably) in the log message
  • Put the statements after the switch in your example into the default clause of the switch
  • If you find EGL errors that are logged not using CEGLUtils::LogError like the one you wrote here - change that so that it uses LogError if possible

@MartijnKaijser MartijnKaijser added the Ignored rules issue that does not follow the rules (no template, missing debug log, ...) label Oct 25, 2018
@yol yol added Component: Logging and removed Ignored rules issue that does not follow the rules (no template, missing debug log, ...) Resolution: Not applicable issue is not relevant to code in this repo and is not an external issue labels Nov 17, 2018
@yol
Copy link
Member

yol commented Nov 17, 2018

#14772 will add strings to EGL error messages

@yol yol reopened this Nov 17, 2018
@yol yol added the v18 Leia label Nov 17, 2018
@lrusak
Copy link
Contributor

lrusak commented Dec 13, 2018

Logging was added however X11 does not use CEGLContextUtils.

The best we can do here is check if OpenGLES was selected in Cmake and bail before the configuration is ever finished

@yol
Copy link
Member

yol commented Dec 14, 2018

@lrusak As X11 will not use CEGLContextUtils in the near future, do we want to close this issue nevertheless? (I mainly reopened it to properly close it when #14772 is merged, which has happened in the meantime)

@boblun
Copy link
Author

boblun commented Dec 14, 2018 via email

@phunkyfish phunkyfish added the Issue Cleanup: Reporter Closed Reporter requested or closed the issue. label Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants