Refactor Application.cpp #939
Conversation
HifiExperiments
added
needs CR
|
|
||
| bool _profilingInitialized { false }; |
There was a problem hiding this comment.
oh yeah @daleglass it didn't look like this was ever set to true, so I deleted it + the one place it was used, is that ok?
There was a problem hiding this comment.
Oops, that looks like something I screwed up.
Memory is a bit rusty right now about why I added that, but I take it PROFILE_RANGE_IF_LONGER and similar are not safe to call before some sort of initialization is done, so a _profilingInitialized=true; is supposed to go somewhere in the startup code after some profiling system init code is called.
There was a problem hiding this comment.
it seemed to be working ok for me without it, maybe if it's a problem during testing I can add it back?
There was a problem hiding this comment.
Sure, it's simple enough that it'd be easy. I did quite a lot of rearranging of that code, and right now can't recall if maybe I just fixed this in some other way in the end.
|
hm I'll look into that... |
| scriptable::ModelProviderPointer provider; | ||
| auto entityTreeRenderer = qApp->getEntities(); | ||
| auto entityTree = entityTreeRenderer->getTree(); | ||
| if (auto entity = entityTree->findEntityByID(entityID)) { |
There was a problem hiding this comment.
In some places, for example in EntitySendThread there's withReadLock around findEntityID call. getEntityModelProvider is called from Graphics.canUpdateModel so it can be called from any script engine thread. I wonder if it needs a lock here.
There was a problem hiding this comment.
Maybe as a later PR we should do a thread safety audit of all the places where entity tree is used?
There was a problem hiding this comment.
oh wow great catch. EntityTree::findEntityByID actually calls EntityTree::findEntityByEntityItemID which does internally use a read lock, but then EntityTreeRenderer::renderableForEntityId looks like it needs it? I filed #1015
| #endif | ||
|
|
||
| using namespace std; | ||
|
|
There was a problem hiding this comment.
Do jsdoc config files need to be updated to point to new files?
There was a problem hiding this comment.
huh maybe? how do I do that?
|
I'm also having trouble building it on Linux using Overte-builder: |
HifiExperiments
force-pushed
the
application
branch
from
b5cc2c6
to
69de648
Compare
|
I tested it on Windows, and it asks to log in every time. A different than usual login screen is used for this:
|
|
It also seems that login screen that it uses in VR is intended for desktop? |
|
The good thing about that different login screen though is that it has option to continue anonymously. Ours is badly missing this. |
|
I believe I've fixed the audio device issue. for the login screen, it turns out I had accidentally deleted I wasn't able to reproduce the issue in kraftwerk; I wonder if the download got corrupted or something? does it still happen if you clear your caches, including ktx cache? |
|
I gave this a test on Linux and so far, everything works fine. I tried clearing shader cache and ktx cache as well. In both scenarios, Kraftwerk renders properly. |
|
I tested it again and now everything works well :) |
|
Would someone like to test in VR on Windows? I don't have a VR setup at the moment. If all works well we could merge it. |
|
there are some conflicts now, I’ll probably wait until after the next release to rebase and resolve them just to avoid causing any problems |
I did some brief testing in VR and everything seems to work as expected.
I will also take a look at this again after the rebase! |
Closes #931
(apologies to whoever reviews this)
Application.cpp has gotten totally out of control over the years (in many cases because of my dumb decisions). This is an attempt to corral it. I've organized the code as best I could into sections - Assets, Camera, Entities, Events, Graphics, Plugins, Setup, and UI - and divided the implementations into separate, smaller files. I completely rebuilt the includes from scratch so we only have what's necessary. I broke some of the classes into their own files. Overall, I changed very little functionally. I removed a few unused variables and functions but nearly everything is directly copy-pasted.
Testing-wise, things should just work as before. In particular, anything related to startup, login UI, and plugins.
Funding
This project is funded through NGI0 Entrust, a fund established by NLnet with financial support from the European Commission's Next Generation Internet program. Learn more at the NLnet project page.