Unexpected behavior on file change reload

[jkukatzki]

Hello, first of all thank you for developing this framework. I'm really happy with my solution for video mapping and live shader coding and it has been used several times at events now for up to ten hours without a single crash on low budget hardware. Started out as a hobby for music events but now I already have two commissions lined up for this summer!

Before these events take place I wanted to restructure how I'm handling video rendering as well as implementing video transitions. (star wipes!)
The approach I was thinking of, to make this happen, was to instantiate two RenderVideoComponents for every VideoPlayer and then blend between them using a RenderTextureComponent (does this seem right?)

I already wrote some code for spawning an entity and adding components to it inside my modules service which works as expected but when a reload is triggered by a file change, a new scene is created which does make sense I guess, but I am having trouble understanding how and when the old scene is deleted and its resources released, or how to properly access the new one. Either way I seem to be doing something wrong, since when changing for instance a fragment shader or adding a VideoPlayer the scene isn't populated with my dynamically created entities but it is when starting the program for the first time.

This is my code for creating an entity:

ResourcePtr<Entity> videoRenderEntity = getCore().getResourceManager()->createObject<Entity>();
videoRenderEntity->mID = "videoRenderEntity" + std::to_string(addedIdIfExisting);
//create a VideoRenderComponent for every VideoPlayer found by the ResourceManger and add it to our videoRenderEntity
std::vector<ResourcePtr<VideoPlayer>> videoPlayers = getCore().getResourceManager()->getObjects<VideoPlayer>();
for (auto videoPlayer : videoPlayers) {
	auto renderVideoComponentResource = getCore().getResourceManager()->createObject<RenderVideoComponent>();
	//TODO: handle case in which user has defined components with same name
	renderVideoComponentResource->mID = "foglio_renderVideoComponent" + videoPlayer->mID;
	nap::Logger::info("FoglioService : creating renderVideoComponent for video player %s", videoPlayer->mID.c_str());
	renderVideoComponentResource->mVideoPlayer = videoPlayer;
	auto texture = getCore().getResourceManager()->createObject<RenderTexture2D>();
	texture->mWidth = videoPlayer->getWidth();
	texture->mHeight = videoPlayer->getHeight();
	texture->mFormat = RenderTexture2D::EFormat::RGBA8;
	if (!texture->init(errorState))
		nap::Logger::error("FoglioService: could not init texture for video rendering");
	renderVideoComponentResource->mOutputTexture = texture;
	videoRenderEntity->mComponents.emplace_back(renderVideoComponentResource);
}

And here's how I get the last scene in the SceneSet from SceneService::getScenes() and how I spawn the entity:

Scene* scene;
for (auto it = mSceneService->getScenes().begin(); it != mSceneService->getScenes().end(); ++it) {
	auto& oneOfTheScenes = *it; // Dereference the iterator to access the scene object
	nap::Logger::info("FoglioService: Inspecting scene %s", oneOfTheScenes->mID.c_str());
	for (auto ent : oneOfTheScenes->getEntities()) {
		nap::Logger::info("FoglioService: (inside for loop that inspects all scenes) Entity %s in scene %s", ent->mID.c_str(), oneOfTheScenes->mID.c_str());
	}
	scene = oneOfTheScenes;
}
mVideoRenderEntityInstance = scene->spawn(*videoRenderEntity, errorState).get();

Here's the repository if you want to build and test it yourself, there's some log outputs set up. The code is inside the modules foglioservice.cpp, some of it is really ugly, kindly ignore those parts. https://github.com/jkukatzki/foglio

Thanks.

[TimGroeneboom]

Hi @jkukatzki , thanks for the kind words and great to hear you're getting gigs using NAP.

Have you considered spawning the entity from a scene that you dynamically create (and retain) outside of the resource manager ? So instead of letting the scene loaded from the resource manager spawn the entity, create your own scene (for example in your FoglioService) and spawn all your dynamic entities in that scene, keeping stuff that can get destroyed or initialized by the ResourceManager seperate from objects that you spawned // created with code.

You can take a look at the RenderAdvancedService that does something along those lines, creating a LightScene thats used to spawn dynamic instances of entities.

// Light scene that can be used for dynamically spawned instances
mLightScene = std::make_unique<nap::Scene>(getCore());
mLightScene->mID = scene::light::id;
if (!mLightScene->init(errorState))
	return false;

