search

Kode / Kha

Ultra-portable, high performance, open source multimedia framework.
http://kha.tech
zlib License
1.48k stars 174 forks source link

Support new vertex data types in HL backend #1384

Closed MoritzBrueckner closed 2 years ago

MoritzBrueckner commented 2 years ago

This fixes some compilation errors due to incomplete switch statements.

To avoid giant switch-case constructs which are prone to difficult to find typing errors, I instead allow the VertexData enum to be implicitly converted to Int and changed the indices there to account for the no-data type 0 in the Kore/Kinc code. The indices weren't accessible before so this should be fine.

If you don't want to have this implicit conversion we could use a private @:to conversion function instead, but I think having duplicate code all over the place like before would be much worse.

Unfortunately the compilation to HL still fails due to the new array types. Maybe it is possible to add a HL build to the CI so that errors like this are noticed earlier?

RobDangerous commented 2 years ago

Adding it to CI would be cool if we could also find someone to then take care of it. Would you be up to that?

RobDangerous commented 2 years ago

That VertexData thing was not fine by the way, I reverted it. It should be a giant switch-block so a change in the Kinc enum doesn't auto-break Kha. Feel free to copy over kha_convert_vertex_data.

MoritzBrueckner commented 2 years ago

Oh, that's not good. I will open a PR for another solution as soon as possible, I just don't have much time until sunday or monday.

Wouldn't it make more sense (be cleaner) to also update kha_convert_vertex_data() in g4.h and possibly other places then (or convert the format to int and add 1 in the HL code)? Storing every format one index lower in Kha than in Kinc seems a bit counterintuitive (and also prone to errors in the future).

Regarding the HL "caretaking": I'm not sure what exactly you mean. If it's about fixing a bug now and then (or at least trying it or finding the cause even if not providing a good fix), yeah, I'm fine with that and I'm more or less already doing that if I stumble upon some problem. But I'm afraid I'm not able to fully take care of it and all the possible issues – for that I have way too little knowledge about so many things (G5, good and fast C++, etc...) and also not enough time.

As for the new array types that currently break compilation in HL, is it just a matter of implementing the new additional types or should there be a fundamental change somewhere?

RobDangerous commented 2 years ago

Kha (or anything) should not rely on concrete enum values in Kinc at all. I know it still does so in some places due to my laziness but it really shouldn't.

For the caretaking I just mean that if we add it to the CI and things go red, would anyone take care of it in a timely manner? Because I wouldn't which is why I don't want the CI to go red on HL problems. I only look at HL very occasionally because I'm not interested in it myself and aside from Armory nobody uses it as far as I know.

The special thing about the array types is that one can be converted to each other one without any actual conversion-work happening, it's all just different views of a byte-array.