Ploaj / SSBHLib

A library for dealing with SSBH rendering format
30 stars 18 forks source link

Nested project #14

Closed CarlosFdez closed 5 years ago

CarlosFdez commented 5 years ago

Currently when you open a model folder, it lists only that folder and its immediate descendants in the filetree. Personally, it'd be much more user friendly if importing a root folder recursively imported sub-directories.

This is something that I was tempted on working on but I wanted to get it in writing first.

My current idea is to introduce a new node type for "projects", which are parent nodes that are renderable that proxy to the first available renderable model. That way, clicking on a folder would allow you to "preview" the model. Other than that it'd be the usual recursive tree.

Current concerns

CarlosFdez commented 5 years ago

Started on it, and even with a small collection there is a load of load time. Some mechanism of lazy loading will be required for such an enhancement.

Currently juggling a few potential ideas. Leaning on 2:

Both together would be absolute best performance but premature optimization is the king of evil.

The first one is easier to do and prevents the atrocities of loading the C:// drive, but combining the logic of a folder node and node loading into a self recursive object violates separation of concerns unless a "FileNodeLoader" object was passed through for sub-node loading. Plus directory peeking needs to be done to support "clicking on a model folder shows the model in there".

The second one requires changes to a bunch of classes which means more testing is required. Also means that there might be a hiccup the first time to load any node resource. But it seems the most architecturally sound to me.

ScanMountGoat commented 5 years ago

Have you done CPU profiling in Visual Studio to determine what the bottleneck is?

CarlosFdez commented 5 years ago

Good call. Result on running it on the "fighters" folder:

image

I figured it'd be the actual file loading that takes up all the time in reasonable conditions, which the results do confirm. OpenDirectory calls OpenFile in a loop, so since OpenFile is taking 95%, it is indeed file parsing and mesh/texture/etc building that's eating up all the time. The ReadBytes call is a paltry 3% by comparison.

I tried to test raw directory loading (C:// has no smash models so I tested there), but unauthorized exceptions crash the application. I should probably have unauthorized folders either silently fail or raise a messagebox....

ScanMountGoat commented 5 years ago

Keep in mind that some file nodes depend on other nodes already being loaded to work properly. Waiting to load a folder until it's expanded makes the most sense. All the necessary files for rendering will be in the same folder.

CarlosFdez commented 5 years ago

Hmm, that's true. I thought all dependencies would have GetRenderableNode() as an available hook but it seems there are a few exceptions (NUMDL gets the mesh and materials via public side-effect variables). While that sort of dependency resolution should probably be tweaked regardless (at least into properties instead), it would take a non-trivial amount of time to hash out a proper design.

Delayed loading makes the most sense then. Only real downside is if we want "click on folder to show model without opening it" as a feature, 2 file passes are required, one to check the names of files before expansion, and one after to build the nodes. Or for ALL DirectoryNodes to be IRenderable and return null if there's no valid model (attempting to render = populate sub-nodes). The more I think about the IRenderable idea the more I like it actually.

PS: Since GetRenderableNode() returns an IRenderable, it should probably be renamed at some point or other.

Ploaj commented 5 years ago

New project allows you to display a folder (and subfolders) when opening them. https://github.com/Ploaj/StudioSB

However, to view the model you still need to click the numdlb. It will then attempt to load the files it needs (in the order it needs them) to the viewport when being clicked instead of loading the files when populating the file tree.

CrossMod didn't need this feature since its only purpose was to render select things.

CarlosFdez commented 5 years ago

Didn't notice there was a new project. Is the goal of the new project to supercede this one outside of the actual parsing logic (SSBHLib specifically?). Makes sense as CrossMod seems to be mostly a test application for SSBHLib.

(Which reminds me, something I found awkward in SSBHLib itself is the reliance on hard-coded strings to access properties, which hurts discoverability. Proxy Methods or some sort of constants or enums collection would help with something like that. Separate discussion though.)

Ploaj commented 5 years ago

Could you clarify what you mean about hardcoded strings to access properties?

In general reading/creating the "Formats" directly isn't a good idea, instead the idea was to use the "Tools" for handing them. Such as using the MeshMaker to generate a MESH file.

CrossMod isn't a good example because it stored the MESH, MATL, etc in the nodes. That was for research purposes. I'll get around to making proper examples at some point.

CarlosFdez commented 5 years ago

Ah, I meant specifically the vertex accessing. For example:

var positions = vertexAccessor.ReadAttribute("Position0", 0, meshObject.VertexCount, meshObject);
var normals = vertexAccessor.ReadAttribute("Normal0", 0, meshObject.VertexCount, meshObject);
var tangents = vertexAccessor.ReadAttribute("Tangent0", 0, meshObject.VertexCount, meshObject);

When I was experimenting with it, figuring out what data existed was a bit difficult without digging deep into example usage code.

I'll need to look into the new project one of these days for sure though! Sounds interesting.

ScanMountGoat commented 5 years ago

You can find attribute information here. The attributes have string names in the numshb file. https://github.com/ScanMountGoat/Smush-Material-Research/blob/master/Vertex%20Attributes.md

Ploaj commented 5 years ago

There are enums in the mesh maker for them, however. I agree that's not very easy to find.

But like SMG said, those names are inside the mesh file. Instead of manually type "Position0" you would normally iterate through the mesh attributes in the MESHObjects and get the strings that way. We used to do that, but it had errors sometimes when rendering.

edit: And because those names are for shader attributes, they can potentially vary between game. That's why when reading them it uses a string instead of enum.

CarlosFdez commented 5 years ago

Ah, yeah just found the enum, though it seems the vertex reader doesn't accept them. I think if there was a comment stating where to find values that are valid for attributes, that'd probably have been enough.

My workaround when I was doing my own experimentation with it was to create extension functions for the vertex reader which seemed to work out well enough.

Good to know the rationale though. Thank you. Starred the material research.