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

ImageryProvider.loadImage bug about KTX/CRN image #7979

Closed
progsung opened this issue Jul 3, 2019 · 5 comments
Closed

ImageryProvider.loadImage bug about KTX/CRN image #7979

progsung opened this issue Jul 3, 2019 · 5 comments
Labels

Comments

@progsung
Copy link

progsung commented Jul 3, 2019

I think found a bug:
Recently I tested ktx and crn image Imagery using UrlTemplateImageryProvider.
I reviewed cesium source code and I thought it's no problem to service.
But at runtime, It's not working. So I review cesium source especially about Imagery.
And found one! at ImageryProvider.js
https://github.com/AnalyticalGraphicsInc/cesium/blob/32fc04d9e7e3b26e78ca1ca4b607091846a68c1e/Source/Scene/ImageryProvider.js#L336

    ImageryProvider.loadImage = function(imageryProvider, url) {

        var resource = Resource.createIfNeeded(url);

        if (ktxRegex.test(resource)) {  //=> it should be "resource.url"
            return loadKTX(resource);
        } else if (crnRegex.test(resource)) {    //=> it should be "resource.url"
            return loadCRN(resource);
        } else if (defined(imageryProvider.tileDiscardPolicy)) {
            return resource.fetchImage({
                preferBlob : true,
                preferImageBitmap : true,
                flipY : true
            });
        }

        return resource.fetchImage({
            preferImageBitmap : true,
            flipY : true
        });
    };

I hope it can help someone.
thanks.

@OmarShehata OmarShehata added category - terrain and imagery good first issue An opportunity for first time contributors type - bug labels Jul 3, 2019
@OmarShehata
Copy link
Contributor

Thanks for reporting this @progsung - it does look like a bug to me. Were you able to get it to load the KTX image by changing that line? If you want to open a pull request to fix this, that would be awesome! The contributing guide is a good place to start here: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/CONTRIBUTING.md#opening-a-pull-request

@ProjectBarks
Copy link
Contributor

@OmarShehata This seems to be working as intended.

KTX takes in a resource, url or buffer and then loads it through our Resource API.

function loadKTX(resourceOrUrlOrBuffer) {
        //>>includeStart('debug', pragmas.debug);
        Check.defined('resourceOrUrlOrBuffer', resourceOrUrlOrBuffer);
        //>>includeEnd('debug');

        var loadPromise;
        if (resourceOrUrlOrBuffer instanceof ArrayBuffer || ArrayBuffer.isView(resourceOrUrlOrBuffer)) {
            loadPromise = when.resolve(resourceOrUrlOrBuffer);
        } else {
            var resource = Resource.createIfNeeded(resourceOrUrlOrBuffer);
            loadPromise = resource.fetchArrayBuffer();
        }

        if (!defined(loadPromise)) {
            return undefined;
        }

        return loadPromise.then(function(data) {
            if (defined(data)) {
                return parseKTX(data);
            }
        });
    }

Same goes for LoadCRN

    function loadCRN(resourceOrUrlOrBuffer) {
        //>>includeStart('debug', pragmas.debug);
        if (!defined(resourceOrUrlOrBuffer)) {
            throw new DeveloperError('resourceOrUrlOrBuffer is required.');
        }
        //>>includeEnd('debug');

        var loadPromise;
        if (resourceOrUrlOrBuffer instanceof ArrayBuffer || ArrayBuffer.isView(resourceOrUrlOrBuffer)) {
            loadPromise = when.resolve(resourceOrUrlOrBuffer);
        } else {
            var resource = Resource.createIfNeeded(resourceOrUrlOrBuffer);
            loadPromise = resource.fetchArrayBuffer();
        }

        if (!defined(loadPromise)) {
            return undefined;
        }

        return loadPromise.then(function(data) {
            if (!defined(data)) {
                return;
            }
            var transferrableObjects = [];
            if (data instanceof ArrayBuffer) {
                transferrableObjects.push(data);
            } else if (data.byteOffset === 0 && data.byteLength === data.buffer.byteLength) {
                transferrableObjects.push(data.buffer);
            } else {
                // data is a view of an array buffer. need to copy so it is transferrable to web worker
                data = data.slice(0, data.length);
                transferrableObjects.push(data.buffer);
            }

            return transcodeTaskProcessor.scheduleTask(data, transferrableObjects);
        }).then(function(compressedTextureBuffer) {
            return CompressedTextureBuffer.clone(compressedTextureBuffer);
        });
    }

So while passing a URL should work it is no different than what we are doing currently. Let me know if I missed anything; otherwise, we can probably close this issue.

@mramato
Copy link
Contributor

mramato commented Aug 15, 2019

@ProjectBarks take another look at the issue original description and sample code. The issue here is that we are passing Resource objects to the ktxRegex and crnRegex regular expression to determine the file type, not that we are passing resources This is happening in both ImageryProvider and Material

@ProjectBarks
Copy link
Contributor

@mramato ahh totally missed it! Fix coming soon.

@OmarShehata
Copy link
Contributor

Fixed in #8064

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants