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

rawToRgb is not a function #120

Open
m-mohr opened this issue Mar 8, 2023 · 9 comments
Open

rawToRgb is not a function #120

m-mohr opened this issue Mar 8, 2023 · 9 comments

Comments

@m-mohr
Copy link
Contributor

m-mohr commented Mar 8, 2023

Describe the bug
I'm trying to visualize the eu_pasture.tiff from the test-data repo in stac-layer.
It results in an error message:

TypeError: r.rawToRgb is not a function

This only happens if I set the mins/maxs/ranges.

Expected behavior
No error ;-)

@DanielJDufour
Copy link
Member

DanielJDufour commented Mar 8, 2023

rawToRgb comes from pixel-utils

I think it might be a conflict between what import system (CJS/TS/ESM) pixel-utils, georaster-layer-for-leaflet and stac-layer uses.

But definitely fixable. I'll look into it.

@m-mohr
Copy link
Contributor Author

m-mohr commented Mar 8, 2023

As it only occurs when I pass in mins/maxs/ranges, I suspect that the import is correct, but either I set the mins/maxs/ranges incorrectly or it doesn't expect that.

Does this look correct?

import parseGeoRaster from "georaster";
import GeoRasterLayer from "georaster-layer-for-leaflet";
import get_epsg_code from "geotiff-epsg-code";
import withTimeout, { TIMEOUT } from "./with-timeout.js";

export default function createGeoRasterLayer(asset, options) {
  return withTimeout(TIMEOUT, async () => {
    const georaster = await parseGeoRaster(asset.getAbsoluteUrl());

    // Handle no-data values
    let noDataValues = asset.getNoDataValues();
    if (noDataValues.length > 0) {
      georaster.noDataValue = noDataValues[0];
    }

    if ([undefined, null, "", 32767].includes(georaster.projection) && georaster._geotiff) {
      georaster.projection = await get_epsg_code(georaster._geotiff);
    }

    const layer = new GeoRasterLayer({ georaster, ...options });

    let mins = [];
    let maxs = [];
    let ranges = [];
    for (let i = 0; i < georaster.numberOfRasters; i++) {
      let { minimum, maximum } = asset.getMinMaxValues(i);
      mins.push(minimum);
      maxs.push(maximum);
      ranges.push(maximum - minimum);
    }
    if (mins.every(min => min !== null) && maxs.every(max => max !== null)) {
      layer.currentStats = { mins, maxs, ranges };
      layer.calcStats = false;
    } else if (Array.isArray(options.bands) && options.bands.length >= 1 && options.bands.length <= 4) {
      // hack to force GeoRasterLayer to calculate statistics
      layer.calcStats = true;
    }

    return layer;
  });
}

@m-mohr
Copy link
Contributor Author

m-mohr commented Mar 8, 2023

Ah it seems setting calcStats to false disables setting this.rawToRgb...

Edit: Confirmed. Not sure how to proceed though...

@m-mohr
Copy link
Contributor Author

m-mohr commented Mar 8, 2023

See #121

@DanielJDufour
Copy link
Member

Would it be possible to set the mins, maxs and ranges on the georaster object and not the currentStats object? Technically, the mins, maxs, ranges are a part of the public georaster definition (https://github.com/geotiff/georaster#required-properties) whereas currentStats isn't technically public (so it doesn't have the same sort of commitment for support/backwards compatability across major version upgrades).

I think this might do it:

    georaster.mins = [];
    georaster.maxs = [];
    georaster.ranges = [];
    for (let i = 0; i < georaster.numberOfRasters; i++) {
      let { minimum, maximum } = asset.getMinMaxValues(i);
      georaster.mins.push(minimum);
      georaster.maxs.push(maximum);
      georaster.ranges.push(maximum - minimum);
    }

Let me know if you think this might work. Honestly, before STAC existed, I didn't envision statistics being available for geotiffs accessed through range requests. It's a definite oversight on my part and improvements to georaster and georaster-layer-for-leaflet are definitely required regardless of the approach we take here.

Open to your thoughts :-)

@m-mohr
Copy link
Contributor Author

m-mohr commented May 30, 2023

Thanks! I guess that should work, will check. I think I only used currentStats based on a proposal from you :-)

@DanielJDufour
Copy link
Member

oof. sorry about that.

DanielJDufour added a commit that referenced this issue Jun 23, 2023
@m-mohr
Copy link
Contributor Author

m-mohr commented Jul 11, 2023

I gave it a shot, but it seems that there's still an error:

TypeError: r is not a function
    Se convert-single-str.js:3
    pixelValuesToColorFn georaster-layer-for-leaflet.js:157
    getColor georaster-layer-for-leaflet.js:877
    e georaster-layer-for-leaflet.js:718
...

It looks like scale in the convert_raw_one_band_pixel_to_rgb_str function is sometimes not defined?!
I'm working with a single band image.

@m-mohr m-mohr closed this as completed Jul 11, 2023
@m-mohr m-mohr reopened this Jul 11, 2023
@m-mohr
Copy link
Contributor Author

m-mohr commented Jul 11, 2023

oh no! I just figured that I was working on 3.10.0. #121 was not released yet, right? @DanielJDufour
It it feasible to release 3.10.1? It looks like it's the last part that is missing for a stac-layer release based on stac-js.

I still need to assign the values to the layer though, the georaster object doesn't work for me.

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

No branches or pull requests

2 participants