Sunday, October 30, 2016

bool + messages = uiRetVal

Often you need to return whether something worked or not. If it didn't, you need to provide a message. That message may actually be a series of messages. And all users need to be able to understand it, also those that cannot read English. In short, you want to return:
  1. The status (OK/Fail)
  2. One or more uiString's if the operation failed
We see this a lot in current code:
uiString errmsg;
if ( !myobj.doIt( errmsg ) )
   uiMSG().error( errmsg );
The new way is to use a uiRetVal. It is implemented as a set of uiStrings. OK means the list is empty. It's really simple. In new interfaces I now routinely use this object, and it works really well. Above example would become:
uiRetVal rv = myobj.doIt();
if ( rv.isError() )
    uiMSG().error( uirv );
Advantages:
  • uiRetval tells you OK/Fail but also has any number of messages with it
  • You cannot 'forget' setting the error message(s)
  • It works really smooth.
Example from the trenches:

uiRetVal Well::Saver::doStore( const IOObj& ioobj ) const
{
    uiRetVal uirv;
    ConstRefMan wd = wellData();
    if ( !wd )
        return uirv; // all OK, that's uiRetVal's default

    Writer wrr( ioobj, *wd );
    const StoreReqs sreqs = curstorereqs_.getObject();
    if ( sreqs.isAll() )
    {
        if ( !wrr.put() )
            uirv.add( wrr.errMsg() );
    }
    else
    {
        if ( sreqs.includes(Trck) && !wrr.putInfoAndTrack() )
            uirv.add( wrr.errMsg() );
        if ( sreqs.includes(D2T) )
        {
            if ( !wrr.putD2T() )
                uirv.add( wrr.errMsg() );
            else if ( wd->haveCheckShotModel() && !wrr.putCSMdl() )
                uirv.add( wrr.errMsg() );
        }
        if ( sreqs.includes(Mrkrs) && !wrr.putMarkers() )
            uirv.add( wrr.errMsg() );
        if ( sreqs.includes(Logs) && !wrr.putLogs() )
            uirv.add( wrr.errMsg() );
        if ( sreqs.includes(DispProps) && !wrr.putDispProps() )
            uirv.add( wrr.errMsg() );
    }

    curstorereqs_.getObject() = StoreReqs::All();
    return uirv;
}

Monday, September 19, 2016

Multiple instances of template classes on Windows

A common problem on Windows is that the linker complains over multiple instances of classes, most often template classes.
The reason for this is that normally, template classes are instantiated where they are used, and their binary code is not exported, i.e. they are not included in the content-list of the library they are baked into.

On Windows, an exported class is required to have all functions compiled and ready in the  library itself, including functions of inherited classes. All these functions are also exported, i.e. added to the content list of the library. Consider the following code(s):


In library Basic (a.h)
template <class T>
class A
{
    public:
                    A();
    int             compute() const;
};

As this is a template class, it is not exported, and no instances are added to the Basic.dll's list of contents.

In library "General" (b.h)

mExpClass(General) B : public A<int>
{
    ...
};

Here the compiler will export all functions in B and all functions in A<int>. This means that A<int>::compute() will be present on General.dll's list of contents. So far so good. The problem does however come if another library does the same:


In library "Well" (c.h)

mExpClass(General) C : public A<int>
{
    ...
};

As the compiler does not know that A<int> is already available in "General", it will instantiate all functions of A<int> into Well.dll just as when the "General" library was compiled. Hence, the linker will complain that there are two versions available, the one in Well.dll and the one from General.dll.

In OpendTect, we have solved this problem by explicitly telling that template classes that are instantiated in each library. This is done in the *export.h files. Everyone who is using a class from say the "General" library will include one or more headers from the General library. These files in turn include the "generalmod.h" header, which in turn includes the "generalexport.h" header.

generalexport.h looks like this:

#ifdef do_import_export
# include <a.h>

mExportTemplClassInst( General ) A<int>;

#endif

Hence, everyone who uses any class from General, will include this. The two macros essentially tell every user of General that A<int> is available in the "General" library. In the example above, the compiler of the class "C" would not create a new instance of A<int> as it would know that it is already done in the "General" library.

If you get problems with multiple instances, figure out in which module the template class is instantiated first, and add it to the export header of that module.

Thursday, September 15, 2016

Use of #pragma once

Fellow developers,

a short note that I have just committed a large change where we go from using double-include protection using

