blt4l icon indicating copy to clipboard operation
blt4l copied to clipboard

PNG Loader missing from basegame

Open ZNixian opened this issue 8 years ago • 34 comments

When using custom textures, calling DB:reload_textures does not seem to do anything - this is very likely a caching issue.

ZNixian avatar Jan 10 '18 22:01 ZNixian

The next logical step is to actually look at the code path for Linux and Windows and see how they differ.

RomanHargrave avatar Jan 11 '18 00:01 RomanHargrave

Ah, I just realized something - VoidUI (which works) uses .texture (dds) images, while the mod icons use PNGs. I'll see if using DDS makes the mod icons show up.

ZNixian avatar Jan 11 '18 02:01 ZNixian

Ah, that does seem to be the issue. The windows binary contains LibPNG, while the GNU+Linux binary does not.

ZNixian avatar Jan 11 '18 02:01 ZNixian

As a note, TGA images do load successfully - they are the only format (besides DDS) supported in the basegame.

It appears one way to implement this is to create a subclass of DSL::ImageParser

Edit: VTable for easy reference:

sym vtable for dsl::ImageParser = 0x137a170 (size = 160)
    ... entries:    20
0       (int (*)(...)) 0
8      (int (*)(...)) (& typeinfo for dsl::ImageParser)
16      (int (*)(...)) (& dsl::ImageParser::~ImageParser())
24      (int (*)(...)) (& dsl::ImageParser::~ImageParser())
32      (int (*)(...)) (& dsl::ImageParser::is_type(unsigned int) const)
40      (int (*)(...)) (& dsl::ImageParser::type_id() const)
48      (int (*)(...)) (& dsl::ImageProducer::open())
56      (int (*)(...)) (& dsl::ImageProducer::close())
64      (int (*)(...)) __cxa_pure_virtual
72      (int (*)(...)) __cxa_pure_virtual
80      (int (*)(...)) __cxa_pure_virtual
88      (int (*)(...)) (& dsl::ImageProducer::slices() const)
96      (int (*)(...)) (& dsl::ImageProducer::set_slice(int))
104      (int (*)(...)) (& dsl::ImageProducer::srgb() const)
112      (int (*)(...)) __cxa_pure_virtual
(int (*)(...)) -0x00000000000008
128      (int (*)(...)) (& typeinfo for dsl::ImageParser)
136      (int (*)(...)) (& non-virtual thunk to dsl::ImageParser::~ImageParser())
144      (int (*)(...)) (& non-virtual thunk to dsl::ImageParser::~ImageParser())

ZNixian avatar Jan 11 '18 02:01 ZNixian

I'm surprised the game doesn't have a PNG decoder somewhere. It would be idea if we didn't have to bring one with us, should we implement this.

The other problem that this brings up is the inevitable libcxx nightmare.

RomanHargrave avatar May 20 '18 05:05 RomanHargrave

I'm surprised the game doesn't have a PNG decoder somewhere. It would be idea if we didn't have to bring one with us,

Since it's only loading DDL encoded images from the asset bundles, there isn't really anything they need the PNG loader for.

On Windows they use it for development reasons (and ship it for some reason), but not during normal use of the game.

should we implement this.

Given that almost every mod with custom images uses PNG, it's probably a good idea.

The other problem that this brings up is the inevitable libcxx nightmare.

IIRC DB:create_entry requires libcxx anyway, so that's not any more of an issue than it is currently. Having said that, skimming though assets.cc I can only see std::string being used, so we might be able to make a tiny reimplementation of libcxx's string class and use that on GCC.

ZNixian avatar May 20 '18 10:05 ZNixian

I had considered reimplementing std::string but run in to something that turned me off of that - I'm not quite sure what it was (though if I had to guess, it was the need to reimplement many containers as well). That being said, if the license is compatible, we also have the option of 'transplanting' it.

RomanHargrave avatar May 20 '18 12:05 RomanHargrave

@ZNixian i've decided to start a project branch to investigate and implement a 'port' of libcxx (mainly re-namespacing), specifically the parts we want. If it becomes unweildy but useful, we can extract it to its own repository.

