curv3d / curv

a language for making art using mathematics
Apache License 2.0
1.14k stars 73 forks source link

Remove an unnecessary check for an API handle #69

Closed elfring closed 5 years ago

elfring commented 5 years ago

An extra check is not needed for a zero API handle in the destructor for the class “Shader”.

doug-moen commented 5 years ago

That check is copypasta from the glslViewer project on github, commit 7c1459602aab11992207f61ec6ce3c2ce3e87442. The checkin comment for that change, where the if (m_program != 0) guard was added, says:

    When no command line options are specified, the program crashes under OSX with:

    (lldb) bt
    * thread #1: tid = 0x940a, 0x00007fff8b4986fd libGL.dylib`glDeleteProgram + 13, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x1280)
      * frame #0: 0x00007fff8b4986fd libGL.dylib`glDeleteProgram + 13
        frame #1: 0x0000000100019ad6 glslViewer`Shader::~Shader() [inlined] Shader::~Shader(this=<unavailable>) + 22 at shader.cpp:11
        frame #2: 0x0000000100019ac4 glslViewer`Shader::~Shader(this=<unavailable>) + 4 at shader.cpp:9
        frame #3: 0x00007fff913cf8cd libsystem_c.dylib`__cxa_finalize_ranges + 322
        frame #4: 0x00007fff913cfbd0 libsystem_c.dylib`exit + 55
        frame #5: 0x00007fff855dd5d0 libdyld.dylib`start + 8
        frame #6: 0x00007fff855dd5c9 libdyld.dylib`start + 1

    Added guard statement to avoid crash.

opengl drivers are, as a class, full of bugs. This appears to be a workaround for a driver bug on MacOS.

So I think I'll leave the code the way it is.

elfring commented 5 years ago
doug-moen commented 5 years ago

The comment is bad, and should be updated. Feel free to submit a pull request for that, if that is something that interests you. A better comment is: // avoid crash on MacOS due to driver bug

The commit comment that I pasted shows a stack trace where MacOS crashes in glDeleteProgram, and the author claims this crash occurs when mProgram==0. I don't have any good reason to disbelieve this. From what I've read, all OpenGL drivers are buggy, none of them correctly implement the full standard as documented. In the case of MacOS, Apple has explicitly deprecated OpenGL, presumably because they don't want to update the driver or fix bugs.

To give an example of the latter, one of things I'll be implementing soon is some standard noise functions for Curv. The GLSL standard includes functions called noise1(), noise2(), noise3() and noise4(). But I can't use these functions, because there is no known OpenGL driver that bothers to implement these functions. Just because these functions are in the official standard doesn't mean you can use them. I'm relying on blog posts and stackexchange for my information on this, of course.

By the way, thanks for doing a code review of my code. I'm not an OpenGL expert, so for all I know there could be lots of problems in the OpenGL parts of Curv, that I mostly copied from other sources.

elfring commented 5 years ago

Have you got any more desire to influence the software situation for mentioned standard functions in positive ways?

doug-moen commented 5 years ago

Have you got any more desire to influence the software situation for mentioned standard functions in positive ways?

That's hard to answer, because there are multiple GPU APIs, and the question is, which standard should I get involved with?

I think that the current situation for OpenGL and other GPU APIs is undesirable.

I personally think that the OpenGL API is badly designed and hard to use. It relies far too much on global variables (which are stored in the OpenGL context), and it uses integer ids instead of abstract data types. These problems are not fixable.

OpenGL is a dead end API that is not being updated. The Kronos group is focused on promoting Vulkan as the successor to OpenGL. Apple refuses to support any version of OpenGL more recent than 4.1, and I need the equivalent of OpenGL 4.3 for Curv features that I plan for later this year. WebGL 2.0 (on web browsers) is also a dead end: it doesn't support compute shaders (which I need for Curv), and it will never be updated to support compute shaders, in part because Apple refuses to update their OpenGL driver to version 4.3. FireFox has an extension to WebGL 2.0 that supports compute shaders on non-MacOS platforms, but Google has decided not to implement this extension in Chrome.

What I personally want is a single cross platform GPU API that runs on personal computers, mobile devices and web browsers. I am not happy about writing Curv back ends for multiple different GPU APIs for different platforms: that's too much complexity and maintenance work.

But the problem is that such a cross platform GPU API currently doesn't exist. OpenGL is at its end of life and OpenGL 4.3 will never be supported on MacOS. Vulkan will never run on a web browser. There will never be a WebVulkan to replace WebGL, because Vulkan is unsafe and insecure. Vulkan is also too complex for my taste. Metal is a proprietary API that only runs on MacOS. Direct X is a proprietary API that only runs on Windows.

There is a consortium that includes Mozilla, Google and Apple who are designing and implementing the WebGPU API as a successor to WebGL. This will be a modern, high level API with all of the features needed by Curv. It will be an order of magnitude easier to program for than Vulkan, and I believe also easier than OpenGL. There will be C and Rust libraries that implement this API, available for personal computer and mobile operating systems. Unfortunately, it will be a few years before this is generally available. This is the API I am waiting for, and I view my current use of OpenGL as a stop-gap measure while I wait for something better to come along.

I do not have enough GPU programming experience to make a positive contribution to the WebGPU project, at this time.

Another possibility is that I might find a high level library that implements a cross platform GPU API on top of OpenGL, Vulkan, Metal and DirectX. Ideally, this library should be available in native form (C/C++/Rust) and also in web form (Javascript). Because Curv is being ported to run in a web browser by @sebastien, but there will continue to be a native version.

I think there are a few WebGPU libraries already in the process of being implemented, as layers on top of the older APIs, even though the design is not finalized. It's possible I could use one of these libraries before WebGPU becomes generally available as a native API in web browsers. WebGPU is still being actively designed, and I have to wait until all the design decisions have been finalized before trying to use one of open source implementations. I don't know how long that will take.

doug-moen commented 5 years ago

comment is fixed.