Skip to content

Add client side raster reprojection support#562

Merged
gberaudo merged 6 commits into
masterfrom
client_side_raster_reprojection
May 2, 2018
Merged

Add client side raster reprojection support#562
gberaudo merged 6 commits into
masterfrom
client_side_raster_reprojection

Conversation

@gberaudo

@gberaudo gberaudo commented Mar 2, 2018

Copy link
Copy Markdown
Member

Fixes #37.

@gberaudo

gberaudo commented Mar 2, 2018

Copy link
Copy Markdown
Member Author

@klokan, your feedbacks would be welcomed.

@ahocevar, we discussed a long time ago about this one. Thanks for your guidances back then at FOSS4G Seoul. If you find some time, a second look from you would be very appreciated.

Comment thread src/olcs/core/olimageryprovider.js Outdated

const tilegrid = this.source_.getTileGridForProjection(this.projection_);
if (z_ < tilegrid.getMinZoom() || z_ > tilegrid.getMaxZoom()) {
return this.emptyCanvas_; // no data

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are several places in this function which do not follow the Cesium API reference and return something other than Promise<Image|Canvas>|undefined. Is that intentional based on undocumented Cesium workings?

@gberaudo gberaudo Apr 16, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @wallw-bits, you are right it works but it is incorrect.
I am fixing this one by wrapping the returned value like this: Promise.resolve(this.emptyCanvas_). Can you come up with a PR to fix the other incorrect places?

wallw-dev added a commit to wallw-dev/opensphere that referenced this pull request Apr 13, 2018
This is related to (and based on) openlayers/ol-cesium#562 and openlayers/ol-cesium#37. However, we

need it for GeoPackage support rather than for reprojection. Additionally, this helps support any

other custom tile classes and tile load functions in Cesium.
Comment thread src/olcs/core/olimageryprovider.js Outdated

const state = tile.getState();
if (state === ol.TileState.LOADED || state === ol.TileState.EMPTY) {
return tile.getImage() || undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This return also needs a promise wrapper if tile.getImage() is not undefined

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, done.

@gberaudo gberaudo force-pushed the client_side_raster_reprojection branch from 5bd340b to 182b5e0 Compare April 17, 2018 10:52
@gberaudo gberaudo force-pushed the client_side_raster_reprojection branch from 182b5e0 to 8a43c2e Compare May 1, 2018 15:49
@gberaudo

gberaudo commented May 1, 2018

Copy link
Copy Markdown
Member Author

I am hiding the feature behind a flag so that it can be merged. @fredj, could you please have a look to it?

@gberaudo gberaudo merged commit 22bc888 into master May 2, 2018
@gberaudo gberaudo deleted the client_side_raster_reprojection branch May 2, 2018 06:39
@gberaudo gberaudo restored the client_side_raster_reprojection branch May 2, 2018 12:32
@gberaudo

gberaudo commented May 2, 2018

Copy link
Copy Markdown
Member Author

This branch have issues. I am restoring it and "unmerging" the PR.

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.

3 participants