Cog support cesium only 1

staffordsmith83 commented 5 months ago

What this PR does

Fixes #156

This PR implements COG support in Cesium Mode.

Whats been done

changes to zoom in Cesium.ts for COGs
changes to zoom in Leaflet. Previously the code tried to access east and west properties of Rectangle but this may be undefined with COGs in 2D mode, so this needs to be handled so zoomTo button doesnt error in 2D mode. Please help me check that this doesnt break anything.
- // Ensure extent is defined before accessing its properties if (isDefined(extent)) {
Added CogCatalogItem. This is greatly reduced in complexity since moving back to 3D only support.

Remaining task

There is a separate draft PR to implement a shortReport warning about non-native projection COGs: https://github.com/TerriaJS/terriajs/pull/7226

It is incomplete, but the strategy to finish it is in the description of the PR.

A note on Reprojecting on the Fly

We had intended to only support COGs prepared in 3857 and 4326 as reprojecting in the browser dynamically is not trivial and was poorly implemented. However the reprojection mechanism used by TIFFImageryProvider has recently been rewritten and is now good enough to be included. It is flagged as expermental, and there is a console warning of [Experimental] Reprojection EPSG:xxxxx . We should be happy that we have any reprojection on the fly.

Not all projections work. I have identified some for the Australian context that do not:

EPSG:9473 does not. This throws a warning, as it is not included in the proj4-fully-loaded library. This might be an easy fix with a PR to proj4-fully-loaded or a return to the async method of supplying projections to TIFFImageryProvider
EPSG:3577 does not. This is GDA94 Australian Albers, and is used for some Australian datasets that cover all Australia as provided by DEA. See the 'Unsupported' group of the test data in the testing sharelink. These datasets load without errors, but are not visible on the map.

This PR also relies on https://www.npmjs.com/package/terriajs-tiff-imagery-provider I have forked this from https://github.com/hongfaqiu/TIFFImageryProvider And made a new repo at https://github.com/TerriaJS/TIFFImageryProvider

Test me

http://ci.terria.io/cog-support-cesium-only-1/#clean&https://gist.githubusercontent.com/staffordsmith83/9a6ba12479100a810dadcf0119242913/raw/8c73e5053c4d26180784281840a9c235eface61f/cogs_three_groups.json

There are three groups of COGs in the catalog. The first group all should work. There are a variety of projections, and tests for feature picking disabled. Also, true color images and single band images.

The second group, none should work. There are some unsupported projections here, and some COG sources with unsupported photometric interpretations.

The third group is for visually testing that traits make it through.

credit should be shown in Basemaps link at bottom of map window:

A few things like tile size and cache time are difficult to test visually but appear to be getting passed. These could be added into the unit tests if you think necessary.

Pixel shifting problem

Here is a collection of coarse gridded scientific data. CMIP6 climate projections in EPSG:4326. I have added one to the testlink catalog gist.

https://nex-gddp-cmip6-cog.s3.us-west-2.amazonaws.com/index.html

Checklist

[x] There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
[ ] I've updated relevant documentation in doc/.
[x] I've updated CHANGES.md with what I changed.
[x] I've provided instructions in the PR description on how to test this PR.

staffordsmith83 commented 5 months ago

Yeah there are some funny issues there arent there unfortunately. It appears that the QGIS image looks identical to the "Nearest Pixel + Nearest Filter" example you provided, what do you think?

I am open to either disabling reprojection, or to add short reports. But I feel we should allow reprojection, and just put a short report that it is experimental. Certainly I can think of uses where I dont mind some edge effects but would be great to use terria to show a preview of a cog I have in a different projection. If I need the absolute best visualisation I can get the link for the resource and download it or whatever.

I suppose its up to people who assemble the catalogs - if you want it to be rock solid, reproject to 4326 (hopefully!) Shall I reproject some of those examples to 3857 to compare the same datasets perhaps?

Thanks for taking such a close look, this is awesome to have other eyes on it. More tomorrow!

staffordsmith83 commented 5 months ago

