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

PB-1121: compare slider support for COGtiffs #1122

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

ltkum
Copy link
Contributor

@ltkum ltkum commented Nov 8, 2024

Issue : COGTiffs are rendered as a webGL layer, which has a different graphical context than other OpenLayers layers. This causes the compare slider to not work on them.

Fix : when we deal with a COGtiff layer, we instead use the webgl context tools to make the slice.

Test link

@github-actions github-actions bot added the bug label Nov 8, 2024
Copy link

cypress bot commented Nov 8, 2024

web-mapviewer    Run #3858

Run Properties:  status check passed Passed #3858  •  git commit 5779d5fa92: PB-1121: using pre compose event to handle COG slider
Project web-mapviewer
Branch Review fix-PB-1121-support-compare-slider-for-COGTIFFs
Run status status check passed Passed #3858
Run duration 05m 13s
Commit git commit 5779d5fa92: PB-1121: using pre compose event to handle COG slider
Committer Martin Künzi
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 212
View all changes introduced in this branch ↗︎

@ltkum ltkum force-pushed the fix-PB-1121-support-compare-slider-for-COGTIFFs branch 2 times, most recently from cc06929 to 087aec6 Compare November 14, 2024 13:47
@ltkum ltkum marked this pull request as ready for review November 14, 2024 13:57
@ltkum ltkum requested a review from pakb November 14, 2024 13:57
@ltkum ltkum force-pushed the fix-PB-1121-support-compare-slider-for-COGTIFFs branch from a99adc1 to 5e9770d Compare November 15, 2024 11:17
@ltkum ltkum requested a review from pakb November 15, 2024 12:48
@ltkum ltkum force-pushed the fix-PB-1121-support-compare-slider-for-COGTIFFs branch from 5e9770d to f96b7fc Compare November 15, 2024 14:32
@pakb
Copy link
Contributor

pakb commented Nov 15, 2024

If you load (import) https://data.geo.admin.ch/ch.swisstopo.rapidmapping/data/2024-012-MISOX/RM_2024_MISOX_DOPmosaic_25cm_20240628_RGB8BIT_LV95_COG.tif while the compare slider is active, the newly created layer isn't cut.
Can you check that that way of "using" the compare also works as expected?

@ltkum
Copy link
Contributor Author

ltkum commented Nov 18, 2024

If you load (import) https://data.geo.admin.ch/ch.swisstopo.rapidmapping/data/2024-012-MISOX/RM_2024_MISOX_DOPmosaic_25cm_20240628_RGB8BIT_LV95_COG.tif while the compare slider is active, the newly created layer isn't cut. Can you check that that way of "using" the compare also works as expected?

Sure

@ltkum ltkum force-pushed the fix-PB-1121-support-compare-slider-for-COGTIFFs branch from 1041d06 to cae50e8 Compare November 18, 2024 10:50
Issue : COGTiffs are rendered as a webGL layer, which has a different graphical context than other OpenLayers layers. This causes the compare slider to not work on them.

Fix : when we deal with a COGtiff layer, we instead use the webgl context tools to make the slice.
- refactored the slicing mechanism to reduce the number of event assignations
- we clear the webGL context before rendering, to avoid artifacts
- we set the id directly in the COGTIFF layer
- Issue : When importing through the menu, the watchers in the compare slider caught the layer on top being modified before the layer was added to the OpenLayers Map object itself
- Fix : when importing through the menu, we now send a signal that we should force a compare slider update once the import is finished
Issue : When importing or toggling visibility on WebGL layers, the layerconfig was updated before the map, and the new layer would not have the prerender and postrender events set.

Fix : OpenLayers Map fire precompose events when webGL layers are involved. This allows us to add a precompose event on the map to add the prerender events just before the first rendering is supposed to happen.
@ltkum ltkum force-pushed the fix-PB-1121-support-compare-slider-for-COGTIFFs branch from 5fea931 to 5779d5f Compare November 18, 2024 14:56
@ltkum ltkum merged commit df85181 into develop Nov 18, 2024
6 checks passed
@ltkum ltkum deleted the fix-PB-1121-support-compare-slider-for-COGTIFFs branch November 18, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants