Callbacks support patch

Developers
2010-12-07
2012-10-31
  • Mihail Naydenov

    Mihail Naydenov - 2010-12-07

    Hello,here is a callback mechanism, which relies on the classical pass-a
    -callback-to-function, but also tries hard not to introduce new API.

    It works as fallows:
    - Have a struct (FreeImageCB) containing all pointer to functions (the same vein as FreeImageIO), and the void for user data, and all options needed for the callbacks.
    Currently the pointers are:
    void
    user - user data pointer
    bool started(void user, int module) - signals a job has started,yser is user
    data, module is (will be) an enum for all internal FreeImage funcs and also
    user's ones. Return true if should continue with the task.

    bool progress(void
    user, double progress) - signals progress in 0.0 - 1.0
    range, return true to continue, false to cancel.

    void finished(void* user, int module) - signals task finished

    Also included in FreeImageCB is 'steps' field - it is used to set the number
    of calls to progress() inside loops. Note, this is just a hint, not the actual
    progress() calls - progress() is also called manually, outside loops, and,
    some operation use many loops, not just one.

    • To use the callback for loading you pass it to the new LoadCB function(s).
      (An important note here is the new LoadCB uses a new loadEx_proc in the Plugin
      struct, which has a new param, inplace of the old void*data, - ExtraIOData -
      which packs the old 'data', the new 'callback' and any possible future extra
      load data, so this new API should last)
      The patch implements fully FIF_JPEG and FIF_TIFF callback loading.

    • And now for the good news. Once loaded the callback will be attached to the returned dib.
      From now on all other functions that implement callbacks will use the
      callback, traveling with the FIBITMAP!
      The patch fully implements callbacks within FreeImage_Rotate without changing
      its API in any way.

    • As an internal detail the patch also adds a new helper utility class (CallbackHelper) which makes it very easy to add (and maintain) callbacks within FreeImage. Here some info about it:
      In its simplest form all you have to do is declare an instance of it in the
      beginning of the function:

      FIMITMAP do_work(FIBITMAP src)
      {
      CallbackHelper CB( FreeImage_GetCallback(src), MODULE);

      // work work
      

      ....
      }

    And you already have started() and finished() emitted for you automatically!

    You can then:

    CB.reportProgress(0.25)
    

    to the user
    or

    CB.setupStepProgress(ALLSTEPS, ENDPROGRESS);
    

    To setup an auto progress report for loops:
    ALLSTEPS - the number of steps the operation will take. For instance if you
    report on every line then ALLSTEPS is height of the image.

    ENDPROGRESS - the desired value of progress when the loop ends.

    And later, within a loop (lets say for height):

    for(int y; y < height; y++ )
    {
        // work work
       CB.reportStepProgress(y);
    }
    

    And that is it! Progress will be reported at every 'steps' from the
    FreeImageCB struct, and when the loop is done the last progress reported will
    be the value of ENDPROGRESS from the setup. (Actually not the exact value,
    because of precision/float-arithmetic errors)

    As a last note besides the expected FreeImage_GetCallback/SetCallback API,
    there is also TakeCallback which returns the attached cb, but also removes it
    from the dib. This is very handy from moving a cb from dib to dib, because you
    never want two dibs with the same callback.

    I think, I have covered it all. The patch is available at
    https://sourceforge.net/tracker/?func=detail&aid=3131506&group_id=11504&atid=
    311504

    feel free to dw, test and comment (here or on the traker)
    thanks
    MihailNaydenov

    P.S included also is a small test - main.cpp

     
  • Ryan Rubley

    Ryan Rubley - 2010-12-07

    Glad to know somebody else finds my patch useful. You should have commented in
    the other thread that you used it :)

     
  • Lucian Sabo

    Lucian Sabo - 2010-12-07

    Glad to know somebody else finds my patch useful. You should have commented
    in the other thread that you used it :)

    This post is a coincidence, because I implemented your version a few days ago,
    I had not time to reply :)

     
  • Mihail Naydenov

    Mihail Naydenov - 2010-12-07

    Well, first I respect ark's solution, and I have studied it when it came out.
    I have also commented it in great detail. Now, I admit, I am no longer on the
    same opinion on all topics cover there, still the main issue still holds - its
    too advanced in concept and implementation.

    This one here is as naive as possible - you pass a struct holding pointers
    (same way you do for IO) and they are called.
    No new concepts like "critical section", no thread id-s, no map look-up, no
    context objects. The thread safeness is handled the way it is handled in
    FreeImage since day one - one dib (and cb) per thread, no data shared and your
    are done.
    Also, because of this no setup and no cleanup needed, started() and finished()
    are just for convenience to the user, you can pass NULL there and ignore.

    Now for the cosmetics - you have control here on how many loop calls to make -
    set cb.steps = 14 and there will be 14 updates from the loops no matter how
    big the image is, if the number is higher then ALLSTEPS (image height or
    blocks or whatever ) - then ALLSTEPS calls will be made. steps also has
    defaults = -1 and 0 will trigger ALLSTEPS updates.

    The other cosmetic is the notion of modules - you can have the progress
    indicator also printing the task in progress, including sub-tasks. For
    instance on jpeg with exifrotate you can have loading, then rotating printed
    if you monitor module param.

    And there is the helper class, but no need to repeat what I have already
    written.

    Lastly, the goal of the patch is to mix the best of both worlds - have simple,
    familiar (to the FreeImage API) callbacks without any new subsystems and also
    have little or no API change. You say 'involve so many changes', but we are
    talking about a single function changed. A small price to pay given the
    simplicity given.

    Ark solution is smart, I liked it a lot, but find it (way) to smart to be used
    in my code, and gladly give away a tiny API break if this will make things
    more KISS.

    Thank you
    MihailNaydenov

     
  • Ryan Rubley

    Ryan Rubley - 2010-12-07

    In the long run, what Mihail says is correct. An API change is required to do
    things the better way.

    However, for a side patch that will never make it into the main FreeImage
    code, I think not breaking the main API is more important.

    Remember that the context function is not required for my approach if you are
    doing regular single threaded applications. It is only when you code thread
    yourself, and are already aware of things like thread IDs and critical
    sections, that you even need to implement that extra bit with my approach.

    For a programmer who uses FreeImage but does not care much for the internals
    of FreeImage, KISS to me implies not breaking the API. This makes the code
    easier to use from the outside. Applying KISS to the internals of FreeImage
    and you will have to break the API and do exactly what is suggested here. This
    is how I view the differences in these approaches.

     
  • Lucian Sabo

    Lucian Sabo - 2010-12-08

    we are talking about a single function changed

    You change more files than in ark42 solution. I don't know what single
    function are your reffering (maybe LoadCB) because from what I can see there
    are numerous changes in several files. I don't say these are difficult to
    implement and surely your solution is good and extensible, but at the first
    glance, this seem a very good candidate for including into FreeImage,
    especially for the way it keeps thread safety, but as a solution which will
    not be included, it is harder to keep in sync with the official version than
    ark's solution. That is why I preffer his solution.

     
  • Mihail Naydenov

    Mihail Naydenov - 2011-04-27

    I have updated the patch with support of messages.

    The new procedure is of the form:

    typedef void (DLL_CALLCONV *FI_OnMessageProc)(void* user, const char* msg, FREE_IMAGE_MSGID msgid, ...);
    

    user is user data

    msg is the default FreeImage message

    mdgid is a new errorcode type of enum, holding all errors and warnings as ints for easy lookup.

    The function is vari-args, because we want to pass all available data to the
    handler. The ars vary based on msgid.

    The idea is to be able to ignore the ready-made c-string and build a custom
    one - using as bare min the errorcode and, if available - the args (most of
    the time the sub-libs already bake them into the string, but... well).

    The new patch impl JPEG loading with the message report (no args, native
    LibJPEG message is passed).
    The FreeImage_LoadEx* also makes use of the messages to report fopen fails
    (filename as arg - char or wchar).

    Included is an updated version of the old test.
    And
    A new test, based on Qt, which tests actual threaded usage - it loads two
    images side by side, printing progress and errors.

    Link to patch:
    https://sourceforge.net/tracker/?func=detail&aid=3131506&group_id=11504&atid=
    311504

    MihailNaydenov

     
  • Mihail Naydenov

    Mihail Naydenov - 2011-08-08

    New version.

    Support for jpg, tiff, png, bmp, exr, hdr, psd, tga (loading progress report
    for all, saving report for some, error report for almost all)

    progress report for Resize and Rotate.

    Changed FI_OnMessageProc to pass params via const void* instead of vari-args,
    because it is easer to use forwarding functions and classes.

    Internal FIException class, used for error transport.

    Updated simple test.

    Link:
    https://sourceforge.net/tracker/?func=detail&aid=3131506&group_id=11504&atid=
    311504

    MihailNaydenov

     

Log in to post a comment.