Translate user-visible text coming from logic classes

[lassoan]

Most text that is displayed for users is coming from Qt GUI classes and translated using Qt translation infrastructure. However, there are probably cases when the GUI displays messages coming from logic classes. We need to review all the module logics classes (e.g., in all Logic, MRML, MRMLDM, VTKWidgets folders) if they provide any strings that end up being displayed for users and make sure they are translated.

Design options:

• Option A. Add a VTK-based translation class or function (that calls QApplication translate function). This is useful if there are many complex messages that logic classes may generate.
• Option B. Translate messages coming from logic classes in the GUI classes that display them. This may be better if there are only few or simple cases when logic classes provide messages visible to the user.

[lassoan]

Examples:

• Volume rendering preset names. The preset name comes from node names in the presets.xml scene file (see Improve volume rendering presets #6762 for some more details).
• View labels (Red, Yellow, ...) and viewport names in layout XML files

[lassoan]

A new MRML_TRANSLATE_NOOP macro could be added, which could work the same way as QT_TRANSLATE_NOOP.

#define QT_TRANSLATE_NOOP(scope, x) x

Source text for translation should be possible by specifying extra options for lupdate.

A global function or singleton class could be added to MRML that would call QCoreApplication::translate via a callback mechanism. The callback mechanism is needed to avoid MRML depend on Qt classes. The Qt application would set the callback function pointer to QCoreApplication::translate at application startup.

[lassoan]

Instead of a macro, vtkMRMLTr method could be added that would be used like this in the source code:

this->SupportedReadFileTypes->InsertNextValue(vtkMRMLTr("vtkMRMLVolumePropertyStorageNode", "Volume Property")+" (.vp)");

lupdate can be configured to recognize this function by modifying how it is used in update_translations.py like this:

command = [lupdate_path, source_code_dir, "-tr-function-alias", "QT_TRANSLATE_NOOP+=vtkMRMLTr", "-locations", "absolute", "-ts", ts_file_path_in_source_tree]

[mhdiop]

Hi @lassoan,
Hi @pieper.

I've started working on the implementation of the vtkMRMLTr function in the Libs/MRML/Core/vtkMRML.h file.

I first wanted to use a function pointer as stated in the previous posts but the QCoreApplication::translate has optional parameters and such approach doesn't seem to be the most suitable one.

I then create a vtkMRMLTr function that has the same signature as QCoreApplication::translate and that directly calls it to make the translation. I tested it and it works.

However, strings present in the logic classes are not always of the same type. Many of them are std::string while others are char*. I can of course convert the QString returned by QCoreApplication::translate to any of those types but my question is which one should I choose as the return type of the translation function.

I know that any choice will have impact on the source code since it may result in many string conversions at translation function calls. Also, I wonder if those types are suitable for storing translated strings without any encoding issue ?

Here are some suggested implementations :

QString vtkMRMLTr(const char *context, const char *sourceText, const char *disambiguation = nullptr, int n = -1)
{
  return QCoreApplication::translate(context, sourceText, disambiguation, n);
}

std::string vtkMRMLTr(const char *context, const char *sourceText, const char *disambiguation = nullptr, int n = -1)
{
  return QCoreApplication::translate(context, sourceText, disambiguation, n).toStdString();
}

Regards.

[jcfr]

Assuming we are focusing on translating strings from logic, what about adding virtual function vtkMRMLApplicationLocic::Translate .... then the call to QCoreApplication::translate would be done in a new class called qSlicerCoreApplicationLogic deriving from vtkSlicerApplicationLogic

[lassoan]

Thank you for working on this. It seems like you are doing nice progress.

We cannot use any Qt functions or variables in vtkMRMLCore component. That's why we need the callback function. You have figured out the solution for default argument issue (create a function that has default arguments) you just need to call a function pointer from there (you cannot call QCoreApplication::translate(...) directly).

in vtkMRMLCore (Qt classes cannot be used):

vtkMRMLTranslateFunctionPtr = nullptr;

std::string vtkMRMLTr(const char *context, const char *sourceText, const char *disambiguation = nullptr, int n = -1)
{
    if (vtkMRMLTranslateFunctionPtr)
    {
        return (*vtkMRMLTranslateFunctionPtr)(context, sourceText, disambiguation, n);
    }
    else
    {
        return sourceText;
    }
}

in Slicer application (Qt classes can be used):

std::string SlicerAppMRMLTranslate(const char *context, const char *sourceText, const char *disambiguation, int n)
{
    return QCoreApplication::translate(context, sourceText, disambiguation, n).toStdString();
}

qSlicerApplicationPrivate::init()
{
    vtkMRMLTranslateFunctionPtr = SlicerAppMRMLTranslate;
}  

Using std::string return value of vtkMRMLTr is good because it avoids pointer ownership issues. Using const char* is safer than const std::string& because setting a null pointer as input will not crash the application.

It may be better to use namespaces or class names instead of polluting the global namespace with functions and function pointers.

[lassoan]

I like @jcfr's suggestion of using virtual functions instead of global function pointers. It keeps the global namespace more tidy and syntax of virtual functions is much more readable than function pointers.

[pieper]

I agree with the virtual method approach. But I'd suggest that instead of creating a qSlicerCoreApplicationLogic class, I would think that we could have a vtkMRMLTranslator class with a virtual translate that is noop as an instance variable of vtkSlicerApplicationLogic with Get/Set methods. Then we could create a subclass qMRMLTranslator that could be set on the application's instance of vtkSlicerApplicationLogic. I think this would be easier to read and understand since the vtkSlicerApplicationLogic would remain a vtk class in all uses and the purpose of the translation code would be localized.

[jcfr]

re: vtkMRMLTranslator + create a subclass qMRMLTranslator

Great idea. And this is consistent with the existing QTranslator¹ class.

Footnotes

1. https://doc.qt.io/qt-5/qtranslator.html ↩

[lassoan]

It could be inplemented the same way as vtkOutputWindow with static GetInstance/SetInstance methods.