search

zturtleman / mm3d

Maverick Model 3D is a 3D model editor and animator for games.
https://clover.moe/mm3d
GNU General Public License v2.0
116 stars 24 forks source link

Remove DrawingContext, drawcontext.h #72

Closed m-7761 closed 5 years ago

m-7761 commented 5 years ago

https://doc.qt.io/qt-5/qglwidget.html#QGLWidget describes sharing OpenGL contexts. If the viewports are created with their context shared with the first viewport (this is trivial) then DrawContext should not be required, and I can't think of a strong argument for why any code would want to use it, hypothetically, and it's obviously bad to use it, so I think it would be best to remove.

The penalty for using it is every texture and vbuffer is duplicated into every viewport. Since MM3D supports a dizzying number of viewports, that's just kind of embarrassingly nonsensical. As a matter of principle it just seems decadent to me. I'm just in favor of not having junk code clouding everything. The Model class itself is so gigantic it's hard to work with. It would probably be worthwhile to think up ways to slim it down.

zturtleman commented 5 years ago

I'm mildly concerned this might break running on some OpenGL drivers. I recently decided to have Maverick 1.3.x/1.4 target the same OpenGL requirements as Misfit Model 3D 1.3.8.

Sharing textures and vertex buffers is planned to be added with OpenGL ES 2 compatibility in Maverick 1.5.x. Vertex array objects are required for OpenGL ES 2 compatibility and are not shared between contexts. I'm not sure how to handle drawing with separate vertex array objects per-context yet and not sure if DrawingContext might be useful.

I've recently completed porting from QGLWidget to QOpenGLWidget (Qt 5.4) which automatically enables context sharing for widgets in the same window and uses framebuffers instead of native OpenGL windows for each viewport (#14). Merging it is currently on hold (for say, 2 to 3 years?) until Maverick 1.5.

m-7761 commented 5 years ago

The vbuffers would have to be duplicated just like textures. Note that there is two kinds of sharing... or one is simply using the same context, and the other is sharing lists, that is the handles for textures and vbuffers, etc.

I'm actually not sure what is done by that Qt sample code I linked to, but I imagine both ways are possible. Either way works, since textures and vbuffers are lists. Both GLX and WGL can share lists, but I don't know if possibly its a latter-day extension. Straight up sharing the context is the other way to work it. I'd be surprised if Qt does not facilitate that.

I'm going to be developing a management system that works with vbuffers to render to VR, transparently implementing instancing techniques and device profiles, and some image enhancement techniques. It's going to be built into my UI framework.

m-7761 commented 5 years ago

THESE ARE BUGS, not "enhancements" but I felt better to not make a separate issue. (I was about to.)

https://github.com/zturtleman/mm3d/blob/167792432eca5bcc52987f08132e88a0d1ef11d7/src/libmm3d/model_texture.cc#L106

This condition is preventing NULL context from initializing its generated texture IDs.

There are also problems in Model::draw (model_draw.cc) that fails to automatically call loadTextures for the NULL context, so it's necessary to call that manually. That disparity should be fixed.

(It does this with a member that is like "is_valid" or something. So the NULL context doesn't have a context object, so Model probably needs to manage loadTextures exclusively and reserve a bool member for the NULL context... personally I would rip out the context system, which reminds me... there is an existing Issue where I should post this...)

Currently cmdline.cc calls loadTextures, but I think that's bogus, since it doesn't have an OpenGL context that it knows of. It's not appropriate to call there. It's likely too early to have initialized OpenGL.

m-7761 commented 5 years ago

EDITED: The cmdline.cc loadTextures is a separate pathway taken by cmdline_runcommand. There is also cmdline_runui, however these appear to be (sloppily) two sides of the same coin, since if one is so, the other is not so.

I suppose that using cmdline_runcommand in undocumented -b mode (batch) and undocumented --testtexcompare and --script and --convert modes, an OpenGL context is created for them, but I haven't tried them to see if that's in fact the case. They may not require one, in which case glGenTextures should not be used.

zturtleman commented 5 years ago

After re-evaluation of what I want of this project this is no longer on the roadmap.