sorlok_reaves
|
|
Posted on: January 10, 2014, 04:32:46 pm |
|
|
Joined: Dec 2013
Posts: 260
|
Hello all, I've added some functions that allow ENIGMA to draw complex (self-intersecting) polygons. For example: Since this is my first contributed feature --and it's really important to me not to step on the other developers' toes-- I decided to post here for feedback before I make a pull request. The code is available here: https://github.com/sorlok/enigma-dev/tree/draw_polygonDiscussion is welcome, but please specify if any issues you have are deal-breaking issues. Everyone has different best practices, and I will try to accommodate your suggestions, but I have other features I'd like to work on too if there are no critical mistakes. In particular, here are some potential issues, none of which I consider deal-breaking. Please let me know what you think: - Currently only works for the GL1 backend. Rationale: I provide a fallback for other platforms that renders convex polygons. I can add GL3 support easily, too.
- Uses a custom class (PolyVertex) in a list to buffer calls to polygon_vertex(). Rationale: Drawing all at once minimizes GL errors (not ending the polygon) and memory leaks (not freeing temp. vertices for tessellation).
- Uses software tessellation (GLU). Rationale: If we move to hardware tessellation, we can still use this code as a fallback for GL1 and possibly GL ES (with some hacking).
- draw_polygon*() functions are in Graphics_Systems, not a plugin. Rationale: Drawing complex polygons is needed for GM5, and is (I feel) substantially different from primitives (such as triangle fans) to warrant inclusion as a main feature.
- (If I made a mistake in the GLU tessellation code, please let me know. That would certainly qualify as deal-breaking.)
Thanks in advance for your comments. If there are no deal-breaking issues, I'll prepare a pull request.
|
|
|
Logged
|
|
|
|
TheExDeus
|
|
Reply #1 Posted on: January 18, 2014, 11:44:37 am |
|
|
Joined: Apr 2008
Posts: 1860
|
Okay, so I made that change to "__stdcall" so it compiled on Windows. Then you made a change for it to compile on Linux by changing it to "CALLBACK". But that again broke in on Windows. That is why we need regression testing. This is the error (just like the one before): In file included from Graphics_Systems/OpenGL1/../General/glew.h:1169:0, from Graphics_Systems/OpenGL1/../General/OpenGLHeaders.h:22, from Graphics_Systems/OpenGL1/GLstdraw.cpp:20: c:\mingw32\i686-w64-mingw32\include\gl\glu.h:78:15: error: initializing argument 3 of 'void gluTessCallback(GLUtesselator*, GLenum, void (__attribute__((__stdcall__)) *)())' [-fpermissive] void APIENTRY gluTessCallback(GLUtesselator *tess,GLenum which,void (CALLBACK *fn)()); ^ Graphics_Systems/OpenGL1/GLstdraw.cpp:227:76: error: invalid conversion from 'void (*)()' to 'void (__attribute__((__stdcall__)) *)()' [-fpermissive] gluTessCallback(tessObj, GLU_TESS_END, (void (CALLBACK*)(void)) glEnd); ^ Here is the full: http://pastebin.com/sqsuC5kb . This is the glew definition: #ifndef CALLBACK #define GLEW_CALLBACK_DEFINED # if defined(__MINGW32__) || defined(__CYGWIN__) # define CALLBACK __attribute__ ((__stdcall__)) # elif (defined(_M_MRX000) || defined(_M_IX86) || defined(_M_ALPHA) || defined(_M_PPC)) && !defined(MIDL_PASS) # define CALLBACK __stdcall # else # define CALLBACK # endif #endif So it seems that on mingw it is defined as "__attribute__ ((__stdcall__))" and not "__stdcall". Any non-hackish ideas how to fix this? For now I just comment out the functions, but we should fix, so people can actually compile on Windows out of the box. If I just add this to the top of the code: #define CALLBACK __stdcall Then of course it works again. But then I think I override that previous declaration. Maybe we can just define our own. On Linux it can be empty and on Windows it should be __stdcall. Also, the functions do work, but they crash when drawing certain shapes.
|
|
« Last Edit: January 18, 2014, 11:46:40 am by TheExDeus »
|
Logged
|
|
|
|
Goombert
|
|
Reply #2 Posted on: January 18, 2014, 02:09:11 pm |
|
|
Location: Cappuccino, CA Joined: Jan 2013
Posts: 2993
|
Yes I am unable to build GL1 now as well, though, you can't blame sorlock, whoever merged it without testing is to blame, but it's fine anyway, just hurry up and make a fix you guys.
|
|
|
Logged
|
I think it was Leonardo da Vinci who once said something along the lines of "If you build the robots, they will make games." or something to that effect.
|
|
|
|
sorlok_reaves
|
|
Reply #4 Posted on: January 18, 2014, 02:26:32 pm |
|
|
Joined: Dec 2013
Posts: 260
|
Lack of regression testing is to blame. We don't test any pull requests, including yours. I've pushed a fix, and tested it on Linux & Windows. Anyway, the blame is mine, I really should have known better than to change platform stuff without testing on Windows as well. But there's some good news! I've finally got ENIGMA + DirectX9 working on this machine!
|
|
|
Logged
|
|
|
|
|
|
TheExDeus
|
|
Reply #7 Posted on: January 18, 2014, 02:40:50 pm |
|
|
Joined: Apr 2008
Posts: 1860
|
I've pushed a fix, and tested it on Linux & Windows. I will pull that for now, but I think Josh will have something to say about that fix. That is what I mean with "hackish" while I wanted "non-hackish".
|
|
|
Logged
|
|
|
|
|
Goombert
|
|
Reply #9 Posted on: January 18, 2014, 04:19:29 pm |
|
|
Location: Cappuccino, CA Joined: Jan 2013
Posts: 2993
|
Confirmed working here, thanks for the quick fix sorlok!
|
|
|
Logged
|
I think it was Leonardo da Vinci who once said something along the lines of "If you build the robots, they will make games." or something to that effect.
|
|
|
|