RWMesh - Fix lossy encoding of non-ASCII node names in glTF export

[Clonephaze]

RWMesh::ReadNameAttribute and RWMesh::FormatName convert node names from TCollection_ExtendedString (UTF-16) to TCollection_AsciiString. Lossy for non-ASCII characters, which are replaced with a substitution byte rather than being encoded correctly.

The PR just adds the small static helper ExtendedToUtf8 that converts via NCollection_UtfString<char>, and all call sites in ReadNameAttribute and FormatName are updated to use this helper.

Verified with a STEP file containing French part names (Pièce, Première, butée) which previously produced \ufffd replacement characters in the glTF JSON chunk. Node names now survive conversion.

[dpasukhi]

Dear @Clonephaze

Thank you for your patch. To proceed with integration, please complete signing CLA process:
The links: Contribution Guide
Or https://dev.opencascade.org/get_involved

In case if you already have signed CLA, please share ID that OCCT team shared with your by email after accepting of CLA.

[Clonephaze]

After further testing while attempting to use the fix in real world situations, I've found that my current submitted approach is insufficient. The original change to RWMesh.cxx did not actually fix the bug, I had convinced myself it worked based on limited testing, but further investigation showed the conversion was already handled correctly elsewhere. I sincerely apologize for that.

The real problem is in Resource_Unicode::ConvertFormatToUnicode when called with Resource_FormatType_UTF8. NCollection_UtfIterator does not validate UTF8 continuation bytes. Instead, it blindly consumes bytes based on the lead byte. So a STEP file with a Latin-1 product name like pièce (where 0xE8 is followed by plain ASCII) gets decoded as a garbage codepoint instead of è.

I've attempted to fix this by validating the string with IsStrictUtf8() first, then falling back to a western European decoder (CP850 for 0x80-0x9F, Latin-1 for 0xA0-0xFF) if the bytes are not well-formed UTF8.

I cannot push to this branch while the PR is blocked, but the corrected diff touches Resource_Unicode.cxx where the actual fix is implemented. Instead I've pushed the changes to a new branch proper-fix found here: https://github.com/Clonephaze/OCCT/tree/proper-fix

Additionally, I've attempted to write a few unit tests to cover the new behavior. They are in Resource_Unicode_Test.cxx and are included in the GTests/FILES.cmake list. A Draw Harness test was also added, bug_gh1321. The tests cover:

• Valid UTF8 sequences (1, 2, and 3 byte sequences)
• Invalid UTF8 sequences that should fall back to western European decoding (Latin-1 and CP850)
• bug_gh1321 creates a synthetic STEP file with a Latin-1 product name and verifies that it round trips

Happy to rebase and update the PR once the CLA is confirmed. Still waiting for that validation.

[gkv311]

@Clonephaze

The real problem is in Resource_Unicode::ConvertFormatToUnicode when called with Resource_FormatType_UTF8. NCollection_UtfIterator does not validate UTF8 continuation bytes. Instead, it blindly consumes bytes based on the lead byte. So a STEP file with a Latin-1 product name like pièce (where 0xE8 is followed by plain ASCII) gets decoded as a garbage codepoint instead of è.

I've attempted to fix this by validating the string with IsStrictUtf8() first, then falling back to a western European decoder (CP850 for 0x80-0x9F, Latin-1 for 0xA0-0xFF) if the bytes are not well-formed UTF8.

There is no any good reason to assume STEP file being stored in CP850 code page, if it contains invalid UTF-8 strings. It can be stored in any other code page, hence you can find a long list of encodings in Resource_FormatType enumeration, with Resource_FormatType_CP850 only one of them. And real-world STEP files are indeed, stored in all sorts of other encodings (when ISO standard suggests using UTF-8 nowadays, and in the past there was a special syntax to write extended symbols in STEP format that OCCT supports).

Even if you'll suggest an encoding specific to user's locale as a best guess (from Windows settings, for instance), it might only work for users exchanging files within the same locale. And for sure, this is not a good way to do that automatically in OCCT level, as it would make conversion results to depend on a OS configuration.

Giving user a choice is probably the most reasonable way to interact with legacy code pages problem.

One may to add an option for using an code page auto-detection mechanism. Such mechanism rely on statistical peculiarities of different text in different languages and require a reasonable amount of non-English text for proper detection, which is not always the case for STEP files, where there might be just a couple of localized strings inside - so that text editors with such autodetection fail to select correct code page.

I think neither autodetection nor automatic fallback to another code page aren't a good way to go with this problem in OCCT. However, since there are some means to detect an invalid UTF-8 stream - probably one may add this validation at translation level or even as a post-processing step after conversion is done to report an error or a warning to user and suggest to pickup another encoding and re-import file again (within an interactive GUI, one may even show user a suspicious string from STEP file with a combo-box to try interpret the same string using different encodings).

[Clonephaze]

@gkv311 Thank you for the feedback

Yeah that makes sense, I don't know why I assumed a fallback to only one encoding would be appropriate. Especially for global userbase.

A fundamental misunderstanding I had was that this encoding issue was specific to legacy STEP files and that modern STEP files (like those exported from Fusion 360) handle writing UTF-8 correctly from the start. The escape-based path in cleanText handles modern files correctly.

Would the right direction be to keep IsStrictUtf8() as a detection step but replace the fallback with just a warning, telling the user to set read.step.codepage to match their file's encoding? That seems to align with what you described. If so I'll start working on that change and updating the tests to look for the warning instead of the fallback behavior.

[gkv311]

@Clonephaze

Would the right direction be to keep IsStrictUtf8() as a detection step but replace the fallback with just a warning, telling the user to set read.step.codepage to match their file's encoding?

I would find it useful, but cannot say for implications and other end users.

I don't know which criteria could be used to validate UTF-8 or UTF-16 - NCollection_UtfIterator has only two checks, but your patch does something different:

  //! Return true if Unicode symbol is within valid range.
  bool IsValid() const
  {
    return myCharUtf32 <= UTF32_MAX_LEGAL;
  }
...
// =======================================================================
// function : readUTF8
// purpose  : Get a UTF-8 character; leave the tracking pointer at the start of the next character.
//            Not protected against invalid UTF-8.
// =======================================================================
template<typename Type>
inline void NCollection_UtfIterator<Type>::readUTF8()
{
  // unsigned arithmetic used
  Standard_Utf8UChar* aPos = (Standard_Utf8UChar* )myPosNext;
  const unsigned char aBytesToRead = UTF8_BYTES_MINUS_ONE[*aPos];
  myCharUtf32 = 0;
  switch (aBytesToRead)
  {
    case 5: myCharUtf32 += *aPos++; myCharUtf32 <<= 6; // remember, illegal UTF-8
      Standard_FALLTHROUGH
    case 4: myCharUtf32 += *aPos++; myCharUtf32 <<= 6; // remember, illegal UTF-8
      Standard_FALLTHROUGH
    case 3: myCharUtf32 += *aPos++; myCharUtf32 <<= 6;
      Standard_FALLTHROUGH

Probably you may start with smaller steps and propose adding a method to NCollection_UtfString / TCollection_AsciiString / TCollection_ExtendedString for validating multi-byte UNICODE symbol sequences.

Then one may suggest some tool for checking string issues in existing XCAF document (that might be not that simple to traverse all occurrences of string in the document, but probably starting from very basics like names of assembly elements might useful enough) or to imply these check in string conversion routines (but as this used only within STEP import, you wouldn't be able to check already translated XCAF documents or imports from other formats like glTF).