Unable to remove shape exception with screen painter

[zcsoka]

Hi Svyatoslav,

Have you managed to reproduce the issue?

[Svyatoslav]

Hi, Working on that right now. Yes, I could reproduce it and I still suspect that addon might be doing something wrong, but my plan is to do it two stage: I want to add some basic protection from invalid input (so bookmap no longer crashes deep inside the core code but instead clearly complains wherever method was called) and then I'll take a closer look at what the addon is doing and if that's correct. I hope to provide you some info within couple days.

[Svyatoslav]

I'm not 100% sure, but I think I know what's going on here: when tab is switched back, addon creates a new canvas. Then it tries to delete an old shape from the new canvas. But the old shape does not belong to the new canvas - it belongs to the old one. So the crash happens.

You can only delete a shape from the same exact canvas that you added it to. I think that's the cause of the issue. If my guess is correct - api is working fine, though the exception is cryptic.

(I'm still going to try making crashes a bit more informative, but I hope that the above will help you with the issue while waiting for next release).

[zcsoka]

Thanks Svyatoslav. Can I detect somehow the tab change? I thought the canvas was tied to the tab. Then basically the issue persists with each and every screen painter. Do you have an idea how to circumvent it? Unfortunately there is not much info on screen painters, therefore it is hard to decode what is happening in Bookmap when I use one.
 

[Svyatoslav]

Take a look at Layer1ApiCanvasColorsDemo. Your ScreenSpacePainter should have #dispose() called once user leaves the tab. That's when you are expected to dispose any canvases that you've created. In current version it looks like dispose might not be called - we'll take a look into that and will try to fix. When tab is reopened you should expect to receive a request for new painter creation.

Basically, intention is that model gets destroyed once tab is left and recreated once it's opened again. Right now it looks like you don't get notified properly when the tab is left and the old model just gets abandoned.

[zcsoka]

In my code I call the canvas.dispose(). In that case I have to wait for your fix before I contest further. 
Many thanks for looking into it. 

[Svyatoslav]

In the version that I tested you create new canvas and then try to dispose the *old* icon as if it's part of the new canvas. But it's not. Please, consider logging all icon removals and additions, as I said. Just do it together with object ID's of the canvases to which you direct those requests. You should see that you are sending removal request to a new canvas, not to the old one.

[zcsoka]

Hi Svyatoslav, 

I understand what you are implying, but since the dispose is not called, I cannot clean up. 
In the update method I delete all icons and generate new ones. In the dispose I also delete all icons, clear the icon data and dispose the canvas. Since the dispose is not called, I have no means to remove the icons, so I suppose when I change tab, the update method on the old SSP still tries to remove shapes from the old canvas. Without a called dispose I cannot really clean up, can I? Maybe I can recreate the canvas in every single update, but I am not sure about the implied cost.

[zcsoka]

As a matter of fact, I tried the only removal I have to put into a try/catch, but could not catch the exception and I also could not find my code in the stack traces. 
I logged the canvas ids and I can confirm, that for different tabs I have different ids, there seem to be no mix between the canvases.

[Svyatoslav]

As a matter of fact, I tried the only removal I have to put into a try/catch, but could not catch the exception and I also could not find my code in the stack traces. 

Yeah, right. That's one of the issues I'm working on. What happens is that bookmap will schedule the request and crash later, basically. Going to be fixed in upcoming versions (will throw exception immediately).

According to what I see on my side with mazones-1.0.0.jar, you seem to create a canvas in response to createScreenSpacePainter and then run removeShape on the newly created canvas. But it does not have a knowledge of the old shape because it's a fresh canvas that was just created.
Without dispose (that I still intend to fix though) it's a bit tricky, but actually doable - you need to discard all the shapes cached elsewhere whenever you are creating a new canvas for the same alias. I believe what might be going on here is that you cache those in some storage that has a longer lifecycle and is not purged on creation of the new canvas.