Luckily, libcxx is quite small (deceiving, though, we are talking about C++ after all) compared to a monster like boost, so I would hope we can start to see success soon.

RomanHargrave avatar May 21 '18 03:05 RomanHargrave

IIRC all we really need is std::string and std::vector, if that helps.

It'd certainly be very nice to get away from requiring libcxx.

PS Have you had a look at SuperBLT, and in particular it's GNU+Linux support? If so, do you have any thoughts about it?

ZNixian avatar May 21 '18 03:05 ZNixian

When did it get gnu/Linux support?

RomanHargrave avatar May 21 '18 05:05 RomanHargrave

Misread that. I'll have to look at it a bit more tomorrow.

RomanHargrave avatar May 21 '18 05:05 RomanHargrave

We can probably merge that @znixian if it is working. Is there s bullet list of roadblocks?

RomanHargrave avatar May 21 '18 05:05 RomanHargrave

Some things off the top of my head:

  • Development of SuperBLT proceeds much faster than that of BLT4L, given that no new featues are being added to regular BLT. This would require the addition of a stable branch and the more common use of tags, though it's an easy to resolve issue.
  • Not a show-stopper by any means, but SuperBLT has a dependency on OpenAL for it's audio API.
  • I should write some kind of unit test system to ensure it always works on GNU+Linux, as much of the development occurs on Windows. Having said that, manually testing it wouldn't be hard either.
  • SuperBLT is a port of the original BLT4W source code, so some minor things will likely be different. Again, these changes can be moved across.
  • While some BLT code was copied into BLT4L and quite a bit of BLT4L code was again copied into SuperBLT, they're quite separate projects and we couldn't really merge them. The only option I can see would be to have SuperBLT replace BLT4L.
  • SuperBLT is on GitLab, potentially hindering collaboration with some of the current GitHub-based users.

ZNixian avatar May 21 '18 06:05 ZNixian

SuperBLT is a port of the original BLT4W source code, so some minor things will likely be different. Again, these changes can be moved across.

Since SuperBLT is as far as I can tell, tooled and designed for WIndows, addition of SBLT features to BLT4L would resemble the porting of BLT4W, where some portions of code may be copied directly, but ultimately a lot of novel code would be written.

Just due to the way the platforms and game versions differ, I don't know that having a cross-platform code base is even reasonable. The compiler, standard library, and game engine versions are totally incompatible. I think that the only shared code between the two would ultimately just be some glue logic, with the platform specific code still being completely separate.

RomanHargrave avatar May 21 '18 17:05 RomanHargrave

Since SuperBLT is as far as I can tell, tooled and designed for WIndows, addition of SBLT features to BLT4L would resemble the porting of BLT4W, where some portions of code may be copied directly, but ultimately a lot of novel code would be written.

Everything platform-specific is in platforms/w32 or platforms/gnu. Everything in src is platform-independent.

Right now, SuperBLT can be compiled on either platform.

Just due to the way the platforms and game versions differ, I don't know that having a cross-platform code base is even reasonable.

Isn't it? That's what SuperBLT is at the moment.

The compiler, standard library, and game engine versions are totally incompatible.

The game is mostly the same on both platforms. Obviously how I find the functions is completely different on both platforms, but the functions are mostly the same.

I think that the only shared code between the two would ultimately just be some glue logic, with the platform specific code still being completely separate.

While this quite possibly would be the case with vanilla BLT, SuperBLT has a lot more platform-independent code.

ZNixian avatar May 21 '18 20:05 ZNixian

@ZNixian i suppose merging the two may be a possibility. Would you be open to moving the project?

Having more development effort focused in one place could also move along the project to map out the engine and support libcxx facets, which would ultimately open the door for other community driven improvement potential, like working voice chat on linux...

RomanHargrave avatar May 21 '18 21:05 RomanHargrave

Would you be open to moving the project?

I do like quite a few of the things GitLab offers (such as built in CI), but if there's a major issue I'm certainly okay with moving it.

