OpenGL.Net icon indicating copy to clipboard operation
OpenGL.Net copied to clipboard

Facilitate porting code based on other libraries

Open luca-piccioni opened this issue 6 years ago • 7 comments

Introduction

This issue is created to collect various ideas about how to facilitate the porting of other code written for OpenTK, Mono and other libraries. It could be a common case to have an existing source code, and for running with OpenGL.Net it necessarily need to be modified.

This does not mean that OpenGL.Net can/should/will adopt all code conventions and API of the other libraries. This is only a point that may be sensible to the users of this library, and this is the place for discuss about it. In short, this is a chance that may apply (why not, even radical) changes.

Everyone is allowed to edit this issue by adding anything not listed below. If any user wants to discuss about the particular points, can write a reply and update it as necessary; i'll do the same.

[1] Change Gl to GL

This is most invasive, stupid but still easy to adapt change. OpenTK and Mono uses the uppercase version, instead other libraries have the camelcase convention like OpenGL.Net.

This can be work-rounded by putting an using GL = OpenGL.Gl; at the beginning of the source files. This seems the most fair trade-off.

[2] Provide methods with explicit "counter" parameters

OpenGL.Net automatically consider a "counter" parameter implicitly defined by the length of another array argument. This obviously deviates from the "raw" OpenGL API, but provide more succinct signatures and less error prone implementation. It should be safe to add "raw" method signatures.

[3] Provide methods with unsafe parameters

OpenGL.Net source code generator support the creation of methods overloads with unsafe parameters. I usually minimized the generation of this methods since the IntPtr overloads were sufficient to develop my application. Currently the exceptions are the method matching the regex ^gl(Program)?Uniform(Matrix)?(1|2|3|4|2x3|2x4|3x2|3x4|4x2|4x3)(d|f|i|ui)v$, but sincerely I could remove even them.

[4] Provide methods with ref parameters

OpenGL.Net treat pointer arguments with IntPtr and object types (with few exception on String). However, the pointer argument can be marshaled with the ref modifier, telling to the default P/Invoke marshal code to pin a pointer to the argument (which need to be a generic type, constrained to be struct).

luca-piccioni avatar Sep 01 '17 22:09 luca-piccioni

Here is my thoughts about the issue.

Change Gl to GL

This has always disturbed me. I don't remember why I've chosen the camel case convention for the project classes (it happened too many years ago). Normally acronyms are spelled with upper case tokens, and I think this should be the case.

To adapt current existing code should be sufficient a Find & Replace Gl./GL. in all files.

Provide methods with explicit "counter" parameters

I'm not a fan about this modification. While it allows user to specify only part of the array, users can introduce errors due a counter parameter greater than length of the array. For example:

uint[] buffers = new uint[4];

Gl.GenBuffers(buffers);           // It works, and user cannot be wrong
Gl.DeleteBuffers(buffers);        // It works, and user cannot be wrong

Gl.GenBuffers(1, buffers);        // It works, and user can store only 1 buffer
Gl.DeleteBuffers(1, buffers);     // It works, and user can specify only 1 buffer

Gl.GenBuffers(5, buffers);        // Memory corrupted and resource leak
Gl.DeleteBuffers(5, buffers);     // Memory corrupted and probably un-managed exception

Provide methods with unsafe parameters

I doubt that this really required for many users. If the GL command takes a pointer, it is typed as IntPtr, which can be constructed from an unsafe parameter. Additionally, a bunch of methods will be added, increasing the size of the binary image (however, this should be measured, since it could be negligible).

Provide methods with ref parameters

....

luca-piccioni avatar Sep 01 '17 23:09 luca-piccioni

[1] Yes. Acronym should be upper case only. [2] No, because of the reasons/examples @luca-piccioni stated/provided. [3] No, because of the reasons @luca-piccioni stated. [4] Could you provide concrete pros/cons?

21c-HK avatar Sep 07 '17 10:09 21c-HK

Provide methods with ref parameters

I found the emblematic use case. P/Invoke default marshaller can pass arguments by value, for free for basic system types, including IntPtr. The IntPtr arguments are normally used to pass array of blittable types (int, short, float), and the delegate is used for the object arguments (which are pinned during call).

As noted by in #60, the object arguments can cause boxing of value types, causing an extra cost of the API. Indeed the following instruction is not efficient: v would be boxed to object (Vertex4f is a blittable structure).

Vertex4f v = new Vertex4f(0.0f);
Gl.BufferData(BufferTarget.ArrayBuffer, 16, v, BufferUsage.StaticDraw)

Instead, modifying the signature with a ref, it would more efficient:

Vertex4f v = new Vertex4f(0.0f);
Gl.BufferData(BufferTarget.ArrayBuffer, 16, ref v, BufferUsage.StaticDraw);

Without the ref override, it is possible to workaround with

void SetUniform(Vertex4f v)
{
	unsafe {
		Gl.Uniform4(1, 1, (float*)&v);
	}
}

~~The signature of the functions must have a template parameter (i.e. Gl.Uniform4<T>(int, int, ref T)).~~

luca-piccioni avatar Sep 08 '17 21:09 luca-piccioni

