projectM-visualizer / projectm

projectM - Cross-platform Music Visualization Library. Open-source and Milkdrop-compatible.
https://discord.gg/mMrxAqaa3W
GNU Lesser General Public License v2.1
3.37k stars 375 forks source link

GL_INVALID_ENUM error on transition to the first preset #823

Closed hack-s closed 3 months ago

hack-s commented 4 months ago

Please confirm the following points:

Affected Project

libprojectM (including the playlist library)

Affected Version

master branch

Operating Systems and Architectures

Linux (x86_64)

Build Tools

Compiler: GNU GCC

Additional Project, OS and Toolset Details

c++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0

Type of Defect

Specific bug in projectM code (please link the code in question)

Log Output

No response

Describe the Issue

GL_INVALID_ENUM error present in OpenGL context via glGetError() on transition to the first/initial preset when the output texture from the current preset (initialized empty at that point) is attempted to being copied.

The error is caused by this line when m_target = 0: https://github.com/projectM-visualizer/projectm/blob/master/src/libprojectM/Renderer/Texture.cpp#L58

The error does not cause any functional issue as far as I can tell, it just trips my error handler.

hack-s commented 4 months ago

Here is my workaround for this issue, not sure if that is the preferred way to fix it though. https://github.com/projectM-visualizer/projectm/pull/824

kblaschke commented 4 months ago

This error is almost always happening because the OpenGL context isn't properly set up, as these OpenGL calls are the first ones being executed when initializing projectM.

To make sure this doesn't happen because of such an issue, please check:

hack-s commented 4 months ago

To be more precise, the issue is happening when transitioning from the idle preset idle://Geiss & Sperl - Feedback (projectM idle HDR mix).milk to the first actual file based preset.

I confirmed this issue is present (on my system) when using the latest frontend-sdl2 from master, tested both, X11 and Wayland on Ubuntu 22.04. OpenGL context setup looks good to me. Detection of the bug is easy to reproduce by adding a minimal GL error handler like the following to the frontend-sdl2 RenderLoop.cpp Run() method, e.g. L51:

    while (true)
    {
        auto error = glGetError();

        switch (error)
        {
            case GL_NO_ERROR:
                break;
            case GL_INVALID_ENUM:
                std::cerr << "OpenGL Error: GL_INVALID_ENUM - Enumeration parameter is not legal" << std::endl;
                exit(1);
            default:
                std::cerr << "OpenGL Error: Error code - 0x" << error << std::endl;
                continue;
        }
        break;
    }

Note requires and additional include: #include <GL/gl.h>

hack-s commented 4 months ago

I have a hard time verifying that it is in fact using GLX on X11. It links the right libs, and the only EGL lib is for Wayland I believe.

libOpenGL.so.0 => /lib/x86_64-linux-gnu/libOpenGL.so.0 (0x000079224c061000)
libwayland-egl.so.1 => /lib/x86_64-linux-gnu/libwayland-egl.so.1 (0x000079224b71b000)
libGLX.so.0 => /lib/x86_64-linux-gnu/libGLX.so.0 (0x000079224b59e000)
libGLdispatch.so.0 => /lib/x86_64-linux-gnu/libGLdispatch.so.0 (0x000079224b4e6000)

Looking at strace is somewhat inconclusive, but it does look like it's is using GLX.

openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libGLX.so.0", O_RDONLY|O_CLOEXEC) = 3
writev(5, [{iov_base="b0\3\0\3\0\1\0", iov_len=8}, {iov_base="GLX", iov_len=3}, {iov_base="\0", iov_len=1}], 3) = 12
writev(5, [{iov_base="b0\3\0\3\0\1\0", iov_len=8}, {iov_base="GLX", iov_len=3}, {iov_base="\0", iov_len=1}], 3) = 12
openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libGLX_mesa.so.0", O_RDONLY|O_CLOEXEC) = 9
writev(5, [{iov_base="b\23\3\0\3\0\0\0", iov_len=8}, {iov_base="GLX", iov_len=3}, {iov_base="\0", iov_len=1}], 3) = 12
writev(5, [{iov_base="b\0\3\0\3\0\0\0GLX\0", iov_len=12}], 1) = 12
read(10, "ension_override\" value=\"-GLX_EXT"..., 4096) = 4096
read(10, "ension_override\" value=\"-GLX_EXT"..., 4096) = 4096
read(10, "ension_override\" value=\"-GLX_EXT"..., 4096) = 4096
read(11, "ension_override\" value=\"-GLX_EXT"..., 4096) = 4096
read(11, "ension_override\" value=\"-GLX_EXT"..., 4096) = 4096
[pid 28374] writev(5, [{iov_base="b\4\3\0\3\0`\4", iov_len=8}, {iov_base="GLX", iov_len=3}, {iov_base="\0", iov_len=1}], 3) = 12

And glxinfo

Extended renderer info (GLX_MESA_query_renderer):
    Vendor: AMD (0x1002)
    Device: AMD Radeon RX 580 Series (polaris10, LLVM 15.0.7, DRM 3.54, 6.5.0-41-generic)
    Version: 23.2.1
    Accelerated: yes
    Video memory: 8192MB
    Unified memory: no
    Preferred profile: core (0x1)
    Max core profile version: 4.6
    Max compat profile version: 4.6
    Max GLES1 profile version: 1.1
    Max GLES[23] profile version: 3.2
kblaschke commented 3 months ago

Thanks for the information! Found and fixed the issue adding proper emptiness checks to CopyTexture, as presets don't have an output texture allocated before rendering the first frame. This may happen to other copy operations in future additions, so having this in the CopyTexture class made more sense to me.

This will also fix issues seen with other integrations, which probably suffered from this bug as well.

We'll push out a 4.1.2 bugfix release for this one, as this is an important fix.