[Mavrick-F]

Summary

Second item of step 4 of #1111: Hillshade writes Byte. A hillshade's values are whole numbers 0–255, but it was made and written as 32-bit floats.

• Hillshade is made as bytes. raster.function with "function": "hillshade", and raster.functionChain when its last step is one, now make an 8-bit (UInt8) raster. Its shades run 1–255 and 0 marks no data: a cell in full shadow is 1 instead of 0, and every lit cell keeps its value. GDAL's gdaldem hillshade writes the same: bytes, shades from 1 and 0 as no-data, so the file reads the same there. A step after a hillshade in a chain reads its no-data and makes floats as before.
• The layer still looks the same. It is drawn as a grey stretch on the session's raster defaults. Without that, a hillshade of gentle ground with 16 or fewer shades would open as Unique Values, as other whole-number rasters do.
• The untiled GeoTIFF writer writes an 8-bit grey band as bytes. The COG writer already did this. The untiled writer (share.exportRaster without cog, and the copy of a made raster a project keeps) wrote every single grey band as 32-bit floats, so a byte raster's file was four times its size. It now writes bytes when the raster is UInt8, every value is a whole number 0–255, and an empty cell occurs only where the no-data value can mark it. Otherwise it writes floats as before. This applies to any 8-bit grey raster, not only hillshades; the values read back unchanged.

A COG of a hillshade holding 64 or fewer distinct shades counts as classes for the COG writer, so its overviews take a pixel instead of an average. A hillshade of a real elevation model holds far more shades than that.

Measured on the 9667 × 11947 elevation model: to follow in this description.

Testing

• New rasterops::tests::hillshade_keeps_zero_for_no_data: a cell in full shadow is 1, and the bytes made of it are UInt8 with 0 for the no-data cell.
• New geotiff::tests::grey_bytes_round_trip_as_bytes: an 8-bit grey band reads back as UInt8 with its no-data. A band with an unmarked empty cell, or with a fraction, stays Float32.
• New tests::hillshade_is_made_and_written_as_bytes: Raster Functions and a chain ending in a hillshade make UInt8 layers drawn as a stretch, including a single-shade plane. A step after a hillshade honours its no-data. The plain and COG exports read back as UInt8 with the same values.
• The existing terrain, raster function, background and cancel tests pass unchanged.
• The pull request CI jobs (lint with clippy -D warnings, the workspace tests and layers, wasm, licences) on the fork at 523e6e6: pass (run)

UI evidence

No visible UI change: the hillshade layer is drawn with the same stretch as before.

Related issue

Part of #1111

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Hillshade results now use 8-bit shades from 1 to 255, with 0 reserved for no-data, and are displayed with a stretch renderer.
  • Eligible single-band GeoTIFF exports are stored as 8-bit grayscale; other single-band exports retain floating-point data.
• Documentation
  • Updated guidance on hillshade values and GeoTIFF export formats.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Hillshade functions now produce UInt8 rasters with values from 1 to 255 and no-data value 0. Function workflows apply raster defaults to hillshade results. Eligible single-band GeoTIFF output uses byte encoding.

Changes: Hillshade output

Layer / File(s)	Summary
Hillshade value encoding crates/analysis/src/rasterops.rs, docs/user-guide/analysis.md	Hillshade values are clamped to 1–255. derive_hillshade creates a UInt8 raster with no-data value 0. A test checks shadow values and no-data handling.
Function workflow integration crates/engine/src/cmd/workflows.rs, crates/engine/src/tests.rs	Single hillshade functions and chains derive hillshade rasters and apply session raster styling when the output ends in hillshade. Integration coverage checks output properties, chains, and exports.
GeoTIFF byte encoding crates/data/src/raster/geotiff.rs, docs/user-guide/data.md	The writer uses Gray8 for eligible UInt8 bands. Bands that do not meet the byte criteria retain Gray32Float output. Tests check round-trip behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant function_work
  participant derive_hillshade
  participant made_hillshade
  function_work->>derive_hillshade: derive UInt8 hillshade raster
  derive_hillshade-->>function_work: return raster
  function_work->>made_hillshade: apply raster defaults

Loading

Possibly related issues

• Tracking: large data (layers over 2 million features, slow hillshade and GeoTIFF writing) #1111: This PR implements the issue’s hillshade byte output and updates the untiled GeoTIFF writer for eligible UInt8 bands. The issue lists additional open steps.

Possibly related PRs

• opengeos/RustGIS#1672: It changes the GeoTIFF byte-selection rule and the workflow made path, which overlap code changed here.
• opengeos/RustGIS#1874: Its changes to fits_bytes and COG byte-type selection affect byte eligibility used by this PR.

Suggested reviewers: giswqs, pick2510

Merge Risk: Minimal · up to 523e6

Hillshade outputs become 8-bit rasters with 0 reserved for no-data, and eligible GeoTIFF exports are written as bytes. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the perf scope (analysis) and the main change: hillshade is now made and written as 8-bit bytes. This matches the changeset, which adds UInt8 hillshade output and a byte-writing path i…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description follows the repository template. It has Summary, Testing, UI evidence, and Related issue sections. The Summary states the change and its reasons: hillshade is written as UInt8 with 0 a…

Full details: Title check

Explanation

