GothicKit / ZenKit

A re-implementation of file formats used by the early 2000's ZenGin
http://zk.gothickit.dev/
MIT License
49 stars 9 forks source link

Build order polution with libsquish #41

Closed Try closed 1 year ago

Try commented 1 year ago

After building OpenGothic, sub-module OpenGothic\lib\phoenix\vendor\libsquish becomes modifyed:

HEAD detached at b28220d
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)
        modified:   vendor/libsquish (modified content)
...
HEAD detached at ae7054a
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   squish_export.h

I'm not sure, if this header usefull at all. Also part of that: libsquish is only to implement texture::as_rgba8, right? Can it be removed eventually?

lmichaelis commented 1 year ago

I've noticed this as well. I'll see if there is a way to disable generation of that header or move it into CMake's build directory. That would probably the best solution since I do want to keep as_rgba8 around since it makes working with textures a lot easier.

It could be moved into ext at some point though which might be a good compromise if I then allow disabling it. That would require you to provide an implementation of the DXT formats though.

Try commented 1 year ago

That would require you to provide an implementation of the DXT formats though.

Right now, in Resources::implLoadTexture DXT1,2,3,4,5 are handled by Tempest::Pixmap explicitly, and rest uses as_rgba8. (nitpick: actually not supported DXT2/4, but no one uses then anyway)

Rest formats:

Probably need to clarify here. In engine there is only one component order R-G-B-A for data-storage. This is strategic kind of choose, to avoid potential problems with compute-shaders and blitting. For sampler-capable images, there is a dx12-style component mappings in sampler, what is not very helpful here.

Current idea is to move conversion workload to OpenGothic resources.cpp, since those formats are quite a legacy. Another thing that I'm worry a little after looking at the code is redundant data copies. [input] buffer -> texture -> texture_to_dds/as_rgba8 -> Tempest::Pixmap -> host-visible -> gpu. Probably need to do a profiler run and check it out eventually.

lmichaelis commented 1 year ago

I'll think about it. libsquish is a bit heavy and I'd like to drop dependencies anyways (I am currently working on removing lexy which is used for MDS parsing).

But I am at least able to fix the issue. It's kinda hacky but my pipelines work so it seems fine. I applied some changes to lmichaelis/phoenix-libsquish which is used as the source: 15d3f56 and c8e060c.

trial/squish-update has the changes. Will do some extra checks before merging but so far it seems fine.

lmichaelis commented 1 year ago

Merged.

Try commented 1 year ago

Fixed, thanks!