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

Crash loading old models #4637

Open
mike-f1 opened this issue Jul 8, 2019 · 18 comments

Comments

Projects
None yet
6 participants
@mike-f1
Copy link
Contributor

commented Jul 8, 2019

Observed behaviour

Pioneer crashes during loading, with these lines on console:

...[SNIP]...
new ModelCache
Shields::Init
BaseSphere::Init
GenerateIndices: triangles count = 2312, mid indexes = 6528, hi edges = 102
CityOnPlanet::Init
decompressed model file Dome1.sgm (137.79 KB) -> 354.22 KB
LoadSurfaceFromFile: models/buildings/vlastan/Dome1_D.dds: Unsupported image format
LoadSurfaceFromFile: models/buildings/vlastan/Dome1_S.dds: Unsupported image format
LoadSurfaceFromFile: models/buildings/vlastan/Dome1_G.dds: Unsupported image format
LoadSurfaceFromFile: : could not read file
No loader for: Group
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
Aborted (core dumped)

Expected behaviour

Pioneer load...

Steps to reproduce

Download or pull last master, compile and run

Hints

It may be related to #4626 or to #4628, thus pinging
@Web-eWorks and @pcercuei

My pioneer version (and OS):
last master (0f3379f) on Ubuntu

@pcercuei

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

It's not related to #4628, I reverted it locally and it didn't fix the bug (which I can reproduce).

@pcercuei

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

I bisected the issue, 09a84e6 ("Better profiling, speedup model saving / loading.") is the first bad commit.

@Web-eWorks

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

That's very weird, because it worked perfectly fine on my end during development. I am going to be unable to seriously debug this code for several weeks, but I'd recommend looking in TextureBuilder.cpp as – IIRC – that's where LoadSurfaceFromFile is defined.

@mike-f1

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

Ok: I just did some investigation.

It seems that if all models are not compiled or all are compiled it works like a charm.
Thus I removed manually some *sgm files and... it keep working 😟

I'm unable to reproduce the bug. @pcercuei , are you able to reproduce it?

OT(?): I get all icons "darker" (which is probably an unrelated issue, but I would check if there's someone else that get the same):
DarkIcons

@pcercuei

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

I removed the installed files, then installed them again, now it works.

@mike-f1

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

I removed the installed files, then installed them again, now it works.

what a pity :/

Anyway:, the only idea I got is of some issue when loading models compiled with the old algorithm, but doing some investigation will take time I don't have right now. @Web-eWorks, did you do some test in order to exclude this scenario?

@vakhoir

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

OT(?): I get all icons "darker" (which is probably an unrelated issue, but I would check if there's someone else that get the same):

Yeah, I was about to ask if anyone had the same problem too.

@fluffyfreak

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

You can probably test this by having a few of the older SGM files in the mods folder.

@CylonicRaider

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

I observe darkened however-that-particular-UI-module-is-called buttons as well. The final on-screen color is #000000 in the main menu and #000010 in-game, and the buttons appear opaque. Perhaps some sort of redundant alpha pre-multiplication or other alpha blending failure? While they look snazzy, I wish they had a distinct on-hover color like they used to.

@vakhoir

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

I figured it out in #4624, it's because of the casting to uint8 in the Shade method introduced in c19aab4.

@pcercuei

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

Oops. Sorry about that.

@pcercuei

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

Both the screenshots issue and the transparency issue are fixed in #4640.

@mike-f1 mike-f1 changed the title Crash on load Crash on load, in case of old models [to be confirmed] Jul 12, 2019

@mike-f1

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2019

Both the screenshots issue and the transparency issue are fixed in #4640.

Nice&quick work 😄

Then, back to original title,

You can probably test this by having a few of the older SGM files in the mods folder.

Right: I simpli don't know when I have enought time to recompile on an old version; compile models; then recompile on newest 😅

I leave this open for now.

@fluffyfreak

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2019

I can confirm this is a bug, SGM6 non-lz4 appear to be broken. Not tracked down the cause yet but I've got a callstack where it's failed in deserialising GeomTree.

I'll dig around a bit.

@fluffyfreak

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2019

Hmm, interesting, in BinaryConverter::LoadNode it seems that it doesn't find the loader from m_loaders for "Group" despite it being present and the string being correct.

@fluffyfreak

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2019

Changing https://github.com/pioneerspacesim/pioneer/blob/master/src/scenegraph/BinaryConverter.cpp#L451 to read auto loadFuncIt = m_loaders.find(ntype.c_str()); seems to fix it but I'm a bit baffled why it happened. I think because the actual ntype string is "Group/0" perhaps.

Hmm, spoke too soon, some models work ok, all of them load, but many have corrupted/default textures. Shields seem broken for many of them so perhaps the material loading is still going wrong.

@Web-eWorks

This comment has been minimized.

Copy link
Member

commented Jul 14, 2019

Hmm. I thought I rewrote string loading/saving functionality to be compatible with the previous iteration. There's probably an off-by-one error in Serializer::Reader::operator<<(std::string) compared to the previous behavior (though it works just fine with SGM files written by the Writer counterpart).

@mike-f1 mike-f1 changed the title Crash on load, in case of old models [to be confirmed] Crash loading old models Jul 15, 2019

@fluffyfreak

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2019

After wasting some time this morning writing tests for the serialisation I can confirm that it works just fine. So the problem is being introduced elsewhere. Possibly conversion between C-style strings and std::string operations maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.