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

PolylineCollection update instanceIndex is out of range #8321

Closed
tinco opened this issue Oct 25, 2019 · 14 comments · Fixed by #8546
Closed

PolylineCollection update instanceIndex is out of range #8321

tinco opened this issue Oct 25, 2019 · 14 comments · Fixed by #8546

Comments

@tinco
Copy link
Contributor

tinco commented Oct 25, 2019

Browser: Chrome
Operating System: MacOS
Cesium version: 1.5.3

Hi, we have an app where we create and destroy a bunch of Polyline's. Basically the user cycles through different polylines, and every time they select a new one we delete the ones they were previously looking at and create the ones they're looking at now using.

The polylines also have a CallbackProperty on the positions property that updates the positions every frame.

We get an error from Cesium originating on this line:

https://github.com/AnalyticalGraphicsInc/cesium/blob/d26ccfea38dcf4623cf5da3faca141913c64cfe1/Source/Scene/PolylineCollection.js#L471

Quite reliably when we delete a set of polylines and then immediately render frames. If we remove the CallbackProperty the error does not occur.

Are we hitting some race condition bug where callback properties are setting a dirty flag on polylines that already have been deleted?

We're creating the polylines with viewer.entities.add and destroying them with viewer.entities.remove.

Screen Shot 2019-10-25 at 4 53 17 PM

@hpinkos
Copy link
Contributor

hpinkos commented Oct 25, 2019

Hello @tinco, I believe we fixed this issue in a newer version of CesiumJS. Can you try updating to the latest release and see if you still experience the problem?

@tinco
Copy link
Contributor Author

tinco commented Oct 25, 2019

hi @hpinkos thanks for the quick reply! I just upgraded to Cesium 1.62, and the error definitely became less reproducible, it takes a couple tries before it breaks now.

Screen Shot 2019-10-25 at 5 43 04 PM

@hpinkos
Copy link
Contributor

hpinkos commented Oct 25, 2019

Thanks for trying that out @tinco. Could you put together a code example that reproduces the issue in Sandcastle? That would be a big help for us tracking down the problem.

Here are examples of similar crashes we've fixed previously, if it's helpful to see their code examples: #4983 (comment) #7153 (comment)

@tinco
Copy link
Contributor Author

tinco commented Oct 31, 2019

Hi, thanks for the examples. We've been trying to reproduce in Sandcastle, but haven't had any luck so far. It seems that disabling the positions callback fixes our problem, but disabling requestRenderMode also does.

My colleague is working on a bigger Sandcastle right now to see what else we need from our app to reproduce the problem.

@tinco
Copy link
Contributor Author

tinco commented Oct 31, 2019

Ok we made some progress, this Sandcastle is huge, but if you leave this running for a couple minutes you'll eventually get the instanceIndex is out of range error:

