asmaloney / asmaloney/libE57Format

Simple API implementation of E57_EXT_surface_normals doesn't handle scaled ints

Open
#150 2 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
C++
Stars
191
Forks
82
Avg merge
7h 21m
Merged PRs (30d)
1

Description

In `Data3DPointsData_t`, the fields for E57_EXT_surface_normals are declared as floats:
```cpp
float *normalX = nullptr; //!< The X component of a surface normal vector (E57_EXT_surface_normals extension).
float *normalY = nullptr; //!< The Y component of a surface normal vector (E57_EXT_surface_normals extension).
float *normalZ = nullptr; //!< The Z component of a surface normal vector (E57_EXT_surface_normals extension).
```

And they are explicitly written as floats:
```cpp
// currently we support writing normals only as float32
if ( data3DHeader.pointFields.normalXField )
{
proto.set( "nor:normalX", FloatNode( imf_, 0., E57_SINGLE, -1., 1. ) );
}
```

~~I'm not sure why this restriction is here, but the [spec for the extension](http://www.libe57.org/E57_EXT_surface_normals.txt) allows for both floats and doubles (and uses doubles in their examples).~~

**Edit:** Should look at supporting scaled ints (see below).

Contributor guide

Open the contributing guide

Research direction

Start with the Data3DPointsData_t normalX, normalY, and normalZ declarations and the proto.set block for nor:normalX; compare their handling with the E57_EXT_surface_normals specification. Trace how scaled integer fields are represented during reading and writing, and consider the work complete when supported scaled normals are handled correctly rather than being restricted to float32.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
data
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.