The title names the perf scope (analysis) and the main change: hillshade is now made and written as 8-bit bytes. This matches the changeset, which adds UInt8 hillshade output and a byte-writing path in the untiled GeoTIFF writer. The title omits the GeoTIFF writer change, but that is secondary to the main point.

Full details: Description check

Explanation

The description follows the repository template. It has Summary, Testing, UI evidence, and Related issue sections. The Summary states the change and its reasons: hillshade is written as UInt8 with 0 as no-data, and the untiled GeoTIFF writer now writes eligible 8-bit grey bands as bytes. The Testing section names three new tests and reports CI results on fork revision 523e6e6. The UI evidence section states that no visible UI change exists, which matches the template instruction. The Related issue section links Part of #1111. One gap remains: the description says the measurement on the 9667 x 11947 elevation model will follow, so a reported result is still missing. This is a non-critical item and does not block the check.

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 shades the hills in gray,
One marks the shadows through the day.
Zero keeps the no-data place,
Byte-sized pixels hold their space.
The raster hops from chain to file,
Then checks its round-trip with a smile.}

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: 2eb08c10-32bf-4d2a-b7a5-e57236a499f6

Commits

Reviewing files that changed from the base of the PR and between 1654377 and 523e6e6.

Files selected for processing: 6

• crates/analysis/src/rasterops.rs
• crates/data/src/raster/geotiff.rs
• crates/engine/src/cmd/workflows.rs
• crates/engine/src/tests.rs
• docs/user-guide/analysis.md
• docs/user-guide/data.md

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

[github-actions]

Code review

Bugs. None found. I checked the untiled byte path's no-data handling: fits_bytes already limits no-data to whole values 0–255, so d as u8 cannot wrap, and invalid cells only reach the byte path when no-data can mark them. I also checked derive_hillshade's clamp (only hillshade output reaches it, so nothing real is dropped), the chain's hillshade and no-data handling, and the test expectations against the code. Confidence: high.

Security. None. The new code adds no input parsing; parameters go through the existing ps/f helpers and clamps. Confidence: high.

Performance. No defect. The byte branch makes two passes over the band (the all(byte) pre-scan, then the conversion) and allocates a w×h byte buffer next to the float path. That should be a net win, since the file is a quarter of the size. The PR body still says the measurements are "to follow", so the size and speed benefit is unmeasured. Confidence: medium.

Quality (nits; I left these for the release quality pass and did not post inline comments):

• crates/data/src/raster/geotiff.rs:77: the reworded doc line runs to about 145 columns, past max_width = 140. rustfmt won't wrap it, so it needs a manual reflow. Confidence: high.
• docs/user-guide/data.md:1185: "an 8-bit grey band … as bytes" leaves out two conditions the writer checks: every value is a whole number, and an empty cell needs a no-data value. Confidence: high.
• crates/engine/src/cmd/workflows.rs: the hillshade-to-stretch logic is repeated in function_work and function_chain_work. A small helper would remove the duplication. Confidence: medium.

AGENTS.md. Compliant on the points I checked:

• No unwrap, expect or panic in non-test code; the unwrap_or_else(|e| panic!) calls are in tests.
• The new work stays inside the existing background closures, and the new byte loop calls progress::check.
• Each change has a test for the clean case and the lossy or fallback case.
• The docs are updated.

One claim I could not verify offline: the PR says a 1–255 shade range with 0 as no-data "is the convention other tools' hillshades use". Before merge, the PR should cite the tool or soften the wording. Confidence in the claim being wrong: low.

Earlier review threads: none (prior-reviews.json is empty), so I left no concerns alone. I posted no inline comments because I found nothing that needs a fix before merge.

Change size

Compared with the base branch (merge base d39c16e to PR head 523e6e6), covering the whole PR.

Measure	Value
Files changed	6 (added 0, modified 6, removed 0, renamed 0)
Size before	3264.2 KB (3.2 MB)
Size after	3272.6 KB (3.2 MB)
Total size change	+8.4 KB

Path	Before	After	Change
crates/engine/src/tests.rs	2591.0 KB	2594.1 KB	+3.2 KB
crates/data/src/raster/geotiff.rs	195.4 KB	197.4 KB	+2.0 KB
crates/analysis/src/rasterops.rs	140.6 KB	142.3 KB	+1.7 KB
crates/engine/src/cmd/workflows.rs	86.7 KB	88.1 KB	+1.3 KB
docs/user-guide/data.md	108.0 KB	108.1 KB	+94 B
docs/user-guide/analysis.md	142.6 KB	142.7 KB	+49 B

[Mavrick-F]

• The convention: the description now names it. GDAL's gdaldem hillshade writes bytes with shades from 1 and 0 as no-data.
• The measurement is still to come; it waits on a release build in line on the build machine, and goes into the description when it's done.
• The three quality points (the 150-column doc line in geotiff.rs, the fuller conditions in data.md, and a shared helper for the stretch in the two raster-function paths) are left for the release quality pass. If this pull request needs another push for a fix, they go into it.

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-1887/
Web app	https://preview.rustgis.app/pr-1887/app/
Commit	523e6e6

GitHub Pages takes a minute or two to publish a new commit. Newer commits from the fork aren't published until a maintainer adds the preview label (once per commit).