[jkukatzki]

Thanks for the quick reply. This does seem to make more sense and I implemented it, destroying my spawned entity like this in the preResourcesLoaded call:

if (mVideoRenderSpawnedEntityInstance != nullptr) {
   mInternalScene->destroy(*mVideoRenderSpawnedEntityInstance);
}

But I'm still at a bit of a loss...
The resource manager seems to hold objects that shouldn't exist anymore and hot reloading is really inconsistent, and sometimes, even though it's rare, the first start of the program doesn't setup everything the right way as well.

To my knowledge I used unique_ptrs or naps ResourcePtrs everywhere but I'm really unsure if some memory isn't cleared properly and it's an error on my part or if the ResourceManager isn't behaving correctly.

This is how I get all VideoPlayer devices inside my setupVideoRendering function which I call in my services postResourcesLoaded call:
std::vector<ResourcePtr<VideoPlayer>> videoPlayers = getCore().getResourceManager()->getObjects<VideoPlayer>();
which I then iterate over to create RenderVideoComponents for my entity.
In the video I attached (github embedded videos are kinda buggy, might need to refresh the page to view it) you can see in the last part I delete the only VideoPlayer present in napkin but
nap::Logger::info("FoglioService : creating renderVideoComponent for video player %s", videoPlayer->mID.c_str());
is still being called from inside the for loop, even though it shouldn't be present anymore by my understanding.
You can also see the number, which I add to the entity's name if one with the same name already exists increasing, so the objects held by the ResourceManager also include the ones I created in my previous postResourcesLoaded using ->createObject() which would surprise me, if it's intended behavior, but I'm happy to be proven wrong and learn how to handle it properly.

I also look for shader declarations that are samplers that have names starting with "i_" and then set their texture to an ImageTexture if an image resource with the same name exists, which works perfectly... until it doesn't.

I just can't seem to figure out why and I don't expect you to wade through my code or anything, since I'm also unsure if this still "vanilla" nap usage, as I'm not authoring resources in napkin, but it'd be great to get this working, I tried to make everything more readable in my last commit.

Thanks.

Nap.Forum.Question.mp4

[jkukatzki]

Also stopping video playback from the GUI crashes the program and I can't make sense of the visual studio debugger information, it worked just fine before I moved the GUI update call from my foglioapp.cpp to my modules service.

[TimGroeneboom]

Hey, I managed to compile your project on Linux. Something I needed to change was a lot of initialization functions where missing return statements, on windows this defaults to true, and GNU default to false.

Anyhow, it seems you're struggling a little bit with stuff that is initialized by the resource manager and stuff that you instantiate dynamically.

I don't have time to go over all your code, but lets take void FoglioService::setupVideoRendering(nap::utility::ErrorState errorState) as an example, since a lot of initialization is happening there.

First you create a videoRenderEntity using the resource manager. using

ResourcePtr<Entity> videoRenderEntity = getCore().getResourceManager()->createObject<Entity>();

This will mean this object gets managed by the resource manager. If you want to work outside the resource manager, instantiate it yourself using

auto videoRenderEntity = std::make_unique<Entity>()

The same goes for the components

auto renderVideoComponentResource = getCore().getResourceManager()->createObject<RenderVideoComponent>();

to

auto renderVideoComponent = std::make_unique<RenderVideoComponent>();

you get the gist...

You can then move the created objects using std::move to member variables of the service and they will be retained as long as the unique ptr is on the stack.

Also, later you do the following:

mVideoRenderSpawnedEntityInstance = std::make_unique<SpawnedEntityInstance>(mInternalScene->spawn(*videoRenderEntity, errorState));

The spawnedentityinstance you don't need to wrap it in a unique_ptr, it is retained/managed by the scene so you can change this line into:

mVideoRenderSpawnedEntityInstance = mInternalScene->spawn(*videoRenderEntity, errorState);

and change the type in the header accordingly.

I'm not going over all your code, but it seems like a lot of problems arise due to mixing stuff that you spawned dynamically with objects that are managed by the resourcemanager.

[cklosters]

In addition to what @TimGroeneboom already covered in his answers, I would strongly recommend following the dynamic pattern used in the naprenderadvanced service: Where we attach a shadow camera to every light, to render shadow maps with:

Example from spotlightcomponent.cpp

// Create shadow camera resource
std::string uuid = math::generateUUID();
mShadowCamEntity = std::make_unique<Entity>();
mShadowCamEntity->mID = utility::stringFormat("%s_shadow_%s", getEntityInstance()->mID.c_str(), uuid.c_str());

// Transform component
mShadowCamXformComponent = std::make_unique<TransformComponent>();
mShadowCamXformComponent->mID = utility::stringFormat("%s_shadow_xform_%s", getEntityInstance()->mID.c_str(), uuid.c_str());
mShadowCamEntity->mComponents.emplace_back(mShadowCamXformComponent.get());

// Shadow Perspective camera component
mShadowCamComponent = std::make_unique<PerspCameraComponent>();
mShadowCamComponent->mID = utility::stringFormat("%s_shadow_camera_%s", getEntityInstance()->mID.c_str(), uuid.c_str());
mShadowCamComponent->mProperties.mNearClippingPlane = resource->mClippingPlanes[0];
mShadowCamComponent->mProperties.mFarClippingPlane = resource->mClippingPlanes[1];
mShadowCamComponent->mProperties.mFieldOfView = mAngle;
mShadowCamEntity->mComponents.emplace_back(mShadowCamComponent.get());

// Shadow Origin component
mGnomonMesh = std::make_unique<GnomonMesh>(*getEntityInstance()->getCore());
mGnomonMesh->mID = utility::stringFormat("%s_shadow_gnomon_%s", getEntityInstance()->mID.c_str(), uuid.c_str());
mGnomonMesh->mSize = mResource->mLocator.mGnomonSize;
if (!mGnomonMesh->init(errorState))
	return false;

mShadowOriginComponent = std::make_unique<RenderGnomonComponent>();
mShadowOriginComponent->mID = utility::stringFormat("%s_shadow_origin_%s", getEntityInstance()->mID.c_str(), uuid.c_str());
mShadowOriginComponent->mDepthMode = EDepthMode::ReadOnly;
mShadowOriginComponent->mMesh = mGnomonMesh.get();
mShadowOriginComponent->mLineWidth = mResource->mLocator.mLineWidth;
mShadowCamEntity->mComponents.emplace_back(mShadowOriginComponent.get());

// Shadow Frustrum component
mShadowFrustrumComponent = std::make_unique<RenderFrustumComponent>();
mShadowFrustrumComponent->mID = utility::stringFormat("%s_shadow_frustrum_%s", getEntityInstance()->mID.c_str(), uuid.c_str());
mShadowFrustrumComponent->mLineWidth = resource->mLocator.mLineWidth;
mShadowCamEntity->mComponents.emplace_back(mShadowFrustrumComponent.get());

// Spawn it in special light scene
mSpawnedCamera = mService->spawn(entity, error);

Make sure to destroy the entity when the resource manager destroys your resource:

void LightComponentInstance::onDestroy()
{
	if (mSpawnedCamera != nullptr)
		mService->destroy(mSpawnedCamera);
	mService->removeLightComponent(*this);
}

When you are iterating over your scenes or using preResourcesLoaded to destroy / create resources you are almost always doing something wrong, the ::init() of your resources should ensure everything is created and ininitialized properly. This is also where you would dynamically create resources or entities (see example above) - completely seperate from the resource manager. You should manage those yourself and use std::make_unique<T>(), not ResourceManager::createObject - because that adds it as a resource to the manager, which implies the resource manager should attempt to manage and patch it, which isn't required because you are taking care of that - they are explicitly created and destroyed independent from your file, similar to the camera attached to a light that renders the shadow map. The dynamic camera is destroyed when LightComponentInstance::onDestroy is called at the right time. Note that we always create and bind objects to a resource that is loaded from file - we (almost) never globally create / destroy objects, it's always local to the object that requires and uses it. This limits the scope and is very close to RAII - a common and good practice to follow here. NAP takes care of the rest (order of initialization etc.). I also see that you

Another thing to be aware of is that the resource manager only destroys objects on shutdown, never before. If you delete an object in Napkin it isn't destroyed. This is something we are aware of but have no solution for atm because it interferes with the way we handle null (invalid) ptrs, ie: do we accept objects to be removed if they're referenced? But that's not of interest here.

Try to find a pattern whare you declare a resource in Napkin that spawns the required dynamic resources and components using the pattern above and try to avoid using ResourceManager::createObject().

I will try and write a FAQ that answers some of these more common questions regarding programming patterns in NAP. But the demos are your friend here. If you run into problems try to reproduce them in a simpler project and when it's a bug report it with a repro case.

Regarding video mixing: you should only have X amount of players for the amount of videos that you want to mix. You should use 1 nap::RenderVideoComponent for every nap::VideoPlayer - to convert the YUV textures into a single RGB texture, and use that texture as the input for a video processing pipeline that can be customized to your liking. If you know the required number of players you should declare those in napkin and avoid creating them dynamically. There is simply no point in doing that, other than complicating your system for no reason.

[jkukatzki]

Hello, thank you for such thorough answers. Very useful to know about the compile errors, I've been putting off trying to compile on linux for a while but will try building on debian and implementing those fixes once I can make some time for it, maybe I'll even go through the article you have on building for raspberry pis while I'm at it.
I did try the approach of only using unique ptrs, but as the behavior with VideoPlayers persisted, I messed around with the ResourceManagers createObject function as well to see if it changed anything. What you saw was just a snapshot of my attempts of fixing it, same with the memory clearing in the preResourcesLoaded call.

Another thing to be aware of is that the resource manager only destroys objects on shutdown, never before. If you delete an object in Napkin it isn't destroyed. This is something we are aware of but have no solution for atm because it interferes with the way we handle null (invalid) ptrs, ie: do we accept objects to be removed if they're referenced? But that's not of interest here.

Glad to know I'm not going insane at least.

I really want to avoid having to author every RenderVideoComponent myself though, I'd rather have an overview of every VideoPlayer with a preview automatically. And then, like with images, set the texture of any sampler declarations that start with "v_" inside the passes that my RenderCanvasComponents own, which basically just take the texture of the last pass and put an effect on it described by a fragment shader (like the alpha mask pass cutting out the fractal shader in the video I posted, by using an image with an alpha channel).
Are you saying this isn't possible with hot reloading because they are only destroyed on shutdown?
Or is there a way to atleast see that they are "marked" as deleted and not handle those?
Is it even viable to create anything dynamically inside the services init function if they are dependant on objects inside the objects.json?
If it's not an option to handle it inside my service this way, I think I'd just create something like a "HandleFoglioResourcesComponent" holding a VideoPlayers array as well as an Images array in their RTTI inputs which I then add every resource to in napkin that I specifically want to have handled if their names are referenced in any passes that my RenderCanvasComponent uses. Does this seem more viable? What about RenderTextures I have to create using the ResourceManager, since RenderVideoComponents outTexture only takes ResourcePtrs? Wouldn't it eventually clog up the system if they don't get destroyed?

Sorry for asking so many questions.

Also my understanding was, since the entities can be unique every file load, that I might as well only define entity and component resources inside the scope of the postResourcesLoaded call, so I dismissed putting any resource initialization in the init function.

The sampler texture not being set sometimes is probably a different issue, hope I can resolve that one.

Also I just noticed I described my approach for video blending wrong.
For blending a video with the one that is next up in the VideoPlayers video files list I'd have to create a copy of the VideoPlayer and start playback of the video that is up next on the copied VideoPlayer once the last one nears it's end and then blend between those two textures, and then do the same the other way around once the new video nears it's end. Just wanted to be clear there.

[cklosters]

Are you saying this isn't possible with hot reloading because they are only destroyed on shutdown?

No, that's why you need to use unique pointers if you dynamically want to spawn entities at runtime. Just make sure to delete them on destroy of your resource. If you look at the example above we don't author every camera ourself, the system does that on initialization of a light. You can do something similar, where a resource defines how many objects to dynamically spawn. It is also fine to link to resources in dynamically created objects, If the resource you link to is changed in Napkin the object that links to it will be re-initialized - thus creating new instances and destroying old ones. (local - not global!). Note that items that change (in Napkin) are destroyed, ensuring the init pattern works as expected. Only when you delete an object that is linked Napkin doesn't destroy it immediately because it could leave the app in an undefined state (dangling references).

From what I gather this is not a system issue but more of a general architectural issue: You don't seem to have a good grasp on what it is you actually need and thus have to create. I recommend going back to the drawing board and figuring out what resources are required (x amount of players, thus x amount of render video components etc.) and design the system from there.

