PrismaticFlower / SWBF-msh-Blender-IO

.msh (SWBF toolchain version) exporter for Blender
Apache License 2.0
19 stars 6 forks source link

Unable to import shadowvolumes from vanilla assets #5

Open Antstafer opened 1 year ago

Antstafer commented 1 year ago

There seems to be an odd issue that plagues both the XSI importer and this one as well. When importing a vanilla mesh, sometimes the shadowvolumes will import only as nulls. I've done some experimenting and found that only the vanilla assets which have their shadowvolumes set to 'hidden' are able to be imported properly. If they are marked 'visible', then they will appear as nulls.

Oddly enough, this is only an issue with vanilla assets, as I've been able to export shadowvolumes as 'visible' and reimport them just fine. Perhaps someone with more knowledge on how these tools work under the hood can explain what's going on here.

PrismaticFlower commented 1 year ago

IIRC there are two ways .msh files store shadow volumes. The first (and the one the addon makes you use) is naming a mesh with the "sv_" prefix. The second is the .msh can have a special chunk (SHDW) that is just for storing shadow volumes. ModelMunge accepts either so I went with using the simpler first one when the exporter was written.

So I'm guessing the assets that import with missing shadows are ones with SHDW chunks instead of "sv_" named meshes. It would be nice to have handling for them in the importer. Not sure when one of us would be able to make that happen though.

WHSnyder commented 1 year ago

@PrismaticFlower's guess is correct, the importer doesn't process geometry stored in SHDW chunks. Also some shadow volumes are also marked by the model type SHADOWVOLUME == 6. Don't have much time to work on this these days but I'll take a closer look and see if I can mop it up today...

maximstewart commented 1 year ago

Provided my other PR gets approved, I can try to take a look at this if @WHSnyder has some suggested starting points I look at.

Initial Research: Would the fix for SHDW be to update msh_scene_read.py > _read_segm given that SHDW resides under it? And, since MTYP is already accounted for it would be a matter of just checking the value to be 6 and then setting some flag to show/hide it, yes?

WHSnyder commented 1 year ago

@maximstewart I have this done but not sufficiently tested on a local branch. I've just been forgetting about over the past couple months... I'll push it to my fork and send you a link when I get off work if you wanna investigate some more...

It works with a bunch of models and is more than likely ready for production but IIRC there was one weird thing about it I needed to ask end users about. I'll try and remember what it was when I take a look tonight.

maximstewart commented 1 year ago

@WHSnyder Sounds like a plan, sure!

maximstewart commented 12 months ago

