ogt_vox: ogt_assert in nTRN chunk for voxedit.io models

[mgerhardy]

There is an external editor available that is creating invalid nTRN chunks where the reserved id is not UINT32_MAX. It would be nice to still be able to load the model files that were exported by this software.

https://github.com/mgerhardy/vengi/commit/993b9bb5253c840efd2b874c612b07e78ed96278

[jpaver]

Hey @mgerhardy, thanks for the report. Not sure if this reserved field will be used in the future, so having the assert is useful to alert us when a new feature supported by MV is possibly being used within a given .vox file.

That being said, not sure it will be used -- MV development appears to have slowed down a lot so perhaps it's fine.

Can you confirm that the voxedit.io model is loadable by MV?

[mgerhardy]

Yes. Sorry for not mentioning it. The model loads fine in magicavoxel

[mgerhardy]

maybe another thing for the context object - hand in a warn or error function to log these things. But asserting would prevent loading these models - even if everything else would be fine.

[jpaver]

But asserting would prevent loading these models - even if everything else would be fine

It is possible for clients of the library to skip specific asserts if they need to by checking the message passed to the assert:.

void _vox_assert_msg(bool condition, const char * msg_str) {
  // condition wasn't violated? exit
  if (condition)
    return;
  // ignore specific asserts.
  if (strcmp(msg_str, "unexpected value for reserved_id in LAYR chunk") == 0)
    return;
  fprintf(stderr, "%s", msg_str);
  raise(SIGTRAP);
}

#define ogt_assert(cond, msg_str)    do { _vox_assert_msg(cond, msg_str); } while(0)
#include "ogt_vox.h"

...but I do sympathize. This sort of assert is really a warning of potential forward compatibility issues. I wonder if we should add an ogt_assert_warn so the library client can distinguish between fatal asserts and warning asserts, and then independently override them. eg

// implement a fatal assert
#define ogt_assert(cond, msg)   do {               \
     if ( !cond ) {                                \
       fprintf( stderr, "%s", msg_str )            \
       raise(SIGTRAP);                             \
  } while( 0 ) 

// implement a warning assert
#define ogt_assert_warn(cond, msg)  do {           \
      if ( !cond )                                 \
        fprintf( stderr, "WARNING: %s", msg_str ); \
  } while( 0 )
#include "ogt_vox.h"

...of course, we'd just turn these reserved_id asserts into ogt_assert_warn, and if ogt_assert_warn wasn't overridden by the client, ogt_vox would define it itself. eg.

// user didn't override warning assert? just map it to ogt_assert
#ifndef ogt_assert_warn
   #define ogt_assert_warn         ogt_assert
#endif

@mgerhardy if you think this is a good direction, feel free to put together a PR - otherwise, it may take me a while to get to it.

[mgerhardy]

@jpaver there we go - a PR is made. But the locations of the ogt_warn usage are still open for discussion I think. I'm not sure if I hit all locations or maybe even hit too many. Feedback welcome.