Having more development effort focused in one place could also move along the project to map out the engine

Oh yes, I've done quite a bit of research on the internals of PAYDAY 2, including with the asset system. I really need to make it available at some point, probably in the form of headers copied out of IDA. An IDAPython script to import/export the headers to allow collaboration would be great, along with generating structs for the vtables.

and support libcxx facets, which would ultimately open the door for other community driven improvement potential, like working voice chat on linux...

This does make me think about something - considering the amount of Linux-specific code involved, perhaps it would be a good idea to keep the SuperBLT GNU+Linux platform module in it's own repo? That would also neatly sidestep the GitLab/GitHub issue and would keep the commit log for the platform-independent part of SuperBLT clean.

ZNixian avatar May 21 '18 22:05 ZNixian

Having an independent module is not a bad idea. Depending on what needs to be implemented, this repo may be able to suit.

After having looked at the SuperBLT repo, I am interested in the project, but my concern is that were I to submit code to it, or make modifications I would find myself re-indenting all the code in it.

RomanHargrave avatar May 23 '18 14:05 RomanHargrave

Having an independent module is not a bad idea. Depending on what needs to be implemented, this repo may be able to suit.

It'd probably be better to keep the 'classic' BLT4L as a separate repo, both for archival reasons and to avoid confusion.

After having looked at the SuperBLT repo, I am interested in the project, but my concern is that were I to submit code to it, or make modifications I would find myself re-indenting all the code in it.

Yeah, that's something I forgot to mention. So far three people have worked on it, and none of us have done a half-decent job in maintaining a consistent code style.

This is something I do plan to change, ending with writing a GNU Indent rule and putting it in CI to prevent violations. Currently, however, it's unfortunately a mess.

The main issue here is breaking git blame, though I can't see any way around it.

ZNixian avatar May 23 '18 14:05 ZNixian

Unfortunately there are things that will always break git, even when normal. Moving files is a good example.

RomanHargrave avatar May 23 '18 14:05 RomanHargrave

Yeah, good point.

I'll start work on that tomorrow then, if I have time.

ZNixian avatar May 23 '18 14:05 ZNixian

If you desperately need blame, you can have a script save a blamefile for the whole repo every time indent is run. Or just step back one commit and use blame.

RomanHargrave avatar May 23 '18 15:05 RomanHargrave

Anyways. Can you describe what exactly constitutes the platform modules?

RomanHargrave avatar May 23 '18 15:05 RomanHargrave

Stepping back a commit is the method I usually use, it's just a little more inconvenient.

ZNixian avatar May 23 '18 15:05 ZNixian

Also (pressed 'comment' too fast), since most of the indentation is consistent, the main thing that needs to be changed is stuff like what line braces are on, which is relatively minor.

ZNixian avatar May 23 '18 15:05 ZNixian

The thing I saw was

namespace name
{
namespace
{

This can get hard to deal with

RomanHargrave avatar May 23 '18 19:05 RomanHargrave

I generally don't indent the contents of namespaces, as each one of them pushes the code further to the right.

C++17 allows the namespace a::b { syntax though, so I should be able to fix this without any major issues.

ZNixian avatar May 23 '18 21:05 ZNixian

Yes, that has been widely regarded as an overdue solution. Having worked on some other C++ projects like Doomsday, I can say for certain that this can get out of hand. When I was trying to troubleshoot Doom64 support, I found myself having to reverse engineer the source code for then engine due to things like this (no followable indentation, things written entirely in combinations of nested templates and macros, etc...).

RomanHargrave avatar May 24 '18 01:05 RomanHargrave

I have no idea why that wasn't in the standard from day 1.

Both macros and, in particular, templates can get out of hand very easily, with the latter in particular turning otherwise understandable code into a unintelligible sea of symbols (looking at you, Boost).

ZNixian avatar May 24 '18 02:05 ZNixian

I've just pushed 164a950 to the SuperBLT repo, standardizing the formatting with AStyle.

ZNixian avatar May 24 '18 07:05 ZNixian