CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.93k stars 3.49k forks source link

Billboards are replaced with black squares on mobile (android/iOS) #6477

Open svenvarennes opened 6 years ago

svenvarennes commented 6 years ago

When adding image billboards, they render correctly on my desktop but when opening the same page on my Android S6 cellphone as well as my iPhone 5SE image billboards do not render correctly and look like this:

bildschirmfoto_2018-04-20_um_15 33 04

I've searched for a solution but could not find any.

Is there a way to check on a HW basis if image billboards are supported?

hpinkos commented 6 years ago

@svenvarennes I haven't seen this before. Can you make a Sandcastle Example to reproduce this?

vividos commented 6 years ago

I see the same issue in my project, usually when adding many entities (> 1000) with a pin image from PinBuilder. Could be a starting point for you to investigate.

hpinkos commented 6 years ago

Thanks @vividos, could you possibly put together a Sandcaslte Example to reproduce this? That will make it easier for us to debug.

mramato commented 6 years ago

This sounds like the GPU running out of memory and returning black textures. (memory is much more limited on mobile). Does the app have any other data loaded (Such as multiple imagery layers?)

Can you post the code you are using to create the pin?

vividos commented 6 years ago

I checked out what the WebView logs, and the following messages are logged to the JavaScript console:

[INFO:CONSOLE(477)] "WebGL: INVALID_VALUE: texImage2D: width or height out of range", source: file:///android_asset/map/Cesium/Cesium.js (477)
[ERROR:gles2_cmd_decoder.cc(15053)] [.Offscreen-For-WebGL-0xdeec2300]GL ERROR :GL_INVALID_VALUE : glCopyTexSubImage2D: bad dimensions.
[ERROR:texture_manager.cc(2877)] [.Offscreen-For-WebGL-0xdeec2300]GL ERROR :GL_INVALID_OPERATION : glTexSubImage2D: level 0 does not exist

The last line is repeated many times, the first two lines only appear once.

I tried to come up with a Sandcastle example, but it seems the bug isn't triggered on mobile so far. I used the following code, maybe @svenvarennes can check out if it works or breaks in his project? https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/#c=dVLBTuMwEP2VUS5JtpVTtGiFllBBCreirKAgrZTLJHbLCMeubKeIXfHv2EmBNqudgzMzfs/vjeMdGtiReBEGLkCJF1gIS13LHvteEjd9udDKISlh4vS8UpXaedaWVNGR5GPmr89+MoDvUfEGrZOCIecrrWWNpuic0yqJrziHVhsRjrPxFNadahz5nRT+Vgp8rLWBJAiS15md+08OJ7NZyCaTT1SIvSsPGyZiQjlyJGzQTQ6AIRS2An5CXEhUz1DLrrcQT49RW20p2PHI/XgLNM5nqL6ztdHttdgYIWxyxmYwgVO/foNbdE/M+Kl1m6RTOP3xv710pFaTlLVGw73cyG4IanETPH/dfG9hoaU2yYe9ULC78vfVslg+3Hj1s5Q5fY0OH+6WyVixvzZhHDUoS0MbOpj08ajPinK1Km+P6W9f5Vv42ften0fTKLfuVYr

vividos commented 6 years ago

Here's the call stack with an unminified Cesium.js, version 1.45:

"Error loading image for billboard: DeveloperError: Width must be less than or equal to the maximum texture size (4096).  Check maximumTextureSize.
Error
    at new DeveloperError (file:///android_asset/map/Cesium/Cesium.js:540:19)
    at new Texture (file:///android_asset/map/Cesium/Cesium.js:88689:19)
    at resizeAtlas (file:///android_asset/map/Cesium/Cesium.js:117342:30)
    at addImage (file:///android_asset/map/Cesium/Cesium.js:117462:13)
    at file:///android_asset/map/Cesium/Cesium.js:117515:13
    at Promise.then (file:///android_asset/map/Cesium/Cesium.js:7663:34)
    at when (file:///android_asset/map/Cesium/Cesium.js:7526:34)
    at TextureAtlas.addImage (file:///android_asset/map/Cesium/Cesium.js:117508:24)
    at Billboard._loadImage (file:///android_asset/map/Cesium/Cesium.js:115687:39)
    at Billboard.setImage (file:///android_asset/map/Cesium/Cesium.js:115774:18)", source: file:///android_asset/map/Cesium/Cesium.js (115717)

When using the unminified source, the pins don't appear at all. The JavaScript code I'm using to add the images is like this:

for (...) {
   var pinImage = 'images/map-marker.svg';
   var url = Cesium.buildModuleUrl(pinImage);
   var entity = {
        name: name,
        description: description,
        position: Cesium.Cartesian3.fromDegrees(longitude, latitude),
        billboard: {
            image: pinBuilder.fromUrl(url, pinColor, 48),
            verticalOrigin: Cesium.VerticalOrigin.BOTTOM,
            heightReference: Cesium.HeightReference.CLAMP_TO_GROUND
        }
    };

   viewer.entities.add(entity);
}

The actual code in its context can be found here, in the addLocationList() function: https://github.com/vividos/WhereToFly/blob/master/src/App/Android/Assets/map/mapView3D.js

hpinkos commented 6 years ago

Thanks for the extra details @vividos! We'll try to look into this soon. Or if you have time to find a fix, we're always happy to accept a pull request

emackey commented 6 years ago

DeveloperError: Width must be less than or equal to the maximum texture size (4096). Check maximumTextureSize.

How big is that images/map-marker.svg you're using? (In pixels, or SVG canvas pixels, not file size)

vividos commented 6 years ago

I think I found the reason: One of my SVG images has the following attributes:

x="0px" y="0px" viewBox="0 0 1000 1000" enable-background="new 0 0 1000 1000" (it's this one here: https://github.com/vividos/WhereToFly/blob/master/src/App/Android/Assets/map/images/paragliding.svg)

I guess that using this image many times exhausts the resources of WebGL at one point.

vividos commented 6 years ago

I tried to update my example with the image URL here, unfortunately the image can't be downloaded from github.com. Link:

https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/#c=dVNRb9owEP4rFi8JA9lU68MEaTXoNmlSEdNKO03ixYmPcKpjR7YTxKr+950DZUC1e0jsu/t8330+t9KxFmELjt0wA1t2Bx6bij91vjQpuu2dNUGiAZf0JyuzMi2hajSzBrW6RP44+tN98oM0qpA+aOBSqaW1Opdu1oRgTZpMlWKVdRCP88mQrRtTBKRIn72sDCNbW8fSWBCpzmhCv4xdjUZxNRgcs6IdWH2vZAmUm2xCqP1YCCe3vMSwafLGgyuoFzCBF7YSLbaorBe/NuBgab/pnaiIKTjhXSGmdS2mRjmLSky9h+ApWguM53tRSydLjQpNyX1bJpNzIo3TxOGgSR4FmVvVaHh0On0j2Z+8I0+Y/XVw4ogBwUfR0pMuoxlZARuzZKaleWa5bjr9kuF5Vm09Ri0p88DjTrpAK2k+8rWz1RcoHYBPP/ERG7Br+n5gcxk23NGV2SrtD9n1f2P9i2o5ap1b6RSVu6AbrVNtfDI1HYOoBik1PBK02jr+c/F7ej+7f/wa61/W6cQCF7CQeuGwxJP+ns78fLZYLhfzc/jrv+3rm/7k69a9YS/zYafhdu//jFVtXYh3mXIuAlS1liSgyJviGWiEvI+wTLyBMoUtQ3Wz6l28m1WPFVp6T5F1o/UD/oFV7zYTlH8G01bGgVpQg1ruYsrm6vZ+7+ScZ4K271Fh/6ZOTvwL

vividos commented 6 years ago

OK I think I found out what is happening on Android devices. The SVG image size was a red herring. Here are the steps:

  1. The lines

    var pinImage = 'images/map-marker.svg';
    var url = Cesium.buildModuleUrl(pinImage);
    this.pinBuilder.fromUrl(url, Cesium.Color.ROYALBLUE, 48)

    create a HTML canvas element. PinBuilder has a cache, so when I call fromUrl() multiple times with the same parameters, the same canvas element is returned.

  2. The generated image is set as image property of an entity with a Billboard:

    var pin = viewer.entities.add({
            name : ...
            position : ...
            billboard : {
                image: pinBuilder.fromUrl(url, Cesium.Color.ROYALBLUE, 48)
            }
        });
  3. Now the setter for the image property (https://github.com/AnalyticalGraphicsInc/cesium/blob/1.45/Source/Scene/Billboard.js#L860) finally calls this.setImage(createGuid(), value); since the object is not of type Resource and has no url attribute (and doesn't fulfill some more cases). This results in the 48x48 image being set using setImage(), which in turn adds it to the internal texture atlas.

  4. The texture atlas uses the ID that was passed to setImage() to determine if it has to add the image to the texture atlas, or if it is already contained. When not, it adds the image to the texture atlas. When needed, the texture atlas texture is resized. Code is in https://github.com/AnalyticalGraphicsInc/cesium/blob/ee522d705cf00f2f00cf51906488c05c9ae2ffc9/Source/Scene/TextureAtlas.js#L167

  5. On mobile devices the maximum texture size is at 4096x4096, which in my case is reached or surpassed in my case after adding about 3600 images of size 48x48. On my PC with Nvidia graphics card the limit is much higher, and I couldn't reach the limit with my tests.

-- So the fix would be to not add the canvas images when they are already done. I'm no expert in CesiumJS, so I can't provide a PR, but maybe a GUID can be generated in PinBuilder for every unique image, and the GUID is set in the HTML canvas element somewhere. The Billboard object then could recognize the GUID and not re-adding all the images, but reusing them via the TextureAtlas. The fix for this could yield a speed gain for people that make heavy use of pins and use the PinBuilder to create them.

shunter commented 6 years ago

The issue is that given a Canvas object, there's no way to tell if it's changed or not, without comparing every pixel.

The easy workaround here is to call toDataURL() on the canvas and use the result as the image of the billboard. The data URL string is then used as the unique key into the TextureAtlas.

You can see this is already the approach demonstrated by the Sandcastle examples which use PinBuilder:

https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/?src=Map%20Pins.html&label=All

vividos commented 6 years ago

Thank you for the explanation. I wondered what all these Cesium.when() and toDataURL() calls are supposed to do. I fixed the bug with this commit: https://github.com/vividos/WhereToFly/commit/e5f341523b3353630d8181f0bc505bec511fc565

@svenvarennes, as you originally opened this issue, does this fix your problem as well?

kobbe commented 6 years ago

To help others with the same problem. I was using a stylefunction that did not cache the image but created a new ol.style.Icon each time. This resulted in the error 'Error loading image for billboard: DeveloperError: Width must be less than or equal to the maximum texture size (4096). Check maximumTextureSize.'. The solution for me was to reuse the same ol.style.Icon.

thw0rted commented 5 years ago

I had a similar requirement and solved it a different way:

const cache = {};
const key = "my pinbuilder arguments concatenated into a single string";
const pin = cache[key] || pinBuilder.fromText(...);
cache[key] = pin;
billboard.setImage(key, pin);

If I read the relevant code correctly, by giving the pin an "ID" that will always be unique to the image I want to generate, the first time I call setImage the TextureAtlas will create a new image in the sprite sheet, but subsequent calls will just return the image index based on the ID I supplied.

I think this is working without leaking memory or killing performance -- at least, no worse than the existing performance hit from turning on clustering. I do have possibly-unrelated issues where I get the "maximum texture size" error (which is how I found this ticket), so I'm not promising that the above solution is actually the right way to do it.

TannerBragg commented 4 years ago

@vividos @hpinkos We're using a back end node.js service to generate symbols and provide the map application datauris in CZML; however, this same effect could easily be done in the map application on the front end.

We can quickly reproduce the black squares utilizing an icon (billboard) generating library much like the pin builder in a matter of a few minutes. You can see the icon generator here. You can test yourself using this library and randomly generating a new symbol for the same entity several thousand times. I would suspect you should see the occurrence by 20,000 updates or so.

I think you need to elevate this bug to a severe status as it makes Cesium difficult to use while representing data annotations in the image itself.

We highly desire the capability to have "dynamic" billboard images. :)

vividos commented 4 years ago

@TannerBragg I don't know how I can help you here, sorry. My fix was to use the PinBuilder in order to not reach the texture size limit.

TannerBragg commented 4 years ago

@vividos My issue is a direct result of a bug in the texture atlas. So is the original one here. I suppose you just worked around the issue by making code changes in PinBuilder?

vividos commented 4 years ago

No, I just used the PinBuilder's cache to fix this. I guess the Cesium devs have to look at this.

TannerBragg commented 4 years ago

Adding more information. Issue from the community page also describes the issue accurately.

https://community.cesium.com/t/billboards-in-billboardcollection-turn-black/6805

OmarShehata commented 4 years ago

@TannerBragg you can only trigger this issue with 20,000 or more billboards, right? Have you tried pooling the number of billboards in your scene, so you can remove/re-use some when you need to add new ones? I imagine there might be other issues with having this many billboards in the scene (either performance-wise or it making the scene visually difficult to decipher). If you have a thread open in the community forum I'm happy to take a look and suggest workarounds or help isolate the underlying issue here more with more minimal test cases that help us reproduce it.

thw0rted commented 4 years ago

Omar, I think the issue is that it's sort of similar to a memory leak. If I remember correctly from a year ago or more when I investigated, the problem is that the "texture atlas" is adding each billboard texture to one big "sheet", trying to do some clever math to re-use parts of it, but once it runs out of scratch space, you get this error. This can happen even if you only have a relatively small number of (frequently-changing) billboards at once.

Like a memory leak, you can get away with just slowing down enough that you probably won't run out, but (again, if I remember correctly) the fundamental problem is that the atlas needs to throw away the whole texture and start over at some point. I believe that throwing away the billboard collection and starting a new one would accomplish that, but I haven't tried it.

TannerBragg commented 4 years ago

@thw0rted is correct. We have quite a few "movers" on the map. Each time one of the movers updates (generally once per second) the icon is going to change to show new metadata annotations. It's just a part of how the MIL-2525 Symbol Standard is going to want to convey information visually. The texture atlas should be able to support updating the image of the billboard frequently.

Clearing the entity collection isn't really an option for us as the data is "live."

Do you need help trying to reproduce the issue? Or does my comment above suffice?

TannerBragg commented 4 years ago

@OmarShehata I should also add it's purely related on the number of updates to the billboard image, not the total number of entities. The same effect could be achieved with a single or a few entities with unique images per update many times.

thw0rted commented 4 years ago

@TannerBragg I'm also working with 2525 symbology but we're not currently using all the annotations that are available so they don't change much over time. Have you considered using a combination of billboards for the more stable information (unit type, etc) and Cesium labels for dynamic information like speed / heading / etc? I spent a little time working on this but scrapped it because our users didn't need that much detail.

TannerBragg commented 4 years ago

@thw0rted Yes, I reduced the amount of information that was changing and rounded headings off to 5 degrees for the direction the track is moving. That seems to help, but I would like to have the option to show more information such as identifier, etc.

thw0rted commented 4 years ago

Have you looked at entity parent/child relationships? The way I understand it, you can make a new Entity and set its parent, then if e.g. you don't define a position, it will use the position of its parent. In this way, you can apply more than one LabelGraphics to a logical "thing". I think you could use this construct to render, say, the heading and speed as Labels but the Identifier (which shouldn't change, right?) using your symbology library. It might take some tweaking to get it to look good but I suspect performance would be better overall.

TannerBragg commented 4 years ago

In all honesty, it would be nice if we could set some billboards to have volatile images so they're just allocated in memory instead of in the TextureAtlas. Let garbage collector do it's thing when they are dereferenced.

I wonder how difficult this would be to implement? Just utilize a dictionary to store the volatile images rather than the atlas? Add an option to the Image property to set volatility?

thw0rted commented 4 years ago

I'm not a WebGL expert but my understanding was that it's more performant to have one big texture ("sprite sheet") allocated on the 3D side, that you draw additional billboards on -- I think the 3D side has its own memory allocations, sort of like talking between the main page and a service worker. The team could definitely comment on this more intelligently than I can but I suspect they'll tell you that "volatile" billboards would have a big performance impact.

TannerBragg commented 4 years ago

@thw0rted As you can clearly see, I'm well out of my comfort zone with this type of code. Would be nice if we could ping someone from Cesium to take a look into this. I saw @mramato commented on #172 regarding some TextureAtlas optimizations quite a few years ago. Maybe he can help out here?

Maarondesigns commented 3 years ago

I have had this issue before with black squares on desktop as well as mobile, but mobile it would happen more consistently. I am not sure if it is relevant to the issues discussed in this thread but I found that it was being caused by creating a billboard from an html canvas that had an odd pixel value for fontSize. Changing to even made the black squares issue go away, although I still see it every now and again, but it's rare. I think it is still happening because of IOS gpu memory running out or something.

Not sure if it is related but I am currently experiencing a different WebGL error: image This occurs when I change the text of my billboards which are html canvas elements. It does not actually stop anything from working or rendering as far as I can tell. The billboards still update as expected, but it shows the warning in the dev console. I am changing billboard like this:

let len = billboardCollection.length;
for (let i = 0; i < len; ++i) {
  let b = billboardCollection.get(i);
  b.image = functionThatGeneratesBillboard(newAttributes);
}

functionThatGeneratesBillboard(attributes){
   let canvas = document.createElement("canvas");
   let ctx = canvas.getContext("2d");

   let dpr = 1; //window.devicePixelRatio || 1; 
   ctx.scale(dpr, dpr);
/* I have experienced odd behavior on some devices setting this. 
* It can cause the black square issue to happen more often. 
* Not sure if it has to do with devices with pixelRatio's of 1.5 or something like that. 
* I just leave it at 1 for the least amount of bugs, 
* however this makes the billboards look fuzzy on IOS or devices with 
* high pixel ratio values. I think new iPhones are 3.
*/

   // set ctx attributes

  return canvas;
}
Nadav42 commented 3 years ago

@thw0rted @TannerBragg using your suggestions I made a temporary fix that fits my use case, I had the same problem where the atlas texture fills with dynamic billboard images but it only happens for me after a minute or so of "live update"

my soultion was to create a seperate BillboardCollection for all dynamic images and clean it's texture atlas before each "live" update

create the collection and billboards:

billBoardCollection = new Cesium.BillboardCollection();
....
this.billboard = billBoardCollection.add({
  position: Cesium.Cartesian3.fromDegrees(...),
  image: null,
  scaleByDistance: ...,
});

I clean the texture atlas before each "live" update:

const textureAtlas = (billBoardCollection as any).textureAtlas;
// billBoardCollection.debugShowTextureAtlas = true; // if you want to see the texture

if (textureAtlas && textureAtlas._context) {
    const context = textureAtlas._context;

    // clear saved cache on each billboard, also fix still referencing old atlas texture image
    (billBoardCollection as any)._billboards.forEach((billboard: Cesium.Billboard) => {
        billboard.image = null;
    });

    // create new atlas texture, all billboard images in this collection are dynamic and change between updates so saving it is useless anyway
    (this.cesiumObjectService.billBoardCollection as any).textureAtlas = new (Cesium as any).TextureAtlas({ context: context });
}

then I update billboard images each update like so

(this.billboard as any).image = generateDynamicImage(data, etc...) // returns html canvas

CustomDataSource is also an option, add all billboards into a data source and access it's atlas like this:

const dataSource = (billboardsDataSource as any);

if (!dataSource || !dataSource._entityCluster || !dataSource._entityCluster._billboardCollection) {
    return;
}

const billBoardCollection = dataSource._entityCluster._billboardCollection; // billBoardCollection.debugShowTextureAtlas = true;
const textureAtlas = (billBoardCollection as any).textureAtlas;
... // clean atlas like previously

I think this should work okay when you are absolutely sure you won't reach the texture atlas limit in a single update, what do you guys think?

edit september 2019, this is a better way to clean the atlas:

const billBoardCollection = dataSource._entityCluster._billboardCollection;

const textureAtlas = (billBoardCollection as any).textureAtlas;

if (textureAtlas && textureAtlas._context) {
        // clear saved cache on each billboard, also fix still referencing old atlas texture image
        for (let index = 0; index < billBoardCollection.length; index++) {
            const billboard = billBoardCollection.get(index);
            if (billboard) {
                billboard.image = null;
            }
        }

        textureAtlas._textureCoordinates = []; // _textureCoordinates saves for each billboard it's coordinates in the atlas texture
        textureAtlas._idHash = {}; // saves texture atlas promises results by promise id

        // keep current WebGL texture object but make atlas think it's empty
        textureAtlas._root.childNode1 = undefined;
        textureAtlas._root.childNode2 = undefined;
        textureAtlas._root.imageIndex = undefined;

        // clear the texture atlas with empty texture (emptyCanvas is a cached canvas same size as the texture atlas filled with opacity zero rectangle)
        textureAtlas._texture.copyFrom({
            source: emptyCanvas,
            xOffset: textureAtlas._root.bottomLeft.x,
            yOffset: textureAtlas._root.bottomLeft.y
        })

        // mark atlas as changed
        textureAtlas._guid = createGuid();
    }
TannerBragg commented 3 years ago

@Nadav42 Does this cause a flicker of the billboard on the redraw for loading new images from a new atlas?

Nadav42 commented 3 years ago

illboard on the redraw for loading new images from a new atla

@TannerBragg it does not (just like loading a new image from the same atlas doesn't cause flickering no?)

TannerBragg commented 3 years ago

illboard on the redraw for loading new images from a new atla

@TannerBragg it does not (just like loading a new image from the same atlas doesn't cause flickering no?)

I've observed a flicker of the image used for a billboard when loading an updated czml... Perhaps we're doing something improper with the loading of czml updates.

We utilize a czml data source to load updated czml data. Almost tempted to write a custom data source at this point.

Nadav42 commented 3 years ago

regarding the flicker, I noticed a flicker happens when loading a new atlas / reseting an existing one only when the switching happen before cesium render event

@TannerBragg it seems to work without flicker when I make sure to switch TextureAtlas on the next Cesium.viewer.scene.postRender Event

for example using the pre render event causes noticeable flicker