API Overhaul

[ljubobratovicrelja]

We have talked before how this project bears a lot of design mistakes in the API. Now that we've joined it with libmir, I'd strongly suggest we join forces and redesign the API, before going into development of new features.

Some of examples we have talked about before:

• remove dcv.core.image #64
• solid design of high level algorithm API (optical flow, rht, stereo matching etc.)
• talk about separating low level API from high level. This also mixes with the story of nothrow @nogc library without druntime - I believe we have agreed that ideally low level API should not use druntime?
• @9il's hint on global function design error : DCV API has similar issues: a function should not change params or it should be a procedure and return void. I assume you we're referring to the idea of pre-allocated buffers as input? If you are, I totally agree and think this is bad and hybrid design that has to be changed.
• latest talk on separated filters #85, and on filters of fixed size #86. Currently there's no filtering specialization in API - e.g. there's no function for sobel edges, which would be good for performance reason, instead we rely on general purpose convolution function which is slower.
• what else...?

@9il if you're interested, I'd be willing to do the heavy lifting on this task, but I'd really like you to follow through each change, and not let me repeat some of those mistakes. What do you think? Of course, anyone else willing to join revision board is more than welcome!

And of course, everyone interested in project is welcome to share his/her opinion on the matter.

[timotheecour]

would be good to have a document specifying a guidelines for all API's to conform to, to enforce consistency as much as possible. Here are a few design choices that need to be done:

• throw on error instead of bool return error code (https://github.com/libmir/dcv/issues/116)

• introduce and use common helper types wherever relevant, eg: opencv has cv::Size, cv::Rect, cv::Point + templated versions of these; dcv could use these consistently everywhere relevant; it's much harder to misuse that way and more self-documenting, eg: Slice!(Contiguous, [2], V*) gaussian(V = double)(V sigma, size_t width, size_t height) pure should be: Slice!(Contiguous, [2], V*) gaussian(V = double)(V sigma, size_t[2] dim) pure or use a struct Dim {size_t width, size_t height}

• individual parameters vs Option struct:

Slice!(Contiguous, [2], T*) radialKernel(T)(size_t radius, T foreground = 1, T background = 0)

vs

struct radialKernelOption(T){
  size_t radius;
  T foreground = 1;
  T background = 0;
}
Slice!(Contiguous, [2], T*) radialKernel(T)(radialKernelOption option);

advantage of latter: self documenting, easier to extend, can use default params for any subset of the fields, easier to pass around across function calls. Maybe it's overkill in this particular example, but useful in general

[wilzbach]

individual parameters vs Option struct:

You might like this DIP https://github.com/dlang/DIPs/pull/71 - though of course at this point it's unclear whether it will be accepted.

[ljubobratovicrelja]

Ping @timotheecour @9il @nervecenter

On @timotheecour's suggestion, I've put these points into one (bit better formulated) document on the wiki: https://github.com/libmir/dcv/wiki/RefactoringProposal

You'll notice the table with the volonteer's username to handle specified issue in this task. Currenly I've self assigned myself on each target, but feel free to deassign me if you'd like to contribute to any of these tasks. :) Also, feel free to add to the document! - I've added only what I know about this, and half of the bullet points in the docs do not come from me, so you're more then welcome to edit the post. (I've temporarily opened wiki edits for everyone)

[kyleingraham]

* @9il's hint on global function design error : _DCV API has similar issues: a function should not change params or it should be a procedure and return void._ I assume you we're referring to the idea of pre-allocated buffers as input? If you are, I totally agree and think this is bad and hybrid design that has to be changed.

Why is a hybrid design bad? I spent some time trying to understand DCV's API in color.d (e.g. rgb2gray) and came away seeing the advantages of returning a Slice and also accepting a pre-allocated buffer:

• Returning a Slice allows functions to be chained
• Accepting a buffer helps with controlling memory usage

I see in https://github.com/libmir/dcv/issues/65 that there is a desire to make DCV's API compatible with being called from languages other than D. Is that the reason a hybrid design is less desirable? Or am I missing that there is a better way to control memory usage outside of pre-allocated buffers?