Skip to content

Allow rasterizer to store z value at double precision - #2388

Merged
echeipesh merged 2 commits into
locationtech:masterfrom
moradology:feature/double-rasterizer
Sep 20, 2017
Merged

echeipesh merged 2 commits into
locationtech:masterfrom
moradology:feature/double-rasterizer

Conversation

@moradology

Copy link
Copy Markdown
Contributor

This PR allows rasterization to store values at double precision (which is immediately useful for elevation rasters) and allows the z values to be stored with a provided CellType

@moradology
moradology force-pushed the feature/double-rasterizer branch from 907c288 to 8af0d1e Compare September 19, 2017 20:19
@jamesmcclain

Copy link
Copy Markdown
Member

I think that this was originally the case, but these values were narrowed for economic reasons.

@echeipesh

Copy link
Copy Markdown
Contributor

Yep, this PR parameterizes it so economy and precision can be served.

case class CellValue(value: Double, zindex: Short)

/** Cell value with its zindex and celltype to be used by the rasterizer. */
case class CellValue(value: Double, zindex: Double, celltype: CellType)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm realizing now that the issue was misstated, exposing cellType for zindex per feature actually leads ambiguous behavior: the order in which tiles are merged is undefined and if two priority tiles do not share the same zindex type it is unclear which will be updated and propagated.

So this parameter should be at function invocation level as zindexCellType. Probably ByteConstantNoDataCellType is a nice conservative default value for it.

@moradology
moradology force-pushed the feature/double-rasterizer branch from 26b8e77 to 6c7227f Compare September 20, 2017 20:46
@moradology

Copy link
Copy Markdown
Contributor Author

@echeipesh I believe I've addressed the issue you raised - let me know if I'm missing something

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