Skip to content

OpenCRG C library: crgOptionSetDefaultModifiers sets too many modifiers #23

@HeyImJordi

Description

@HeyImJordi

crgOptionSetDefaultModifiers in the OpenCRG C library sets the following modifiers:

crgOptionSetInt( optionList, dCrgModGridNaNMode, dCrgGridNaNKeepLast );
crgOptionSetDouble( optionList, dCrgModRefPointX,   0.0 );
crgOptionSetDouble( optionList, dCrgModRefPointY,   0.0 );
crgOptionSetDouble( optionList, dCrgModRefPointZ,   0.0 );
crgOptionSetDouble( optionList, dCrgModRefPointPhi, 0.0 );

Some background before coming to the actual issue: the CRG standard says that these modifiers are only valid if no modifier section is set in the CRG file. If any modifier is set, these default modifiers must not have any effect. The implementation is free to set default modifiers, if the file does not provide any.
The OpenCRG library implements this correctly, so from the point of view of standard compliance, this is fine.

It is also worth noting that setting e.g. dCrgModRefPointX to zero is not a no-op - it completely changes where the CRG file is placed in the world. See applyXform in crgDataApplyTransformation in crgMgr.c, where it checks whether the modifier is set.

That being set, the current dCrgModRefPoint default modifiers cause a few problems:

  • These modifiers are only set in the C API, not in the Matlab API. This means that on a CRG file without modifiers, the C API will return different coordinates than the Matlab API. This is highly confusing for users, and has caused a few days of additional work for us.
  • As far as I understand it, these modifiers make it impossible to use global CRGs without overriding these modifiers in the CRG file.

It seems like none of the CRG files we have use any modifiers, so all of them are affected.

I propose removing the dCrgModRefPoint modifiers from crgOptionSetDefaultModifiers to make the C- and Matlab-API more consistent and less surprising. Users of the library could still add these modifiers if desired using the public OpenCRG API.

Metadata

Metadata

Assignees

No one assigned

    Labels

    isState:ClassifiedAn issue that has a type and that needs to be accepted by the group.

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions