[choupeanut]

Summary

Closes #1798. The map's Shaded Relief lit a raster stored bottom up (positive pixel height, as some COGs are) from the mirrored azimuth: a slope facing south, lit from the north, came out brighter than flat ground. A raster stored right to left was mirrored east–west the same way.

Cause: Pass1::rows (crates/render/src/raster.rs) built the 3×3 window in grid order. It then handed it to rustgis_map::raster::hillshade, which takes the window's first row as north and its first column as west.

Fix: Pass1 now carries the grid's direction (east_north): −1 for columns stored right to left and for rows stored bottom up. The window's offsets are multiplied by it, so the window is always read west to east and north to south. This is the same idea as analysis::rasterops::terrain_on, which already handles it. hillshade itself and the unsigned cell sizes are unchanged, and nothing else calls hillshade with a grid window (the DEM tiles' hillshade reads north-up web tiles).

The cast shadows (#1796) follow the transform's signs already, so on a bottom-up raster the hillshade and the shadows now agree.

Testing

• New raster::tests::shaded_relief_follows_the_grids_orientation:
  • It draws the same 10×10 terrain stored in all four directions (north up, bottom up, right to left, and both).
  • Terrain rising to the north, lit from azimuth 0 at 45°: north up stays 159, darker than flat ground, and the other three directions draw 159 too.
  • A diagonal slope lit from 45°, 135°, 225° and 315° draws the same in all four directions, which covers east–west mirroring.
  • Without the fix, it fails with the issue's numbers: 230 bottom up against 159.
• cargo test -p rustgis-render -p rustgis-map passes (render 428, map 311).
• cargo clippy -p rustgis-render -p rustgis-map --all-targets -- -D warnings and cargo fmt --all --check are clean.
• No UI or docs change. The draw path uses no threads or timers, so the web build is unaffected (not built locally: zstd-sys needs a clang this machine lacks).

Summary by CodeRabbit

• Bug Fixes
  • Shaded relief now follows the correct terrain neighborhood when raster data is stored in different orientations, including grids that run right-to-left or bottom-up. This improves consistency across raster layouts, so changing how data is stored no longer changes the shaded-relief result.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Pass1 now records grid directions from pixel dimensions. Shaded-relief neighbor sampling uses those directions, and a new test compares output across north-up, right-to-left, bottom-up, and combined reversed orientations.

Changes: Shaded-relief orientation

Layer / File(s)	Summary
Direction-aware hillshade sampling crates/render/src/raster.rs	Pass1 stores column and row directions derived from pixel dimensions. Shaded-relief neighbor offsets use those directions. A test compares rendered output across four storage orientations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: giswqs, deniial00

Merge Risk: Minimal · up to d1e61

Shaded Relief should now light reversed-orientation rasters consistently with north-up ones. No merge-blocking risk remains after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: Shaded Relief lighting on bottom-up or right-to-left rasters, which the raw summary and objectives both describe. It is specific and matches the changeset. Its length …
Description check		The description follows the template. It has a Summary that explains the cause and fix, and a Testing section with the new test, the commands run, and the reasons the web build was not run. The UI evi…
Linked Issues check		Linked issue #1798 is the only direct, active target. Its coding requirements are: (1) draw the same terrain the same way whatever the storage direction; (2) give hillshade the window in geographic or…
Out of Scope Changes check		The whole-PR diff against the merge base changes only crates/render/src/raster.rs. The changes are: one east_north field initialiser in draw_with, the Pass1 field and its doc comment, one line applyin…

Full details: Title check

Explanation

The title names the main change: Shaded Relief lighting on bottom-up or right-to-left rasters, which the raw summary and objectives both describe. It is specific and matches the changeset. Its length (97 characters) is on the long side but still a single clear sentence.

Full details: Description check

Explanation

The description follows the template. It has a Summary that explains the cause and fix, and a Testing section with the new test, the commands run, and the reasons the web build was not run. The UI evidence section states that there is no visible UI change, which the template allows. The Related issue content is covered by 'Closes #1798' in the Summary, though the template's dedicated Related issue heading is absent.

Full details: Linked Issues check

Explanation

Linked issue #1798 is the only direct, active target. Its coding requirements are: (1) draw the same terrain the same way whatever the storage direction; (2) give hillshade the window in geographic order; (3) add a test that draws both orientations and compares them. The diff in crates/render/src/raster.rs meets each one. Pass1 gains east_north, which is -1 when pixel_width < 0 (columns right to left) and -1 when pixel_height > 0 (rows bottom up). The 3x3 offsets are multiplied by it before sampling, so the window is west to east and north to south, as rustgis_map::raster::hillshade expects. I checked the sign logic against the grid indexing: for bottom-up grids, dy = -1 maps to row ri + 1, the northern row, and for right-to-left grids, dx = -1 maps to column ci + 1, the western column. The new test shaded_relief_follows_the_grids_orientation draws the same terrain in four storage orientations. It checks the north-facing slope value 159 and compares diagonal slopes at 45, 135, 225 and 315 degrees. This matches the issue's expected behaviour. Cast shadows (#1796) already use transform signs, so no change is needed there, as the issue states. Limitation: I verified this by reading the code and diff. I did not run the tests, so the reported values (159, 230) are not independently confirmed.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base changes only crates/render/src/raster.rs. The changes are: one east_north field initialiser in draw_with, the Pass1 field and its doc comment, one line applying the offset multiplier in the 3x3 window loop, and one new test. Each change supports the shaded-relief orientation fix for #1798. hillshade, unsigned cell sizes, and the DEM web-tile path are not modified, as the PR description states. No unrelated change was found.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the slopes at dawn,
And follows north where rows are drawn.
Eastward, westward, heights align,
Four stored views now shade as one.
The rabbit hops, then marks the sign.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 3132ac1c-e653-45c8-8ace-2a940e80b0c6

Commits

Reviewing files that changed from the base of the PR and between cf61cc6 and d1e613d.

Files selected for processing: 1

• crates/render/src/raster.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

[github-actions]

Code review

Bugs: None found. I traced the window reorientation against hillshade in crates/map/src/raster.rs, which takes the window north row first and west column first. For pixel_width < 0 the column step is −1, and for pixel_height > 0 the row step is −1, so the flipped offsets give the north-up window for every orientation. The north-up case is unchanged (both steps are +1). Only one Pass1 construction site exists, so the new field can't be missed. The data layer keeps bottom-up and right-to-left rasters as they are (GeoTIFF and COG export refuse them, geotiff.rs:1672, cogwrite.rs:532), so the bug path is real. Confidence: high.

Security: None. No new input parsing, I/O or unsafe code. The new code is integer arithmetic on ±1 steps, and value() already bounds-checks through get(). Confidence: high.

Performance: None. The per-pixel cost is two integer multiplies on a 9-entry loop. Confidence: high.

Quality (nits, not inline-worthy):

• crates/render/src/raster.rs:520 packs both sign tests into one long tuple expression. A named local above the struct literal would read better.
• The field name east_north describes the step direction, not the grid, so grid_step or window_sign would be clearer. The doc comment already explains it.
• The test's assert_eq!(lit[0], 159) is a magic number. It works as a regression anchor, so I'd leave it.

These are style points and can wait for the release quality pass.

AGENTS.md: Compliant. No unwrap/expect in non-test code (the test uses unwrap like its neighbours), no unsafe, no UI or threading change, and the new test sits beside the existing draw tests in the same style. Line lengths fit max_width = 140. The test covers both lossless orientations (north up and the other three), so the "test both ways" spirit holds.

Test verification: I did not run the tests, since this review has no execution tool. The 159 and 230 values are the PR's own claims and are not confirmed here. Reasoning about the test's geometry (linear terrain, cell choice at the view centre) suggests it is robust to which neighbouring cell Nearest picks, and it does discriminate the bug: without the fix the bottom-up case mirrors the window.

Prior concerns: No review threads exist on this PR, so none were left alone.

No inline comments were posted, since no finding warrants one.

Change size

Compared with the base branch: the whole PR against main (merge base cf61cc6 to PR head d1e613d).

Files changed	Added	Modified	Removed	Renamed	Size before	Size after	Total size change
1	0	1	0	0	60.5 KB	63.8 KB	+3.3 KB

Path	Size before	Size after	Size change
crates/render/src/raster.rs	60.5 KB	63.8 KB	+3.3 KB

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-1849/
Web app	https://preview.rustgis.app/pr-1849/app/
Commit	d1e613d

GitHub Pages takes a minute or two to publish a new commit.