Just going to implement short report now. We can disable all reprojection later if decided.

staffordsmith83 commented 5 months ago

@nf-s Ill add some gridded climate data examples from here: https://registry.opendata.aws/nex-gddp-cmip6/.

UPDATE: Added one to the 'Supported' group in the test sharelink gist. It looks bad. Data should be square pixels:

It may be worth testing the COG Url in the TIFFImageryProvider online demo: https://tiff-imagery-provider.opendde.com/

staffordsmith83 commented 5 months ago

The dataset should look like this:

staffordsmith83 commented 5 months ago

Here is a separate draft PR to implement a shortReport warning about non-native projection COGs: https://github.com/TerriaJS/terriajs/pull/7226

It is incomplete, but the strategy to finish it is in the description of the PR.

staffordsmith83 commented 5 months ago

Regarding resampling error: Hongfaqiu is aware of the issue and is going to try to fix it next week. With a bit of luck after that a merge from upstream and COGs will be good to go! https://github.com/hongfaqiu/TIFFImageryProvider/issues/30#issuecomment-2193947286

imakihi commented 3 months ago

@staffordsmith83 Thank you for working on this! I checked if the value picking issue I described in https://github.com/TerriaJS/terriajs/pull/6833 was solved. It looks like the value picking on the 3D mode working correctly!!! Great!!

staffordsmith83 commented 3 months ago

Dear @imakihi thats great! Im no longer working on the Terria team, so Im not sure when they plan to merge this, @nf-s whats the plans with COGs? I could try to find some time over the summer to help get it done?

imakihi commented 3 months ago

Hi @staffordsmith83 I didn't know that you left the Terria team. Thank you for starting this great feature and worked on it. I think this COG capability benefits many Terria users. Thank you!

staffordsmith83 commented 3 months ago

Hi @staffordsmith83 I didn't know that you left the Terria team. Thank you for starting this great feature and worked on it. I think this COG capability benefits many Terria users. Thank you!

My pleasure Hiroo! Hope to meet you some day at a conference or something :-)

nf-s commented 2 months ago

Dear @imakihi thats great! Im no longer working on the Terria team, so Im not sure when they plan to merge this, @nf-s whats the plans with COGs? I could try to find some time over the summer to help get it done?

We are in a bit of a frenzy at the moment, and I'm about to be off for 2 weeks, but it would be awesome to get this merged in soon. Thanks for offering your time @staffordsmith83! That is very kind of you. I'll see if someone in the team has time to take a look first to save you the effort - I'll let you know if I can find someone to look at it over the next 2 weeks 🙂

imakihi commented 2 months ago

@nf-s Thank so much you. It would be super super cool if Terria users could use COG data.

staffordsmith83 commented 2 months ago

Thanks @nf-s yes let me know, Im crazy flat out but I would just love to see this get finished, lets make it happen together!

na9da commented 2 months ago

@staffordsmith83 - Thanks for offering to help. I'll get some time next week to have a look. I'll need to get familiar with COG stuff though. Will ping here if I have any questions :)

staffordsmith83 commented 2 months ago

@na9da OK Ill keep an eye on this ticket!

imakihi commented 2 months ago

@na9da @nf-s I was wondering if you had time to think about this COG support...

na9da commented 2 months ago

@imakihi - sorry i haven't had a chance to look at it yet. But hopefully soon.

CLAassistant commented 1 month ago

All committers have signed the CLA.

na9da commented 1 month ago

Hey @staffordsmith83 @nf-s - I have updated the PR with the following main changes:

Upgraded terriajs-tiff-imagery-provider to match latest upstream main
- linear mode has been replaced with a better bilinear mode which looks great, so I have made it the default. Here's a comparison of nearest (left) & bilinear (right) modes (CI test link)
- minimized custom changes so that merges from upstream becomes easier (turned of prettier etc)
- changes documented here
CogCatalogItem changes
- switched imagery provider loading to use the fromUrl API (the url option is deprecated in upstream).
- added short report when using non-standard projections