https://sandcastle.cesium.com/#c=7Vptc9s2Ev4raD7U1JwCE3ynHGcudXy5XpPYE7vNdWxPh6YgiQlF8gjKjpvqv9+CAEmApGyn13TuZi7tJCKw2F3sy4PFkjdRiW4SektLdIgyeouOKEs2a/xTPWbsxfXjUZ5VUZLRcm+KPl9mCFW0LGHgtMxvkjktZ82yuKRRRd/nZTo/FyTGZMoXlPRfG8qqdzQD8jf5nM5QVW4ozG0nB5fZZSaUwCymGcVzer1Znq3y27+V0ZqyU1qe0TjP5qAjX1XTg+JVklJGK11z8Y/98lxMGrW+mzKdob1VVRVstr/PqryMlhQv83yZ0qhIGI7z9X5Ey5zFUfY0YrCQPZ3Tm/15VEW/8Kd9izz94e/lzT8259Z3VfniReJ75T/Pfj6iP57tS0XwB5Zne+NbKspknVTJDWU4ms8NuYITSrJf83x9nqsTl9lik8VVkmeo2KTpac4S/nCeH4FRysgo5MBE+KSk1abUSVlLe9EQX00uzCtQcSd7NuDPegKaYbyOCsMoJujwuSBACJzEKhTNP2xYRec9x0RlBb+izDYmKvU8KalQ4xBpJotrNXA7LxYNuOGMLiHojJZu2iow2bkkL9dRmvxKjYb0EYvWm7RKivTuu7uzOEqjUllrYtNVOKD9feTGazmwplmFqvw2KucMiU3tEsFDo+NaDJSSLmhG+eB2Ir0ZpxC36A2N2KaktczaKbWRy00MMW/A74p+qoBtluVVJIxV5ElWHWcVOJWyKUohz5unSePXagU5ohGCsy6uDpRZdd1gspMHU92DSiJ1g3n56+Ae0drzPVqojzXZlv/FuPQYCbAaM8pEj+ddWxOzO82iTJ8UdcbArGSMUL6pON+jPM0BP7U8SdPrKP4I4FrQsrozDDXD+J9kgYxOWcxoCoEP6fbtt8oeOKQVeQZxcArxNVHXt3HUSOQ64J+PX78+eX/QkW27nzRldJfYR7A++vnF28cz/lLF3xy//P7HN2en775/++rVu+Pj+0Q9yOzli3c/nLx78fbV8RiX7RQtIuAjDjXFje+TebWaIasdj4d+5SOGCWAh/sek41Ikn2h6Bng0Q64c3Er5imH4YYTXIsFPeVgxvMjL4yheAQzzZz1QRPyp9F1yYSoDtoYcxSqAXA3Az0TsTrvJbubheN1xZHEtB0ZEQtJMS5Z2tj5OFbIm13CxYStD3V9D2K6Q5xFUInevabasVhr8jBk0rckO+uk/ksDx/zP3fzpzl1EhwVdjcw4szk5fvDt+e96SziO2EvEzQ8TUhk+jCophyAjL7WduFz1v4JwpkyjVC6LTPL3jsy+BTUPRho4SdJPfgQRTlECl/UmPvxRq5UycsRlkZ2sm7sWaHP0FEXR4eKhmzOSe+K3VWNJ8TavyDld3Ba1X7/GdLaEU7oWAlG1+mZdHvdn86C+SItrdHAxWdF55BBoW0kUzXTEFJNkMXUiL3+ufC67Z1dVUZaPw+DI81er6R8sew12EbvsnF/+zluE408JXIdmOobNaJwlw5iPdfGNtUd5rV0JjAN0yxblhlJr2y0vYruybg7QyvzN61V17/VXL0F01ZZdmVM8teXNpAwnUzW+oQQf7Gtaz/zlP7do0Ztdtc0eod1wmy1X1HnIkv+VAfVKnwmfEA0cGepPT8Ih4Ws+6nJau1wJsJtS4+Iw+zZDth15o2phYJjEdy/H9KQJGdhjYTohD4pKABL5PpuhXqHbM0LICG65Pvhm4XkjayFJ4Eex4xAktYnoqK9+DIT/wA09hZeLACpzQCUY5mcDJce3QcmxNqdD1bMcy7dBVWBHsOqZn+iQMx3hZmHgWCWyTtKxcKOp82/Y81woDX9tgYFt+SEJioe3Vgx45v83/eI8QHBDiBcR1HVVhC+zh24HjB46isINNxw1s3yG2O25IM3Rc8KXruSo3GPVMWESIZkiwsu2ZbjjCKwhDbIIgxwKGKiswI0iyHV/Vy8IeCUwfrGuNqxU4lusRP/QtlRcPHssJgsDW9sjnHNO0woec0tR9ikMe45Jxp/QTxfYg8B2vlyemZYeO43u2FkYQcp7tW7alILCeKQ6BnYHNtUyxfR+EmK6lZYpnhS74xRllFoQBt49vWbBas2VgQVpbQWBpfnFBcBj65g7NwDOQSoFFiKV52bUh6cALoalmsYtNSCzb7tJlKnoZSn3JT94hkk1Hcqnpd3WuTeniawOgh02wEwk8V/MrAacSJ4AI1kErgBR0IQPGotrBNqQtsWwAKZWXabthaLuOHyop4gfYJ54NgkH0GDcIOJvYEEKQpio3ADJiO15gq8xC7PgWbAbwd1yzELDDg9zW4o0HM3g0CHQU8HmOe0Ewmm26S74KAjoYDEgcjkOqtq4LCG+6ZkA8Td3QDT0f0NIbR3/YOT/LQt2/HH4s24TY1azo265rQ/R74+dbAAcDkKoICLzA6qBqCAu1WHFNz7JgF/a4d8Ebrg9HHNFixQ9CO7ThWAp9LW19oPPBUc4DTvmKCOhhy/fgnLGJqW0fotgkjgtbtfVcgR0GxPfHgcbBDrgMIjxUojvAcGIEjsMPcy1VHPAWnBeeswtPwT4QBnB6aI7xPc+zga2rKeb4AKcOYOY4M3CNDzUAAKel7bMuWAJIfx1QQ8hf+BV690PgAMqmw1RqABDuLYsUfJkvUL4pUVQU9RjBaI+mdX29h26jKl5BPbxKKsbve7wAl5MoYajgL0fmcMGqV1oYtT2MAmQhoyXN6koX0U/RukihsI2yeb0E/rRL2s3IxfXfg5VLWnUreSe9vr1O0byMbo+FOKOm4t31NKVCjo1VAmTwhyYY+QgbCGqERBjFKY1KppE3s9cYpXleMASleNlahmuu048ujrm9YK5a0TWCOj1K+Xai/vUNsRxIogqJh7p9D6wYv5fFq4bZHHZIF3A5YcAhlrdGVMhroyJSNLqZsAGbDDVzMLql0i3AS/PkBD2VMzyYwNCGeGotr9pNbg5uOI2P+SLue330qHF8s5bfhjklWKO+o4JQA0yy00fXUtYyuaFZX2Fgwke69lrz2rCziPAvuEFPKN2DhiQr+jNCjWsw60fWvXo5h1vXWS3ySEvS4XsYqelUXtz01yxNSB02wTX6kkSsVKY0g4sGz8ispplCVsMDB/RKuWyz7qoM2z3nfkh4rK14nMGlFWKN3vDbabZEP20o/sDQoszXPD8+8jEI4Np5dTdJ8OENpG9qjX7RDNpdeoeT9TsVeePVegNDUmUrKjq0kaHauXauvtuDvpKNL377DX0jf+M61RWFtS7VVu0s9DZRcxxYWC7UuCvNvLGWvvY27fO27a/0UeQQFfWPluDk+gNEARao0faX1E7KXhMozZt+2Sul1QzJPlQbS2MtIHXLogWk7FZ+HmCooay9dus1NqQnRxw1ljAD09U9aNV63K8aMGj9yT5k1DjUax/yl61f7GGtDhKWaZVc6xqucduk6gvU1qfNywyTZ2ZtID2L5fcb7QZlxnSoIUkVZOf7n6IsWtP/AvNyXruBqw8Xo0Q7hG0bEY/AxR263KfCYyTXpuCG5oLgH2V0cGTtRnhO39xqd3UzlUjo9tI7dO5vH+rII75e6D7UaL5m0KkaRdVvEXYlBwYomjd5sMYDCDsUG62fJp0gvo8RMVrESR0X6d15PkaMe21jrcWfLxaMg542iNCKRnM42mbSElg+T+sD8uTlCa/D4k0KKCeOvg3jByGvM8QnJ7IUoo0l5Xqoc1gFP3VhRQLV3gyZU324jLIl1V5EyddRmzISb2dNTLS5rdq5n/STTguyGkuOTocIpgGGli6GVkL1IORPSOR4F7HIoCGJlnrK3nnJoJduhoqAk0fghiaoV430WO8uSobm79clO5BIoPzAPHXm6iXpoV5wXJhX+stbpsnsLYZqSB9pzqNniDxOmx3n5ljc9a+83QkUP6Y42l0bxaI4GiGIe05VkfiPKaLi0SrqDyqiurNUrQj6tlUTVFbkR6f31uT69ANVuU7cr+Z2KfM7ouNPq6uUtlh3Rfssbv56K1rtwFwhZR1cwqujiFH5HcCuC+Pgfsg5wGn0PTi7vIlSI2J3WYyUr0saxnjs1sMJotsoqdCcptGdQUzTnGirlCqwDwsQzYukZKLDsfdoXvoB8QDWqGL0ltDXEkiu6iytvyn+vRJHBKlsG2bbKXIFB/6f8Fz74a2Q0Bzc+ne2PEIAXtYJo4ZRUpanN/ztBuXoo0Y3BMZ5sqb5puqoWo7KF6JPpk+eseoupc/Fur8msG/At02ZGhjvV3Rd8LqF7V9v4o8UcIbVL8+f7TeLns2TG5TMDy+f9L4Kv3yC6g4IzCygbubfsV0+ef5sH+i1ZWleFzsnN7SEbXOSFXn+WgxijJ/tw+NwVZXn6TXkYcfx3w

