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

feat(geotiff): RGB GeoTIFFLoader #2839

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

giovannicimolin
Copy link
Collaborator

This is an experimental version of a GeoTIFF loader. Its goal is to add native RGB GeoTIFF rendering support for deck.gl to render aerial images, orthophotos and overlays into a deck instance.

Discussion on: #1280

TODO:

  • Retrieve bounds/geospatial data from GeoTiff and add them to loader response
  • Add tests
  • Add example to deck.gl (with sample GeoTiff file)
  • Split methods into proper files and folders
  • Write testing instructions

Testing instructions:

TBD

@ibgreen
Copy link
Collaborator

ibgreen commented Dec 12, 2023

@giovannicimolin I sent you an invite to the loaders.gl repository. That way you can push branches directly instead of working through a fork.

It would be nice if you could reopen this PR against a local branch, then I can push commits to your branch more easily. This can be quicker than writing a lot of feedback and waiting for you to make every small change.

Comment on lines +16 to +18
interface GeoTiffLoaderOptions extends LoaderOptions {
enableAlpha: boolean;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Follow pattern set by other loaders

Suggested change
interface GeoTiffLoaderOptions extends LoaderOptions {
enableAlpha: boolean;
}
export type GeoTiffLoaderOptions = LoaderOptions & {
geotiff?: {
enableAlpha: boolean;
}
}

@@ -0,0 +1,68 @@
import { fromArrayBuffer } from "geotiff";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: following pattern in other loader modules I would name this file geotiff-loader.ts

@ibgreen ibgreen marked this pull request as ready for review December 13, 2023 12:18
@ibgreen ibgreen changed the title RGB GeoTiff Loader [WIP] feat(geotiff): RGB GeoTIFFLoader Dec 13, 2023
@ibgreen ibgreen merged commit d8d3dc6 into visgl:master Dec 13, 2023
0 of 3 checks passed
@giovannicimolin
Copy link
Collaborator Author

@ibgreen That was fast! I didn't even had the time to fix the comments you made. 😅

I'll create a follow-up PR with fixing the comments and adding some items from the todo list too.

Thanks for the quick review!

@ibgreen
Copy link
Collaborator

ibgreen commented Dec 13, 2023

Didn't want this to go stale. I already put up a PR with fixes

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

Successfully merging this pull request may close these issues.

None yet

2 participants