-
Notifications
You must be signed in to change notification settings - Fork 1
Add cell layer only option #107
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for a new cell_layer_only mode that restricts rendering and data fetching to the cell layer, and documents the new parameter in the spec.
- Extract and apply
cell_layer_onlyinlandscape_istto conditionally toggle off other layers. - Update viewport calculation to set
max_tiles_to_viewto 0 whencell_layer_onlyis true. - Document
cell_layer_onlyin the LandscapeFiles file format spec.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| js/viz/landscape_ist.js | Introduce cell_layer_only flag and adjust layer visibility logic |
| js/deck-gl/core/calc_viewport.js | Destructure cell_layer_only and conditionally set max_tiles_to_view |
| docs/overview/file_formats.md | Add documentation for the new cell_layer_only parameter |
Comments suppressed due to low confidence (1)
js/viz/landscape_ist.js:402
- Add unit tests covering the
cell_layer_onlyflag path to ensure only the cell layer toggles are applied as expected.
if (cell_layer_only) {
| await set_landscape_parameters(viz_state.img, base_url, viz_state.aws); | ||
|
|
||
| const cell_layer_only = | ||
| viz_state.img.landscape_parameters.cell_layer_only === true; |
Copilot
AI
Jul 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider destructuring cell_layer_only alongside other landscape parameters at the top of the function to keep extraction logic consistent and avoid inline property access throughout the function.
| ) => { | ||
| const { tile_size } = viz_state.img.landscape_parameters; | ||
| const max_tiles_to_view = 50; | ||
| const { tile_size, cell_layer_only } = |
Copilot
AI
Jul 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tile_size variable is destructured but not used in this function; consider removing it if it's unnecessary to avoid confusion.
Summary
landscape_istcell_layer_onlyparameter in the LandscapeFiles specTesting
npm test(fails: jest not found)https://chatgpt.com/codex/tasks/task_b_6863ef798fb4832ab9acc647280c78f6