It's a bit annoying that it takes a couple minutes, and that it's such a large slice of code, so we'll continue whittling away at it.

BTW the tileset used in that Sandcastle is property of my employer, please don't use it for any other purpose than researching this bug.

@hpinkos
Copy link
Contributor

hpinkos commented Oct 31, 2019

Thanks for taking the time to put this together @tinco
I'll try to make some time next week to look into this issue.

@tinco
Copy link
Contributor Author

tinco commented Oct 31, 2019

I just noticed that it reliably happens when I switch away from the tab for a couple minutes and then switch back. I have no idea what that means. I guess maybe chrome buffers the intervals and then runs a bunch in quick sequence and that makes it more likely for the bug to happen?

@OmarShehata
Copy link
Contributor

Thanks @tinco . I am able to reproduce this. Were you able to get the same issue using any of the tilesets already on Cesium ion (like this photogrammetry one https://sandcastle.cesium.com/index.html?src=3D%20Tiles%20Photogrammetry.html) ? That way you won't have to have a public link to your dataset if you can't share it.

@tinco
Copy link
Contributor Author

tinco commented Nov 5, 2019

No worries @OmarShehata, the data is not really secret or anything. Good to hear you were able to reproduce, did you also trigger it by switching tabs? In our real app there was some sequence of actions that would reliably trigger the exception without switching tabs. I think I'll spend some time this week or the week after improving the sandcastle or maybe digging through the source to find the source of the race condition.

@OmarShehata
Copy link
Contributor

@tinco yes I was able to trigger it by switching tabs (didn't try leaving it long enough without switching).

I have run into a similar behavior where a CesiumJS app will crash if the tab is switched away for too long (although only on Linux, and I haven't been able to consistently reproduce), so creating and digging into a reproducible example here is definitely helpful!

@hpinkos
Copy link
Contributor

hpinkos commented Nov 14, 2019

@tinco sorry for the delay on this. We've been able to reproduce the issue but haven't tracked down exactly what is causing the error yet. We'll keep you posted when we have updates. Thanks

@mramato
Copy link
Contributor

mramato commented Nov 18, 2019

@tinco I'm pretty positive that requestRenderMode combined with viewer.flyTo is the primary culprit here but haven't been able to track down exactly how. If I change your example to the below, the problem goes away:

setInterval(async () => {
  testCase.drawElement(element)
  viewer.scene.requestRender();
  await delay(1000)
  testCase.selectPart(element.parts[0], 'first part')
  viewer.scene.requestRender();
  await delay(1000)
  testCase.selectComponentPart(element.parts[0].componentParts[0], 'first component part')
  viewer.scene.requestRender();
  await delay(1000)
  testCase.selectComponentPart(element.parts[0].componentParts[1], 'second component part')
  viewer.scene.requestRender();
  await delay(1000)
  testCase.selectPart(element.parts[1], 'second part')
  viewer.scene.requestRender();
}, 5000)

In fact, it seems only a single requestRender call may actually be needed. The below change fixes your demo (as far as I cantell).

    setInterval(async () => {
      testCase.drawElement(element)
      viewer.scene.requestRender();  
      await delay(1000)
      testCase.selectPart(element.parts[0], 'first part')
      await delay(1000)
      testCase.selectComponentPart(element.parts[0].componentParts[0], 'first component part')
      await delay(1000)
      testCase.selectComponentPart(element.parts[0].componentParts[1], 'second component part')
      await delay(1000)
      testCase.selectPart(element.parts[1], 'second part')
    }, 5000)

The intent of the above change is to continue to use requestRenderMode, but also inform Cesium after the scene changes so that it will render a single frame and performany bookeeping that the Entity API needs to do.

The general idea of requestRender mode is that you call scene.requestRender whenever you change anything about the scene manually. Whileit works without calls to scene.requestRender most of the time, in this case the use of flyTo is triggering some code that I think is out of sync with the scene. I'm going to continue to look into this and see if there's something we can do at the Cesium level that makes sense to fix it, but I recommend you add a scene.requrestRender call anytime to change the scene anyway.

Can you test this change to see if the problem goes away? If so, can you then test that change in your real application and report back?

Thanks.

@tinco
Copy link
Contributor Author

tinco commented Jan 10, 2020

Thank you @mramato, so your fix is to reduce the amount of request renders, so the scene has some time to naturally go through the rendering? I get that might fix the problem in the test demo, but I don't think that's a feasible fix for us. I make the calls to requestRender because I can't be sure there is other movement going on that will eventually trigger the render. Sometimes the camera doesn't have the move, and cesium is done loading the highest quality, and all the selectComponentPart does is update a color or something (through a callbackproperty), and it won't show up unless we do the requestRender.

I get the code paths intermingling here are pretty complex, but why is calling requestRender different than just having cesium continuously render? If flyTo was able to go out of sync with render calls, would that problem not be even bigger if requestRenderMode was off?

By the way, I also get this error when flyTo is not active, I think it's purely an interaction between viewer.entities.remove, callbackProperty and scene.requestRender.

@OmarShehata
Copy link
Contributor

Here's a reduced test case I got over email. I'm able to pretty consistently reproduce this now, crashes almost immediately when switching out to another tab and back.

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.

4 participants