Also my understanding was, since the entities can be unique every file load, that I might as well only define entity and component resources inside the scope of the postResourcesLoaded call, so I dismissed putting any resource initialization in the init function.

Don't do this, I explained why above.

I think I'd just create something like a "HandleFoglioResourcesComponent"

This is a better approach, let resources handle spawning of dynamic objects, not any global object including services.

For blending a video with the one that is next up in the VideoPlayers video files list I'd have to create a copy of the VideoPlayer and start playback of the video that is up next on the copied VideoPlayer once the last one nears it's end and then blend between those two textures, and then do the same the other way around once the new video nears it's end. Just wanted to be clear there.

A video player plays back a video and stores the result in 3 textures that you render to 1 RGB texture using a render video component. Your system should have only 2 video players if you want to blend between 2 videos, which you can define in Napkin.

Other than that I can't help you here. I'm going to close this discussion, feel free to open a new one but try to be more specific. Your questions are currently a bit out of scope and hard to answer. Try to break down your problems in smaller projects and take it from there. Similar to previous discussions we had on the old message board.

[jkukatzki]

I think we're talking past each other sadly, my problem was that there's not any ground truth of objects that reside in the json file, like VideoPlayers, that I can get from the ResourceManager, which was what I expected when I call getObjects in the postResourcesLoaded call. I don't even need dynamically created VideoPlayers at this point. I'm quite confident in my design, even more so because it works perfectly, I just wanted hot reloading to work as well. Anyways, thanks for your help.

…

On Wed, 19 Jun 2024 at 10:17, Coen Klosters ***@***.***> wrote: Closed #27 <#27> as resolved. — Reply to this email directly, view it on GitHub <#27>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASMHJBX5H6TQT4VTJQUCS6DZIE5BNAVCNFSM6AAAAABJC2RFXCVHI2DSMVQWIX3LMV45UABFIRUXGY3VONZWS33OIV3GK3TUHI5E433UNFTGSY3BORUW63R3GEZTINRRHA3Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cklosters]

Don't use getObjects in the postResourcesLoaded call, explicitly create and link to objects on init, that's all. If you do that and follow general init() guidelines hot reloading should work as expexted. Globbing is always a bad idea and should be avoided, as explained. I am going to remove the getObjects call in a future version of NAP because of this.

[jkukatzki]

If you could just answer one more thing I'm curious about then I'll be off your back: In the RenderBloomComponent in the flocking demo a render target and texture is created using the createObject function, if this happens on every init wouldn't these add up after some time? Thanks.

[cklosters]

every init wouldn't these add up after some time?

In theory yes. You ask the resource manager to create and manage an object. The resource manager doesn't know when the object isn't required anymore (relation to other resources = unknown) so will keep it in memory until the application is closed. That's why in these situations it's better to create and destroy objects on init() yourself, as seen in for example the video player:

// Create V Texture
mVTexture = std::make_unique<Texture2D>(mService.getCore());
mVTexture->mUsage = Texture::EUsage::DynamicWrite;
if (!mVTexture->init(tex_description, false, 0, error))
    return false;

When the video player goes out of scope the texture is destroyed. The resource manager could decide to delete the player when a change is detected, as part of the ResourceManager::loadFile call. Consider the following example:

You have a nap::VideoPlayer and nap::RenderVideoComponent, where the component links to the player. When you add a video to the VideoFiles property and save the file (triggering a hot-reload) NAP does more or less the following:

• Loads and deserializes the file - creating new objects
• It detects that the video player has changed and needs to be updated (by comparing properties)
  • Objects that link directly link to the video player are patched:
    • This means that the every ObjectPtr with the same ID is given the address of the new one, releasing the old one (patched)

But we're not there yet. Every object that links to the video player (directly or indirectly) needs to be re-initialized, because it (might) require access to the YUV textures the player created. So the system will initialize those in the right order, starting with the video player and working it's way back up. It knows how to do this because it build a graph that maps out all dependencies.

• If initialization succeeds the resource manager calls onDestroy before deleting the replaced (old) object. This also frees everything managed by that object, including in this case the YUV textures.

That's why it's important to create your resources on init(), which is called at the appropiate time by the resource manager. I've seen others (including NAP developers) create and manage objects on postResourcesLoaded, which is always a sign of code smell - there's always a better, more local solution.

On that note:

We could (should) adjust the RenderBloomComponent to create and manage it's own resources instead of using createObject.