As someone starting to rewrite the OpenGL backend in my 3D graphics library, I guess I'll give some feedback here.

[1] Change Gl to GL

I have a couple of suggestions here:

  1. If the choice is between Gl and GL, then I slightly prefer GL. Gl has the unfortunate property of looking like G-capital-i, rather than G-capital-L (if that made sense 😄).

  2. I've begun to prefer a native binding approach where the native functions are exposed with the exact same name as they are truly defined with. Then, a "using static" can be added, and the functions can be called in a natural manner. This is a more radical departure from what you have, so I'll leave the suggestion at that.

[2] Provide methods with explicit "counter" parameters

This is necessary, IMO. This ties into some other feedback I have, but suffice to say, I think it's 100% needed.

[3] Provide methods with unsafe parameters

This doesn't do much for me, personally. If there are IntPtr overloads, then it is enough. Unfortunately, it doesn't really look like there are IntPtr overloads for everything that there needs to be. For example, glGenBuffers only has two wrapped overloads:

GenBuffers(uint[]) GenBuffer()

Unless I'm missing something, there's no way to call glGenBuffers without allocating an array. That makes the API very cumbersome -- I have to manage extra temporary arrays that might be useless otherwise. At the very least, the GenBuffer overload shouldn't allocate a temporary 1-element array. It should just create a local and pass its address to the native function.

This could be avoided if all of the functions had at least one raw overload exposed.

[4] Provide methods with ref parameters

I was the one who #60 , so obviously I would love if these overloads were added. out overloads for "generate" functions would also be nice, e.g.:

glGenBuffers(uint count, out uint buffers)

That's my feedback for the 4 immediate topics. I'll probably type something up a bit more general as well.

mellinoe avatar Oct 23 '17 23:10 mellinoe

General feedback:

I actually have just a very small set of requirements, so I might not be a typical user of the library. At the moment, I am planning to just build a very minimal wrapper library myself, for use in my graphics library internally. I'm just going to wrap the stuff that I use, and not make an attempt to wrap everything. I think MonoGame is doing something similar ATM.

Stuff that I'm looking for in bindings:

  • Allocation-less: It needs to be possible to do everything without allocating any GC'd objects. This means that non-array overloads should be available (either ref-like, or raw pointers), and the library shouldn't allocate any temporary arrays or other temporary objects on my behalf. Also, there shouldn't be any boxing. This might seem silly, but I just recently profiled my renderer, and found that a stray allocation (actually caused by a C# compiler bug) in a function called many times per frame was responsible for a huge amount of GC pressure compared with everything else. If any individual OpenGL functions that are called many times per frame are also doing that, then it's a big problem.
  • Close to "native". I'm not really interested in the higher-order stuff here -- I just want the raw bindings. I want to handle all memory allocation and resource disposal myself. Like I said though, I might not be the typical user. I'm essentially trying to "hide" OpenGL and other graphics API's from the user.
  • "Policy-less". I would actually prefer if the function bindings were separated out from everything else, like the math/color structures, etc. Maybe they could be in a separate library, but I'm guessing they are there so that GL overloads can be generated for them. This one isn't a huge deal and wouldn't leak out of my library. Ideally, calling GL functions won't require me to cast to or from those structures, though.
  • .NET Standard support. I know this is already checked in, but I'd love to be able to just grab it off of NuGet for simplicity.

Apologies if this is the wrong channel for feedback. As an interested party, this seemed like an okay place to comment.

mellinoe avatar Oct 23 '17 23:10 mellinoe

@mellinoe

  • Allocation-less: generally speaking the wrapper methods really try to avoid allocation of temporary objects. The Gen* commands returning a single GL object were loosely based on the complete "array"wrapper (now changed by the above commit), and this is fixed. The only cases that come up in my mind are the "object" overrides, which allocates a GCHandle object onto the stack for each object parameter: however, "object" overrides are always generated because a special "IntPtr" argument, so it should be easy to avoid these methods.
  • Policy-less: I've always procrastinated on demerging math data structures from OpenGL.Net; effectively they was always been decoupled from the GL commands, and any user could use any compatible math library out there without any issue (except having 200 KB of unused code on OpenGL.Net, or name collisions). This is fixed now by the commit above.
  • Close to "native": I really don't understand the point here. The wrappers are quite minimal, and the only simplifications or abstractions are derived directly from the XML specification, and they does not affect the "raw" execution.
  • .NET Standard support: before having a NuGet package I need to resolve some "bad behavior" of VS 2017 having multiple .NET Standard projects. It fails to compile because it cannot resolve framework types.

If you have any other critic/doubt/question about, you are free to open new issues as you wish: this makes issue tracking easier.

luca-piccioni avatar Oct 31 '17 09:10 luca-piccioni

The two changes above look great. As for "close to native", I guess all that means for me is that I'm not interested in the higher-level object-oriented stuff (OpenGL.Net.Objects). But that's not to say it wouldn't be useful for someone else -- just trying to clarify what I'd like from the library.

If you have any other critic/doubt/question about, you are free to open new issues as you wish: this makes issue tracking easier.

Will do. Thanks for the discussion!

mellinoe avatar Oct 31 '17 19:10 mellinoe