#ifndef filename_h
#define filename_h

...

#endif

But using the simpler, and less error prone

#pragma once

in the top of the file. The ifdef in the end of the file goes away.

Cheers,


Kristofer

Friday, May 20, 2016

Pick:Set's and their Manager

Well, I did it. Just like that, I committed weeks of work to make Pick::Set's (more) MT-safe, and unify the I/O handling of them. What I did wouldn't have been possible without Kris' work with the new RefMan<T> object handling. Here goes.



Monitorable


First of all, Pick::Set's are now Monitorable. Such an object promises to be MT-compliant, and notify changes in its state. That means, for example, multi-read locking. So no public data members or even references to internal, everything is done via get and set functions. For 'loose' Set's, unstored just for work, everything stays the same - albeit that you can no longer access internals directly. Oh and they have to be created like:
RefMan<Pick::Set> ps = new Pick::Set;
Because of the ref counting there is no longer a destructor. Small cost for big upside; and you'll get used to it.


The SetMGR


Then for everything stored, there is a new Pick::SetMGR() object. You ask it for a set with a set ID (the IOObj's MultiID), and it will make sure it's loaded. Like in:
uiString errmsg; 
ConstRefMan<Pick::Set> ps = Pick::SetMGR().fetch( id, errmsg ); 
if ( !errmsg.isEmpty() ) 
    { errmsg_ = errmsg; return false; }
The Set you are looking for may already be in memory, but it may also be read from disk. All you know is you get a ref, and other objects may also be looking at the same set. Or even, they may be changing it.


Monitoring a Set


If I want to know about some change, then:
mAttachCB( ps->objectChanged(), MyObj::pickSetChgCB );
By unpacking the 'Capsule' in the callback you can figure out what actually happened - but that's the topic of some other post. The thing is, the Pick::SetMGR() is not the keeper of events, like the old Pick::Mgr() was. Consequently, the new SetMGR is all about storing and sharing. Almost like a real OO database.
For example, all Set's, being derived from Monitorable, have a dirtyCount(). The SetMGR keeps special objects called Pick::SetSaver that keep track of the last saved dirty count so they know whether a Set was changed since it was added, read, or last saved.
If your stuff needs to be MT-ready, you'll want some kind of lock to prevent others from changing the Set while you are iterating over it. That is there, it's called a MonitorLock. Typically, one would do:
MonitorLock ml( *ps ); 
for ( int idx=0; idxsize(); idx++ ) 
{ 
    const Coord pos = ps->getPos( idx ); 
    // etc. 
} 
ml.unlockNow();
One thing: do not try to change the set while iterating. That is, unless you want to study deadlocks. Editing a set while having a MonitorLock on is simply not possible. Collect the changes may be an (MT-unsafe) idea. Or make a copy, and edit that. Then to finish copy back:
RefMan<Pick::Set> editedset = new Pick::Set( *ps ); 
doChanges( *editedset ); 
*ps = *editedset;


New Sets


To add a set, you could do something like:
RefMan<Pick::Set> ps = new Pick::Set;
fillPS( *ps );
uiString errmsg = Pick::SetMGR().store( *ps, id );
if ( !errmsg.isEmpty() )
    { errmsg_ = errmsg; return false; }
So, how do you get an ID? Well, quite possibly from the uiPickSetIOObjSel in uipicksettools.h. If you want no user interaction, you can still just give the Set a good name and store it with that:
uiString errmsg = Pick::SetMGR().store( *ps );
That will work, but you may just overwrite something, maybe something precious that another object is working with. Try:
if ( Pick::SetMGR().nameExists(ps->name()) )
{
    // Hmmm ... what now?
}


What's next?


On my TODO list is now to bring back undo handling (better than it was) and, the experienced MVC people probably guessed, burst event handling. My plan is to do that via a new object called ChangeNotifyBlocker combined with 'Entire object changed' change data.
And then ... we may have a template for other objects, to make them MT-ready, sharable, monitorable, tranparently stored. Wavelets seem a (rather simple) candidate, much more difficult but probably very rewarding may be wells.

Thursday, April 21, 2016

Effective Enum Handling in UI

Enums in OpendTect have three different aspects:
  1. The enum value
  2. The Key string (mostly used in IOPar, also going to text files)
  3. The UiString as shown in the user interface, say in a uiComboBox.

To enable a convenient transformation from one aspect to another we have the class EnumDef which manifests itself in a derived template class  EnumDefImpl<ENUM>. For each enum type, the corresponding EnumDef should be declared in the header file using the macro mDeclareEnumUtils or mDeclareNameSpaceEnumUtils. See enums.h for details. For example:

class MyClass
{
    enum State    { Good, Bad, OK, Undef };
                   mDeclareEnumUtils(State)


Then in the source file we need to define the keys using the macro mDefineEnumUtils:

 mDefineEnumUtils(MyClass,State,"My class state")
    { "Good", "Bad", "OK", "Undefined", 0 }

The strings above are the keys that would be useful in an IOPar or while writing to a file. By default the same strings will also be used in the UI. But if you want to use a different set of strings in UI, you can optionally define the uiStrings:

template <>
void EnumDefImpl<MyClass::State>::init()

{
    uistrings_ += tr("Lovely");
    uistrings_ += tr("Terrible");
    uistrings_ += toUiString("OK");
    uistrings_ += tr("Not sure");  
}

To use them in the UI, you can directly pass the EnumDef to classes like uiComboBox and uiStringListInpSpec. To set the value you can use:

combobox_->setCurrentItem( MyClass::StateDef().indexOf(oldval) );

And to get the value from the UI as enum you can do:

MyClass::State inp = MyClass::StateDef().getEnumValForIndex(
                                combobox_->currentItem() );

While reading from or writing to an IOPar, you should always use the enum key and the functions:

    EnumDef::indexOf(const char*)
    EnumDef::getKeyForIndex(int)

If you want more customization for a particular UI class, you can make your own copy of the EnumDef and make modifications like changing the uiStrings, removing some of the enums etc. For example see uiprestackattrib.cc.

The bottom line is, we have a lot of tools available, but to keep the code safe we need to take care of the following:
  • Always use the enum type (and not their int value) as class members or function arguments.
  • In IOPar or text files always write the key strings (not the int values or strings derived from uiStrings).
  • For existing enum types you can modify the uiStrings but never change the key strings if you want to maintain any backward compatibility with old files.


Monday, April 4, 2016

Changes to the DataPack and DataPackMgr classes

Following up on last week's blog, we are going further with re-engineering of some basic classes in preparation for the next major release.

To ensure thread safety, the DataPack classes are receiving an overhaul. The largest change is that the DataPack class is now reference counted, sharing the code with the rest of all reference counted objects. This ensures thread safety and a central knowledge about reference counting.

The major changes are:
  • DataPack::obtain() is replaced by ref().
  • DataPack::release() is replaced by unRef()
  • DataPackMgr::addAndObtain() going away. Your code should instead simply call DataPackMgr::add(). Care should be taken that you reference your object apropriately, as add() only makes the DataPackMgr aware of the object, it does not reference it.
  • DataPackMgr::obtain() is replaced by DataPackMgr::get() to get the object.
  • DataPackMgr::release() should be replace by calling either DataPackMgr::unRef(ID), or calling the unRef() on the object itself.
The old functionality still works as before, but we are phasing out its usage over the coming weeks. For the next major release, these functions will be marked as obsolete, and eventually be removed.

There is also new, nice functionality in there, such as DataPackMgr::getAndCast<T>( id ). By using it the following code:

RefMan<PreStack::Gather> = 0;
DataPack* dp = DPM(DataPackMgr::FlatID).obtain( id );
if ( dp )
{
    mDynamicCast( PreStack::Gather*, gather, dp );
    if ( !gather ) DPM(DataPackMgr::FlatID).release( dp );
}

can be replaced by:

RefMan<PreStack::Gather> gather 
        = DPM(DataPackMgr::FlatID).getAndCast<PreStack::Gather>(id);

As part of these changes, we are also investigating the thread safety at various places. So you can expect some API changes. For example, we are more and more using RefMan<T> rather than bare pointers.

Friday, April 1, 2016

Weak pointers for reference counted objects

OpendTect has used reference counting for more than ten years in the visualization and the earth model libraries. We are now taking this one step further and introducing a weak pointer for reference-counted objects. In order to do so, we had to change the way reference counted objects work. Previously, our reference-counted objects used a macro which implemented the necessary functions. That has now been replaced by inheritance:

class A : public RefCount::Referenced
{
protected:
             ~A();
};

The good thing is that apart from this, the API remains the same, so no changes are needed by classes using reference counted classes.

This change enables us to do two things:

  1. Use RefMan<A> as class variables with a forward declared A. Previously, the compiler required a full definition of the A variable. Hence we can include less files and get slimmer dependency trees:

    #include <refcount.h>
    class A; //Forward declaration of A

    class B
    {
    public:
                    B();
    private:
        RefMan<A>   a_;
    };
  2. Use weak pointers. Weak pointers point to objects as long as objects are alive, but become NULL when the object they point to goes out of scope:

    #include <refcount.h>

    class A;

    class B 
    {
    public:
                    B(A* a) : a_(a) {}    void        compute();
    private:
        WeakPtr<A>  a_;
    };

    The variable a_ will be a valid pointer to the instance of A as long as the instance is alive. When the instance goes out of scope, it will be set to NULL. If you actually want to use the pointer in a_, you have to retrieve it using the WeakPtr<A>::get() function:

    void B::compute()
    {
        RefMan<A> a = a_.get();
        //Instance is reffed if still alive

        if ( !a ) //Check if it is alive
             return;

        a->doSomething();
    } //a goes out of scope, and A instance is unreffed
All reference counting is threadsafe, as it is managed by atomic variables. The new functionality is available in our master branch. It can unfortunately not be ported to the stable branch due to ABI restrictions.

Wednesday, March 2, 2016

DataPack tracking

DataPacks need to be obtained and released. If you suspect some code (of course not anything made by you!) is not doing it right, then you can now enable tracking, which will go to the debug log (stderr on Linux). It is also usable in release mode (on-site at clients), in which case it will go to the normal log file.

To enable, set the environment variable OD_TRACK_DATAPACKS to yes.

This is the output of creating one inline with default data, and immediately removing it:

[DP]: add 4 '4 Dip steered median filter'
[DP]: obtain 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: obtain 4 nrusers=2
[DP]: release 4 nrusers=1
[DP]: release/delete 4

Tuesday, March 1, 2016

Keep virtual!

I've just made some code changes to code in which rather consistently this is happening:

class Base
{
   virtual void func();
};

class Derived : public Base
{
    void func();
};

What is the consequence of not keeping the virtual keyword in Derived? Well, for the compiler, nothing. It's OK. The derived class' func() will be virtual no matter whether you declare it as such.

But code is not only for the C++ compiler. I prefer to keep the virtual keyword in Derived, By repeating the virtual keyword you say something to the reader of the class' interface: hey, take a look at the base class, I'm implementing (or overruling) an existing interface here. Note that C++11 has 'override' to help you with this.

There's also potential human suffering. If Derived is later derived from, then using func() in the constructor is a dangerous thing (virtual functions and constructors - scary combo). Hopefully the creater of that new class will take a peek at base.h early to discover this before having to debug this away ....

Monday, February 1, 2016

Building on Linux



I just built OpendTect 6.0 from source on my kUbuntu 15.10 system (64 bits). It was so easy ...
svn co https://github.com/opendtect/opendtect.git/branches/od6.0 ./od6.0
Installed openscenegraph (already had cmake) from the package manager (turns out to be 3.2.1), then:
cd od6.0; cmake-gui .
Set QTDIR to /usr/lib/x86_64-linux-gnu/qt4 (just leave OSG_DIR empty). Configure, Generate. Now make. Got 8 processors with 16 threads, so let's rock:
make -j12
6 minutes later: Done.
cd bin/lux64/Debug; ./od_main

If all you want is to use the free OD base then this may be faster than using the installation manager :)

Friday, January 29, 2016

Platform consideration for next major OpendTect version

Though we have just released OpendTect v6 and OpendTect Pro, we are already looking ahead for the next version of OpendTect. Already now, we have made platform decisions, so we can start develop under the platforms we are going to use.

Our next major release will (unless otherwise is communicated later) be based on:

  • Linux: Built on Centos 6 (Redhat 6.0), 64 bit only.
  • Windows: Visual Studio 2013, 64 bits for sure, but 32 bits may be dropped depending on installation base. Currently windows 32 has about 10% of all installations.
  • MacOSX: Built on MacOSX 10.8.
With regards to dependent libraries, we will be on Qt5 of some version. We are now using Qt 5.5.1 internally, but if there are stable releases closer to the actual release, we may go for a later version. OpenSceneGraph will be used at the latest, stable release when we come close to our release. Currently, we use OpenSceneGraph version 3.4.