#1497 closed enhancement (fixed)
[PATCH] Support for multiple UV sets per model
Reported by: | myconid | Owned by: | myconid |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 11 |
Component: | Core engine | Keywords: | patch texturing |
Cc: | Patch: |
Description
Collada files can support more than one UV coordinate set, however 0ad's .PMD version 3 files are unable to store them so they are currently ignored.
0ad's models use overlapping UVs to make optimal use of UV space on their base textures. While this is beneficial, it makes it impossible to implement more advanced texturing techniques, such as lightmaps (e.g., AO maps), decal maps and so on.
The attached patch upgrades the engine to .PMD version 4, which introduces support for multiple UV sets.
Attachments (4)
Change History (28)
by , 12 years ago
Attachment: | multicoord.diff added |
---|
comment:1 by , 12 years ago
Keywords: | review added |
---|---|
Milestone: | Backlog → Alpha 11 |
Summary: | Support for multiple UV sets per model → [PATCH] Support for multiple UV sets per model |
comment:2 by , 12 years ago
follow-ups: 4 6 comment:3 by , 12 years ago
It's been tested with Blender 2.63 and it works great. See the usual forum thread for a pic and some test files. Blender's exporter must have come a long way lately, because the file with UVs is actually smaller than the original (1.3M -> 1.1M)!
This is still needed if you're generating the AO textures in the engine. With AO (and lightmaps in general) you must have UVs that don't overlap, since each surface needs to be lit independently of the others.
Models need to be exported like this from the beginning, if you want to apply lightmaps to them (if you don't, then it doesn't matter if the second UVs are missing). Lightmaps are a much bigger topic than just AO, and all lightmaps share the same UV set. We want lightmaps to be both loadable from textures (created by artists) and possible to compute in the engine. As a result, the only way that UVs can be consistent in all these cases is if they are stored in the model.
In Blender, creating a second UV set automatically and rendering AO is as simple as a few mouse clicks. There seems to be a way to write a Blender script to do these things for us...
follow-up: 5 comment:4 by , 12 years ago
Replying to myconid:
Models need to be exported like this from the beginning, if you want to apply lightmaps to them
OK, I just wanted to confirm that this does require re-exporting and some extra work on the part of the artists :) I'll see if we have most or all of the original .blend
/.max
files, otherwise we'd need to import the DAEs first...
comment:5 by , 12 years ago
Replying to historic_bruno:
OK, I just wanted to confirm that this does require re-exporting and some extra work on the part of the artists :) I'll see if we have most or all of the original
.blend
/.max
files, otherwise we'd need to import the DAEs first...
We can import the DAEs, though Blender seems to have some issues with missing texture files that require us to go in the xml and remove the references manually. I think we should use Autodesk's free utility to convert DAE -> FBX -> OBJ and import the OBJ into Blender, as that has no issues.
comment:6 by , 12 years ago
Replying to myconid:
In Blender, creating a second UV set automatically and rendering AO is as simple as a few mouse clicks. There seems to be a way to write a Blender script to do these things for us...
If Blender can do this automatically why can't we? Is it very complicated, that we couldn't add it to our model importing? (if it turns out to be simple, I don't see why we couldn't do this ourselves and save having to import and re-export a bunch of models from Blender)
Maybe there's an advantage to doing it in Blender, in that it preserves the multiple UV sets in the DAE.
comment:7 by , 12 years ago
I doubt it's too complicated, though the same UVs that you'd generate in the importer also need to be available to the artists outside the game when they're creating their lightmaps. Either you write a DAE exporter to save out the UVs you generate, or you let the artists generate the UVs and we import them into the game. The patch provides the latter, IMHO better option.
comment:8 by , 12 years ago
OK we can go with that for now. If it proves to be too much work for artists, maybe we can create a second UV set for models imported with only one (or older PMD version than whatever we are currently) which would allow a proposed engine AO calculation for all models even if they haven't been updated to multiple UV sets.
Anyway that would seem to build on this patch, rather than replace it.
follow-up: 10 comment:9 by , 12 years ago
There's a build error btw, I checked and it's still in your git repo. source/collada/PMDConvert.cpp
fails in a few places because vertexCount
is referenced in an assert()
before it's defined. Easy fix :)
comment:10 by , 12 years ago
Replying to historic_bruno:
There's a build error btw, I checked and it's still in your git repo.
source/collada/PMDConvert.cpp
fails in a few places becausevertexCount
is referenced in anassert()
before it's defined. Easy fix :)
I should point out you won't notice this error unless you build in debug config, as the assert() does nothing in release builds, so it's easy to overlook.
by , 12 years ago
Attachment: | multicoord-final.diff added |
---|
follow-up: 12 comment:11 by , 12 years ago
Submitting multicoord-final.diff for review. The assert error has been corrected (I believe).
comment:12 by , 12 years ago
Replying to myconid:
Submitting multicoord-final.diff for review. The assert error has been corrected (I believe).
I forgot to ask last time, what's the motivation behind commenting out the stream flags stuff? (in ShaderProgram.cpp
, HWLightingModelRenderer.cpp
, etc.) If it's something totally incompatible with the new changes, we should remove it, otherwise we should make it work.
Are these patches ready to be committed after review?
by , 12 years ago
Attachment: | multicoord-final2.diff added |
---|
comment:13 by , 12 years ago
Fixed stream thingy. It was an omission, no good reason for it.
Ready for review.
comment:14 by , 12 years ago
In GeomReindex.cpp
You modify the struct to have std::vector<float> uvs. It would be nicer to create a uv struct {float u, float v} and have a vector of them.
In bool operator==(const VertexData& a, const VertexData& b) you compare uvs using ==, if I understand this correctly this isn't going to use the similar() function so the tolerance for the comparison will be lost.
In PMDConvert.cpp it would be nice to have a slightly expanded comment for line 507.
I haven't finished looking through the whole patch yet, I will need to sleep before I can do any more. It applies cleanly compiles and runs fine for me which is good.
comment:15 by , 12 years ago
Thanks. Since these are really minor changes I'll update the patch when you're through the whole thing, if that's alright.
comment:16 by , 12 years ago
I've had a look through the rest. It looks like a nice patch.
InstancingModelRenderer.cpp 56: VertexArray::Attribute m_UVs[5]; I wasn't entirely sure what the [5] meant in this context. My best guess was that it makes an array of 5 possible m_UVs to be used. If so then there needs to be some form of error checking in case the number of UV's >5.
comment:17 by , 12 years ago
Whoops! :/ That line was replaced in a later commit on git, which I forgot to include in the patch (how typical of me).
Alright, I'll get you the updated patch once I get home.
by , 12 years ago
Attachment: | multicoord-final3.diff added |
---|
comment:20 by , 12 years ago
Looks like you committed the older version to svn. Was that intentional?
comment:21 by , 12 years ago
Uh oh! Can you provide a patch with the changes missed? I'll find someone to commit it ASAP.
comment:23 by , 12 years ago
Keywords: | review removed |
---|
Replying to myconid:
Hopefully Blender correctly exports models with multiple UV coordinate sets? Preferably a recent version of Blender (2.6+) and in a way which works with our importer, since Blender's COLLADA support is sketchy :( I'm curious if you tested that? And that it doesn't break existing model import/rendering. Max exporter would be nice to test if we can find someone actively using it.
Also how does this tie in with pre-generating AO textures in the engine, would models need to be exported with multiple UV sets from the beginning or can they be created on demand?