Now that my other PR got merged I am hungry to do more. (I guess I'm a glutton for punishment. =P ) I don't see any posted link nor when checking your fork any pushes recently @WHSnyder . But, I'm game to take a crack at this or go over your version and fix that one noted bug you had/have.

WHSnyder commented 11 months ago

@maximstewart I pushed my progress to wip/shadowvolumes. It correctly imported a bunch of shadow volumes I tested but there are some extrinsic issues related to naming iirc. It's been a while since I worked in this space so I might remember incorrectly, but SWBF2 shadow volumes must be named with the sv_ prefix when that isn't the case for the shadow volumes relevant to this particular issue. So you'll have to think about whether or not there should be some configurable behavior that automatically renames the imported shadowvolumes so they can work in SWBF2 when if the model was immediately reexported.

On a more general note, the one thing I was trying to do in tandem with this issue was set up some test suite on a set of standard and strange SWBF2 models. How exactly to do this is wasn't clear to me... I've always been testing new features manually but that has often proven insufficient with significant bugs making it into releases. I think that would be another solid PR idea.

maximstewart commented 11 months ago

Yeah, getting a testing suit setup would do wonders for preventing regressions. It'd also make it easier for others to pickup the code in terms of understanding intent and intended functionality. (if done well enough and E2E based)

In a prior comment you mentioned that the code was probably "...more than likely ready for production.." so I can try and play with a testing setup as well as just test your changes just in case what you remember does show up.

PrismaticFlower commented 11 months ago

I will quickly say that .msh files from the mod tools (or derived from them) can't be added to this repository cause of copyright. So they'll either need to be made specifically for the tests from scratch or the files for import tests will need to be acquired separately from anything in this repository.

Of course that's just what I'm comfortable with happening here. And I'm more than happy if people would rather fork this, add a test suite with lots of stock models and carry on development elsewhere. I'll gladly point people towards that work.

maximstewart commented 11 months ago

@PrismaticFlower Yeah, that's fare. I was imagining more like a statement of "Go download the mod tools and then point to the respective folder(s)." in some settings file provided in the unit test folder.

maximstewart commented 11 months ago

@WHSnyder Okay, I think I understand better what you mean about the naming differences. Its also sightly different in that the 'sv_' has a white material using a white.tga image and the SHDW chunks don't have anything like that. A goofy question came to mind which is whether or not it's possible to stitch the chunks and treat it as one mesh like "sv_" looks like it's as? In effect treating it as "sv_" altogether so we'd just use the same pipeline.

Edit: I also did a grep for who's using SHDW vs sv_ and attached the image below the 1st one.

Image 1: 2023-11-23-002904_3840x1080_scrot

Image 2: scrshot_2023-11-23 00:46:06

PrismaticFlower commented 11 months ago

@PrismaticFlower Yeah, that's fare. I was imagining more like a statement of "Go download the mod tools and then point to the respective folder(s)." in some settings file provided in the unit test folder.

That setup would be fine with me. That would also enable the ability to have a huge broad test (in addition to other ones) that made sure every stock model imported without at least generating errors even if you didn't verify all the data.

LeovanGit commented 10 months ago

Can you explain me, please, what is edges[3] (reference to twin edge?) in edges: List[Tuple[int,int,int,int]] = field(default_factory=list) and how it is used in ModelMunge?

https://schlechtwetterfront.github.io/ze_filetypes/msh.html#SHDW said that edges[3] in most cases equal to edges[2], but this is incorrect

Also interesting for what we need edges[4] = USHORT_MAX, just aligning?

WHSnyder commented 10 months ago

@LeovanGit I recall finding some instances of the third and fourth entries not being equal, but IIRC I couldn't figure out how to incorporate them for any benefit. The meshes imported from shadowvolumes turn out just fine without using the 3d and 4th entries,

WHSnyder commented 10 months ago

@PrismaticFlower Yeah, that's fare. I was imagining more like a statement of "Go download the mod tools and then point to the respective folder(s)." in some settings file provided in the unit test folder.

That setup would be fine with me. That would also enable the ability to have a huge broad test (in addition to other ones) that made sure every stock model imported without at least generating errors even if you didn't verify all the data.

I think this'll work to a great extent. It would be really useful to divide the test up into static and skinning models, and most importantly, BF1-unique vs BF2 models. BF1 models have been known to cause more problems, not just in terms of format-versions we know are unique to that game.

However it's also (more) important to test models that were exported from ZETools. Throughout the development of this plugin the models that cause problems are more often than not exported from XSI via ZETools. Every one of these problematic models I encountered would munge and render ingame just fine. They frequently break logical assumptions implied by the most detailed msh docs (e.g. MODLs having unique MNDXs, bones MODLs being typed properly). Other times they adhered properly to the formatting specs but ordered chunks in unexpected ways or were missing some small (but important to the plugin) details. I can scroll through my discord messages a get a list together of ones that caused issues...

LeovanGit commented 10 months ago

Hello, please, take a look (maybe I made a mistake somewhere): https://github.com/PrismaticFlower/SWBF-msh-Blender-IO/pull/16

I tried to add the ability to export shadowvolumes back to the SHDW chunk without the "renaming to sv_" hack.

Also, edges[3] is important value for ModelMunge (it does not create shadowvolumes without it). This is reference/index to twin edge (edge of the adjacent face that shares vertices with this edge).

In your code, when you build shadow mesh, you just take first vertex index + 1 (or smth like this) to move to the next polygon, but I guess ModelMunge uses this twin edge index for this. Also it shows an error when twin_edge_index == next_edge_index and doesn’t build shadowvolume.

@WHSnyder