Re: [PATCH v2 0/4] MR174: Consolidate the TPF and d3dbc handling code.

[Giovanni Mascellani (＠giomasce)]

> I don't want to be insensitive to the concern, particularly since both Giovanni and you raised it, but I just don't get it? Navigating within a file seems at least as easy as navigating between files, and files on the order of \~2k lines in case of d3dbc.c and \~5k lines in case of tpf.c don't seem particularly unusual in either vkd3d or Wine. (I had been thinking we might as well merge hlsl.c, hlsl_codegen.c and hlsl_constant_ops.c, but I guess not then...)

I suppose it depends a lot on one's own thought processes and habits, so I don't claim that what's better for me is intrinsecally better for everybody. Also, as you say, Wine's standards are much worse than what we're discussing here, so, while I don't think they are good standards, I had to come to terms with them.

I agree with Zeb that having smaller files helps segregation of code, and if code is segregated properly with reasonable interfaces between the segregated parts it is easier (for me at least) to understand and change, because I can study each of the segregated parts somewhat independently. In theory it is not the number of lines or functions that counts, but in practice, with some exceptions, once a file has grown past a few thousands lines of code it's hard for me not to think that it would make good use of some splitting.

For example, I would find ideal to have `hlsl_codegen.c` splitted in a few pieces. The prepend/append copies and copy propagation sections could be easily moved to an independent file each one, keeping basically everything `static` except a handful of functions that are called by `hlsl_emit_bytecode()`. You have a small interface, but everything else you have to do in copy propagation doesn't need to interact with all the rest of `hlsl_codegen.c`, and splitting the file ensures that. If somebody wants to learn how copy propagation works, they can read `hlsl_copy_propagation.c` instead of having to potentially have to go through the whole of `hlsl_codegen.c`. This luckily already happened for `hlsl_constant_ops.c`. I wouldn't move each other pass in its own file because that would be too small, though. There is some sweet spot, which of course can change from person to person.

As for `tpf.c`, the parser and writer parts are basically completely independent. They don't even share the internal representation. When I am working on the parser it's unlikely that I care about the writer at all. So having them in the same place is just confusing for me. Since they need to share some code (mostly the definitions) it makes sense to move that in a shader header, as it was before. If, say, I want to refactor how an hypothetical field `type` is used, then I'm likely to search through the file for that identifier. And if unrelated code sits in the same file, I'll have a lot more false positives.

As I said above, that's probably a very personal thing, and having bigger files is not a blocker for me. Just wanted to share my point of view.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/174#note_30845