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

littlexpconnect: Messes up colors #983

Closed
maxhille opened this issue Jan 10, 2023 · 19 comments · Fixed by albar965/littlexpconnect#2
Closed

littlexpconnect: Messes up colors #983

maxhille opened this issue Jan 10, 2023 · 19 comments · Fixed by albar965/littlexpconnect#2

Comments

@maxhille
Copy link

The LittleXPConnect seems to mess up the colors in my sim, see attached screenshots. The reddish one is with the Plugin installed, the proper/white-ish one without the plugin. This is 100% reproducible for me.

I get that XP12 support might just be accidental at the moment and it might aswell be a bug in the sim. Since I have done some plugin dev myself already I might take a shot at it if I find the time (no promises).

OS: Ubuntu 22.10
NVIDIA-Driver: 525.78.01 (ppa)
X-Plane: 12.01b1

CirrusSF50 - 2023-01-10 13 22 49
CirrusSF50 - 2023-01-10 13 24 25

@albar965
Copy link
Owner

Known issue but I have absolutely no idea how to fix this. I already reduced dependencies (shared libraries) to a minimum but it did not help.

What Xpconnect version do you use?

@maxhille
Copy link
Author

maxhille commented Jan 10, 2023

What Xpconnect version do you use?

1.0.33 which came bundled with LNM 22.04-2.8.7.

Intuition says we modify a dataref somewhere, otherwise we should not be able to modify the sims state at all - unless it is an XP bug of course.

@albar965
Copy link
Owner

1.0.33 which came bundled with LNM 22.04-2.8.7.

Ok. This is the latest.

Intuition says we modify a dataref somewhere, otherwise we should not be able to modify the sims state at all - unless it is an XP bug of course.

Nope. For sure not. My internal code even does not allow this. I'd rather suspect that XP does not like my internal multithreading (all safe according to LR specs and not accessing datarefs from other threads than main loop).

You can try to disable Load AI Aircraft Information or Fetch AI in the LNM Xpconnect plugin menu which reduces file accesses.

@maxhille
Copy link
Author

@albar965 I investigated a bit today. It seems like just creating a QCoreApplication instance leads to this behaviour. I created a very stripped down version of LXP to demonstrate this: https://github.com/maxhille/littlexpconnect . Maybe we can approach LR with this.

Is this a Ubuntu/Linux issue only?
Since which XP version does this happen?

@albar965
Copy link
Owner

Thank your for demo. The issue appears only on Linux and only with X-Plane 12.

No idea for removing QCoreApplication. This will affect a lot of program parts like logging, signals, threads and whatnot. I'll have a look if I can simplify this more.

@albar965
Copy link
Owner

Have to see if I can run this without QCoreApplication.

@albar965 albar965 added the reschedule Assign to one of the milestones label Jan 11, 2023
@maxhille
Copy link
Author

QCoreApplication docs make it pretty clear that you cannot have multiple instances in one Process and so it makes sense that stuff breaks this way.

Do you think it makes sense to talk to LR about this? Are you in their 3rd party dev Slack by any chance?

@albar965
Copy link
Owner

QCoreApplication docs make it pretty clear that you cannot have multiple instances in one Process and so it makes sense that stuff breaks this way.

Then why does this happen with only one plugin installed? Who is the other Qt user then? LR certainly not.
Why does it only happen on Linux?
Why only with XP12 and not with 11?

I'll try to get rid of the application instance but I'm not sure if this is doable. Not using Qt is no option since I have to support multiple OS. Have to see.

@maxhille
Copy link
Author

Then why does this happen with only one plugin installed? Who is the other Qt user then? LR certainly not.

Maybe XP uses QT? I don't know.

Why does it only happen on Linux?
Why only with XP12 and not with 11?

The behaviour for multiple instances of QCoreApplication is undefined.

@albar965
Copy link
Owner

Maybe XP uses QT? I don't know.

For sure not. Never seen it mentioned. See no library references.

The behaviour for multiple instances of QCoreApplication is undefined.

You have a source?

@maxhille
Copy link
Author

For sure not. Never seen it mentioned. See no library references.

Maybe it is compiled in statically? If I understand the docs correctly, I can try to check whether QCoreApplication#instance returns something.

You have a source?

From https://stuff.mit.edu/afs/athena/software/texmaker_v5.0.2/qt57/doc/qtcore/qcoreapplication.html#details

For non-GUI application that uses Qt, there should be exactly one QCoreApplication object.

@maxhille
Copy link
Author

maxhille commented Feb 2, 2023

QCoreApplication::instance(); does return null, so I guess we are not competing with another QT instance here.

So for the deeper reason part, I am out of ideas. Maybe @bsupnik can give us a hint?

I'll try to get rid of the application instance but I'm not sure if this is doable. Not using Qt is no option since I have to support multiple OS. Have to see.

What part of QT does the plugin need?

@maxhille
Copy link
Author

maxhille commented Feb 2, 2023

@albar965 Ok I think I found the culprit.

I found another plugin which uses QT (ExtPlane) and they have this interesting line and comment at https://github.com/vranki/ExtPlane/blob/master/extplane-plugin/xplaneplugin.cpp#L54

Tested and verified that this does the trick. I guess behind the scenes something like "string to float" or similar got messed up by QT changing the process locale.

@albar965
Copy link
Owner

albar965 commented Feb 2, 2023

Tested and verified that this does the trick. I guess behind the scenes something like "string to float" or similar got messed up by QT changing the process locale.

Terrific. 👍

That absolutely makes sense. It can mess up number conversion. I also ran into this already.

I'll merge, cherry pick into release/1.0 and attach the builds here for testing.

Thanks a lot!

Alex

@albar965 albar965 reopened this Feb 2, 2023
@albar965
Copy link
Owner

albar965 commented Feb 2, 2023

This new version 1.0.35 includes your fix:
Little Xpconnect-22.04-1.0.35.tar.gz
Let me know if this is working.

@maxhille
Copy link
Author

maxhille commented Feb 3, 2023

Corfirming your build works.

@maxhille maxhille closed this as completed Feb 3, 2023
@albar965
Copy link
Owner

albar965 commented Feb 3, 2023

@jonaseberle
Copy link
Contributor

Thank you both for digging into and discovering that! As a (hobby) Qt user I am reading this with much interest!

A little bit down in that stackoverflow thread it is considered ok to set the user's locale on init. So it's a rather obscure problem where X-Plane forgets to set its required locale when it uses it for conversions. It seems nobody expected a plugin (called from the main thread?) to set it to something else. Bad luck I would say ;) Fantastic that you found a workaround for it!

albar965 added a commit to albar965/littlexpconnect that referenced this issue Feb 3, 2023
@albar965
Copy link
Owner

albar965 commented Feb 3, 2023

Hotfix in Avsim forum: https://www.avsim.com/forums/topic/630237-hotfix-for-issues-with-little-xpconnect-x-plane-12-on-linux/

I also attached the modified builds to the 2.8.8 release: https://github.com/albar965/littlenavmap/releases/tag/v2.8.8

Thanks all for the help and big thank you to Max Hille for the fix!

@albar965 albar965 removed the reschedule Assign to one of the milestones label Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants