#2455 closed enhancement (fixed)
Texture System Update
Reported by: | IronNerd | Owned by: | IronNerd |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 16 |
Component: | Core engine | Keywords: | |
Cc: | Patch: |
Description
This is a precursor to changes in OglTex that will be required for the OpenGL 2 update.
The change is mostly refactoring to make the interface more consistent with the rest of the project, but it has some incidental improvements. Free is automatically called in the destructor, making it harder to accidentally leak memory and requiring a mild update to the get_average_colour function. Some sections of code have been made shorter or more readable by the update.
This will probably be due for another (smaller) revisit mid-way through the renderer update, sometime after cubemaps have native support and external references to member variables have been mostly reduced to the codecs.
Attachments (2)
Change History (12)
by , 10 years ago
comment:1 by , 10 years ago
by , 10 years ago
Attachment: | Tex_v2.patch added |
---|
comment:2 by , 10 years ago
I booted linux in a VM and managed to reproduce the problem. I've uploaded Tex_v2.patch, which should fix both the warning and the crash. Thanks for the detailed outputs!
comment:3 by , 10 years ago
The second patch seems to compile and run fine (except there still are a few possibly related memory warnings from valgrind). This should be committable after someone more familiar with graphics programming approves the patch logic.
Thanks for the effort you're putting into this!
comment:4 by , 10 years ago
Compiled nicely on my system (using OSX 10.8 sdk). Works in-game. This fits our convention and seems a good change anyway...
Why not go al the way and make "Tex" a class now that it behaves really quite like one?
comment:5 by , 10 years ago
This is just the first stage of the change. I intend to officially change it to a class when its metamorphosis is complete and it's actually used like one. By used as a class, I mean that basically everything is done through meaningful functions and not just get/set the member variables to get the job done.
Step 1 (this): Move all of the major functions inside the struct and make it RAII by default
Step 2: Change the Tex_Codec code to actually use this like a class (coded, but not uploaded for review, as it would make the review too big... about this size of a change again)
Step 3: Overhaul the OglTex implementation, which also currently doesn't really use this like a class (This is actually about 2-3 steps if we want to keep review sizes reasonable)
Step 4: Implement native OpenGL cubemap support and change the rendering code to use that instead of its current hack (which uses tex quite like a struct still)
Step 5: Translate this into a real full-fledged class with private members and a meaningful interface.
As you can see, there is still quite a bit of work to do before we can really elevate Tex to its desired status, and the whole work-chain required is many times larger than this small patch. As soon as this one is in, we can move to reviewing step 2 as I implement step 3.
comment:6 by , 10 years ago
Hmm, so some bad news. I noticed that the minimap colors are distorted when your patch is applied.
Normal: https://googledrive.com/host/0BwIjnJSd7iThRXRiN0ExcVVaeDQ/normal-minimap.png With Patch: https://googledrive.com/host/0BwIjnJSd7iThRXRiN0ExcVVaeDQ/with-patch-minimap.png
It seems the terrain textures aren't loaded properly on the minimap.
Edit: Probably something is getting corrupted as with a reload of that map, instead of black it is neon green.
comment:7 by , 10 years ago
Cannot reproduce, I get this blackness only until the textures are loaded. Please flush your cache and try again.
comment:9 by , 10 years ago
Milestone: | Backlog → Alpha 16 |
---|
comment:10 by , 10 years ago
IronNerd: Do you think you'll have time to complete any more work on 0AD? Your contributions were very appreciated and we'd love to see you back, even if it's just small patches.
The actual patch style looks good, but I had some issues running/compiling with your patch on linux.
First, a build warning:
The second issue was a segfault on start. Here is the backtrace from gdb (Gnu DeBugger):
Here is the output of valgrind(linux memory debugger) from start to exit:
Not all those warnings/errors are related to your patch.