Pending: update test cases

na9da commented 1 month ago

@nf-s new test url - http://ci.terria.io/cog-support-cesium-only-1/#share=s-t5i1wQbevr0dGRXNqUgTfjMNHcl&clean&https://gist.githubusercontent.com/na9da/18ce40ab4d93a722932c3efe5c947139/raw/b06ed050745f57ba9b081e7700e6504ea6719a9b/cogs.json

nf-s commented 1 month ago

Also did a quick test to see if alignment is correct - and it looks to be good!

na9da commented 1 month ago

On the quality, i had a look - the particular opendrone dataset has 8 images in the TIFF pyramid. The imagery provider seems to be correctly picking and cycling through them as Cesium requests new zoom levels.

Not sure exactly whats causing the pixelized look. In 3D mode, there might be different active zoom levels (see screenshot below), which could be one reason.

However, I switched the map to 2d mode, which resulted in a single zoom level for all tiles, but the pixelization didn't improve much.

The other thought is, the COG has 8 images, but does that map linearly with zoom levels, i.e at zoom=5, is it correct to use the 5th image in the COG, or is there some other math to figure out which zoom level to use.

Also, not sure if there is any other stretching happening from adjusting for image shape (aspect ratio related).

The other possibility, is some processing happening in cesium that makes the images look less sharp. I tried changing the cesium imagery layer to use "NEAREST" sampling (as suggested here), which makes it slightly less blurry Edit: I see you already explored this in a previous comment

(left with NEAREST filter slightly sharper vs right with default linear)

I also compared it with openlayers, which looks super crisp. No idea how to get anywhere close to that.

(openlayers - no pixelation at all)

TLDR; not much clue where that pixelation is coming from.

staffordsmith83 commented 1 month ago

Great to see the upstream changes have improved this! Ill try to find some some for a closer look next week. Thanks @na9da !

nf-s commented 1 month ago

On the quality, i had a look - the particular opendrone dataset has 8 images in the TIFF pyramid. The imagery provider seems to be correctly picking and cycling through them as Cesium requests new zoom levels.

Not sure exactly whats causing the pixelized look. In 3D mode, there might be different active zoom levels (see screenshot below), which could be one reason.

However, I switched the map to 2d mode, which resulted in a single zoom level for all tiles, but the pixelization didn't improve much.

The other thought is, the COG has 8 images, but does that map linearly with zoom levels, i.e at zoom=5, is it correct to use the 5th image in the COG, or is there some other math to figure out which zoom level to use.

Also, not sure if there is any other stretching happening from adjusting for image shape (aspect ratio related).

The other possibility, is some processing happening in cesium that makes the images look less sharp. I tried changing the cesium imagery layer to use "NEAREST" sampling (as suggested here), which makes it slightly less blurry Edit: I see you already explored this in a previous comment

(left with NEAREST filter slightly sharper vs right with default linear)

I also compared it with openlayers, which looks super crisp. No idea how to get anywhere close to that.

(openlayers - no pixelation at all)

TLDR; not much clue where that pixelation is coming from.

All good, thanks for checking @na9da ! The quality issue is just a nice-to-have for me. We can improve it later. It could be something in TIFFImageryProvider causing the issue.

Thanks a lot @staffordsmith83 and @na9da ! We're so close to merging this in 😄 !

na9da commented 1 month ago

@nf-s thanks for the review.

I have now fixed some additional CI errors (related to a change in @types/leaflet).

Also made this change to free the imageryProvider when not used. The imageryProvider creates a bunch of webworkers. This change ensures that they get freed when the item is removed from the workbench. Ideally, we would have a destroy() life-cycle method for catalog items that would make this more explicit instead of relying on onBecomeUnobserved. Maybe we should add it when we deal with https://github.com/TerriaJS/terriajs/issues/4816

staffordsmith83 commented 1 month ago

Awesome, its in! Thanks @na9da and @nf-s

imakihi commented 1 month ago

Super cool. @na9da @nf-s @staffordsmith83

TerriaJS / terriajs