Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PNG Loader missing from basegame #102

Open
ZNixian opened this issue Jan 10, 2018 · 34 comments
Open

PNG Loader missing from basegame #102

ZNixian opened this issue Jan 10, 2018 · 34 comments
Labels
bug Confirmed fault in BLT4L payday-linux-specific Issues relating to PAYDAY2 insofar as Linux specific changes

Comments

@ZNixian
Copy link
Member

ZNixian commented Jan 10, 2018

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

@ZNixian ZNixian added the bug Confirmed fault in BLT4L label Jan 10, 2018
@RomanHargrave
Copy link
Member

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

@ZNixian
Copy link
Member Author

ZNixian commented Jan 11, 2018

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
Copy link
Member Author

ZNixian commented Jan 11, 2018

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

@ZNixian ZNixian changed the title DB:reload_textures does not appear to work with the custom assetloader implementation PNG Loader missing from basegame Jan 11, 2018
@ZNixian
Copy link
Member Author

ZNixian commented Jan 11, 2018

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())

@RomanHargrave
Copy link
Member

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 RomanHargrave added the payday-linux-specific Issues relating to PAYDAY2 insofar as Linux specific changes label May 20, 2018
@ZNixian
Copy link
Member Author

ZNixian commented May 20, 2018

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.

@RomanHargrave
Copy link
Member

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
Copy link
Member

@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.

@ZNixian
Copy link
Member Author

ZNixian commented May 21, 2018

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?

@RomanHargrave
Copy link
Member

When did it get gnu/Linux support?

@RomanHargrave
Copy link
Member

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

@RomanHargrave
Copy link
Member

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

@ZNixian
Copy link
Member Author

ZNixian commented May 21, 2018

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.

@RomanHargrave
Copy link
Member

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.

@ZNixian
Copy link
Member Author

ZNixian commented May 21, 2018

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.

@RomanHargrave
Copy link
Member

@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...

@ZNixian
Copy link
Member Author

ZNixian commented May 21, 2018

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.

@RomanHargrave
Copy link
Member

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.

@ZNixian
Copy link
Member Author

ZNixian commented May 23, 2018

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.

@RomanHargrave
Copy link
Member

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

@ZNixian
Copy link
Member Author

ZNixian commented May 23, 2018

Yeah, good point.

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

@RomanHargrave
Copy link
Member

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
Copy link
Member

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

@ZNixian
Copy link
Member Author

ZNixian commented May 23, 2018

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

@ZNixian
Copy link
Member Author

ZNixian commented May 23, 2018

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.

@RomanHargrave
Copy link
Member

The thing I saw was

namespace name
{
namespace
{

This can get hard to deal with

@ZNixian
Copy link
Member Author

ZNixian commented May 23, 2018

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.

@RomanHargrave
Copy link
Member

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...).

@ZNixian
Copy link
Member Author

ZNixian commented May 24, 2018

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
Copy link
Member Author

ZNixian commented May 24, 2018

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

@ZNixian
Copy link
Member Author

ZNixian commented Jun 30, 2018

Update on the above items

  • 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. Continuous Integration now builds all commits (though I haven't finished my testing system) and I use tags for each release.
  • 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.
  • 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. This can be solved by hosting the GNU+Linux module on GitHub.

@RomanHargrave
Copy link
Member

@ZNixian i (think i) have a gitlab account (roman@hargrave.info) if you want to add me as a collaborator

@ZNixian
Copy link
Member Author

ZNixian commented Jul 6, 2018

@RomanHargrave Done.

@RomanHargrave
Copy link
Member

Cool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed fault in BLT4L payday-linux-specific Issues relating to PAYDAY2 insofar as Linux specific changes
Projects
None yet
Development

No branches or pull requests

2 participants