-
Notifications
You must be signed in to change notification settings - Fork 18
RefactoringProposal
These guidelines correspond to ongoing conversations originating in issues #87 #64 etc.
As previously discussed, major refactoring bullet points would be:
- removal of the dcv.core.image.Image class from the API
- solid design form of high level algorithms in the library
- introduction of common helper types (e.g. Size, Point, Rectangle, etc.)
- strongly formulated error handling techniques used thought the library
- introduce
optionparameter over individual parameters
| dcv.core.image.Image | solid API | Common Types | Error Handling | Option Params | |
|---|---|---|---|---|---|
| contributor | @rljubobratovic | @rljubobratovic | @rljubobratovic | @rljubobratovic | @rljubobratovic |
| relevant PRs | n/a | n/a | n/a | n/a | n/a |
| status | n/a | n/a | n/a | n/a | n/a |
Motivation behind this point is to achieve more type safe and self-documenting code,
and potentially to avoid runtime overhead introduced by using runtime info on
data type contained in the Image container. Also, the existence of such type in
the library API introduces ambiguity of the “First Class Type” in the library.
Code that would probably mostly be affected by this change is dcv.io package. Due
to the runtime-morphable data type, dcv client code doesn’t need to define the expected
type of the image to be read:
import dcv.core.image;
import dcv.io.image;
Image image = imread("/path/to/the/image.ext");
BitDepth depth = image.depth(); // bit-depth of the image
ImageFormat fomat = image.format(); // rgb, bgr etc.
size_t channelCount = image.channels();
// After the runtime inspection of the image format, slice it
// to the mir.ndslice.Slice of given type, and start the real
// work.
Slice!(Contiguous, [3], ubyte*) imageData = image.sliced!ubyte;From this it can be concluded that Image type is used merely as convenient data
and metadata holder for the image IO. But, being formulated as nearly complete image
representation type, it can be confusing for users that this class should be used
throughout the library instead of mir.ndslice.Slice.
If Image is to be removed, Slice would probably become the data reciever type,
and imread should be templated:
// Read RGB image as 32-bit (floating point)
Slice!(Contiguous, [3], float*) image = imread!(float, ImageFormat.RGB)("/path/to/image.ext");Or we could introduce Metadata struct, as runtime info holder on the image, and store
the image data into a “default” ubyte return value slice:
struct Metadata { ... }
Slice!(Contiguous, [3], ubyte*) imread(in string path, out Metadata metadata);
Metadata metadata;
Slice!(Contiguous, [3], ubyte*) image = imread("/path/to/image.ext", metadata);
BitDepth depth = metadata.depth(); // bit-depth of the image
ImageFormat fomat = metadata.format(); // rgb, bgr etc.
size_t channelCount = metadata.channels();
// And other image metadata (EXIF)
uint sensorWidth = metadata["sensorWidth"];After the interpretation of the metadata, user can cast (reinterpret) and reshape
the returned Slice value to fit the image actual format. This variant is obviously
much less safe, but offers the same functionality as the current solution with the Image.
This point implies the need for more extensible code in the library. If rules of the API design are well defined, users are much easier accustomed to it. It is also much easier for potential contributors to build new modules in the library (such as Feature Matching, Optical Flow, Stereo etc.).
Present design can be observed in following packages:
- dcv.tracking.opticalflow
- dcv.multiview.stereo
- dcv.features.corner.fast
- dcv.features.rht
Most of these packages are designed in OOP style, except the rht module. Random Hough Transform
algorithms are implemented by @DmitryOlshansky. He has utilized template mixin feature to implement
overall API and most of the repeated code in it (dcv.features.rht.BaseRht), where the special (derived) code
is implemented in the actual API structs - RhtLines, RhtCircles etc.
So, options are:
- OOP
-
template mixinbasis - what else?
The idea is to implement common types used throughout the library - Point, Size, Rectangle etc.
to boost type safety and self-documentation of the code:
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}
// and have a
Slice!(Contiguous, [2], V*) gaussian(V = double)(V sigma, Dim dim) pureThe inspiration of this bullet point comes from the imwrite function:
bool imwrite(in string path, size_t width, size_t height, ImageFormat format, BitDepth depth, ubyte[] data);The idea is to avoid C-ish error handling with bool and int return types, and rather use “modern” solutions.
@timotheecour suggests we should not run from the exception handling, especially for non-@nogc, and not performant
critical calls.
There’s maybe one more option to be considered: using the Optional values (e.g. Alexandescu’s Expected,
or std.typecons.Nullable from Phobos). This strategy would allow unified error handling for all code -
non-@nogc and @nogc alike.
Idea is to wrap multiple function parameters into (option) packs. Example given by @timotheecour:
struct radialKernelOption(T){
size_t radius;
T foreground = 1;
T background = 0;
}
Slice!(Contiguous, [2], T*) radialKernel(T)(radialKernelOption option);Again, this empowers the self-documentation of the code, but also is much easier to extend without the API breakage.
This strategy could be utilized practically anywhere and everywhere in the library.