Development Guide

Contributions to viren2d are highly welcome via pull request. If you plan on adding features, please consider the following style guide, design decisions and recommendations. This ensures that your new feature can be swiftly integrated into the library.

Coding Style

The implementation should (mostly) follow the Google coding style guides for C++ and for Python.

General design decision: The Python bindings should use snake case (except for class & type names), whereas the C++ interface should use camel case.

Since the documentation is build using sphinx, the reStructuredText (reST) markup should be used to document the Python interface. Refer to the reStructuredText Primer if you need a refresher on the reST syntax.

Documentation

If you’re about to extend viren2d, you should already be familiar with its API documentation here on RTD. So the most important advice for documenting new features is: Be consistent!

Following the coding style, the interface documentation should focus on clarity and readability. There’s of course a tradeoff due to using reST markup for the RTD documentation.

New features must be included in the API documentation.

Additionally, it might be useful to include a short example in the quickstart tutorial.

Implementation

The following C++/Python guidelines summarize the most important implementation tasks (besides implementing the actual functionality). Please also familiarize yourself with the library layout explained below.

C++ Library

Warning

TODO order this list

  • For each custom type (where applicable), add a c’tor using std::initializer_list for less cluttered & more convenient use.

  • All drawing functions should shift the user-given coordinates by 0.5 if applicable, to support sharp lines. For details refer to the corresponding Cairo FAQ.

  • Each drawing function should call helpers::CheckCanvas and if possible, reuse the implemented sanity checks, e.g. helpers::CheckLineStyle. Of course, implement additional sanity checks as needed.

  • Drawing functions won’t be tested via mocking, because mocking the Cairo C interface would be a pain & I neither want to switch to cairomm nor write my own C++ wrapper.

    There are, however, tests to ensure that we can identify code changes that would break the drawing API before releasing them into the wild, e.g. check tests/test_painter.py. Besides these interface stability tests, there is currently no plan for more detailed drawing functionality testing. Instead, focus on interface clarity and/or provide easily understandable/extendable demos.

  • Any non-drawing/non-visualization related functionality should be properly tested.

    The rule-of-thumb is to implement program-logic tests (input validation, algorithmic correctness, operator overloading madness, etc.) in the C++ test suite. Usability tests (like the interface stability checks mentioned earlier) should be conducted via the python test suite.

  • TODO add task-list template for new interface functions/classes

Python Library

Warning

TODO order this list

  • When extending the Painter, keep the alphabetic order of its draw_xxx bindings to aid maintainability.

  • TODO Design choice: consistent parameter names in draw_xxx, i.e. if the method expect a LineStyle, the parameter name should be line_style.

    Benefits imho: a) using the drawing interface becomes easier (I have an XyzStyle, so the parameter will be xyz_style) b) is less likely to break the interface if we add extensions, e.g. if we choose to support an additional label somewhere, we would need an additional style parameter of type TextStyle). And then, we run into the hardest problem of CS, i.e. naming ;-)

  • All relevant interface methods, such as the draw_xxx variants of the Painter should provide a minimal code example via their docstring.

    This code example should be copy/pastable to aid the users of this library. For example, refer to the docstring of draw_line().

  • TODO rework/update the following instructions!

    How to bind a new class X

    • Test initialization, pickling, comparison, etc.

    • Declare it py::implicitly_convertible if a simple/intuitive conversion exists

    • @deprecated Implement moddef::CreateX (init from py::tuple/list/whatever)

    • All this info does not hold for ImageBuffer - which exposes a buffer view (and we need to be able to convert to/from numpy arrays)

    • support implicit casts (e.g. from tuples) – then you can also add py::pickle

    • Implement __str__ & __repr__

    • nice-to-have: operator == and !=

Intricacies, I wish I had know before:

  • Don’t ever use python keywords as names of function arguments, or users can’t re-order the inputs via kwargs, such as f(arg_x=foo, arg_a=1).

    Yes, this was “fun” (read: a pain) to debug.

    Refer to the Python documentation for a listing of the language keywords.

  • Double-check the python bindings for typos, semantic errors due to copying/pasting, etc.

    For example, due to the inherent Python design, it is perfectly legal to override existing attributes. A copy/paste error can easily lead to different Python class attributes modifying the same C++ class attribute.

    Debugging this is also not as much fun as it sounds.

Testing Environment

TODO list notes on testing (googletest, pytest)

  • C++ tests for program-logic googletest

    Manual C++ Testing Workflow
    # Recommendation: Enable color output for ctest/googletest
    # To enable this permanently, add this definition to your shell
    # configuration, e.g. ~/.bashrc
    export GTEST_COLOR=1
    
    # Build
    cd /path/to/viren2d/build
    cmake --build .
    
    ctest -j....TODO
    
  • Python tests for interface usage - to avoid/identify breaking API changes (parameter/variable naming, ordering, type conversions, etc.) pytest

    Manual Python Testing Workflow
    #TODO doc pytest
    pip install pytest
    pytest tests/test_*.py
    

Library Layout

Before diving into the layout of the code framework, note: to avoid name clashes or having to use naming schemes which use underscores, the physical C++ and Python libraries are named differently:

  • The target name of the C++ library is viren2d++, whereas the target name of the Python library is viren2d.

  • Currently, I prefer to statically link the C++ library into the consuming application. The Python bindings, however, have to be dynamic libraries.

The following subsections provide a hands-on introduction on the library layout with supplementary explanations on some design choices.

Drawing Functionality

To familiarize yourself with the library layout, let’s pick a drawing method of the Painter, e.g. draw_line().

First, look up the corresponding Python binding in src/bindings/bindings_painter.cpp. In our example, draw_line() is defined here.

You’ll note that viren2d uses an additional PainterWrapper class between the Python and C++ interface, i.e. the PainterWrapper::DrawLine defined here. This is due to a design choice: I prefer clean public interfaces, which requires pure virtual methods.

While this can be handled by pybind11 directly, it would require trampoline classes which need additional pybind11 macros & dependencies in the otherwise (rather) puristic C++ interface. The current wrapper-based solution is much cleaner in my opinion.

The PainterWrapper simply forwards each call to the public C++ Painter interface, see include/viren2d/drawing.h. Our DrawLine method is defined here.

Another design choice is that the public C++ interface should provide sane default values for optional parameters. To avoid the hazzle of potentially defining different defaults in the implementing subclasses, I use protected pure virtual DrawXXXImpl (implementation) methods. For example, the DrawLineImpl is declared here.

This abstract interface is implemented by the ImagePainter within src/drawing.cpp. Its main tasks are handling the internal Cairo context (i.e. resource allocation and cleaning up) and to export the visualization upon user request.

To aid maintainability, the actual drawing functionalities are implemented as seperate helpers. These helpers are declared in the header file src/helpers/drawing_helpers.h. Thus, the ImagePainter’s DrawXXXImpl methods (e.g. for our line example) only need to invoke the appropriate helper.

As another design choice, viren2d refrains from using external language bindings of Cairo and instead directly uses its C library. The major reason is that I had to learn about Cairo anyhow and I was too lazy to look up the corresponding idioms in cairomm. Now, those familiar with using any C library will understand, why encapsulating (read hiding) the complexity of the required wrapping code is another good reason to outsource these drawing helpers.

TL;DR, the actual drawing part of this draw_line() walkthrough is implemented here.

Recommended Readings:

In case you need to familiarize yourself with Cairo, I can recommend:

Other Functionality

TODO design decisions for pseudocolor (256 bins, RGB), etc.

Nice-to-Have

Some functional features, that I’d like to see at some time in the future:

  • Creating a collage (auto-padding, maybe aspect-aware resizing). Best option might be to reuse image overlay, i.e. initialize the canvas and simply layout the images.

  • Support switching between the image surface and cairo’s SVG surface. Summary of preliminary trials:

    • SVG version must be set explicitly to 1.2

    • Only RGB images seem to be included properly in the SVG file (empty outputs with other formats; didn’t investigate further so far)

    • Backend must be configurable via the Painter interface

    • Need to decide how the units should be best handled (SVG surface uses points, all other functionality assumes pixels).

  • Creating stereoglyphs

Some workflow-related extensions, I’d fancy:

  • Packaging and publishing on PyPI

  • Packaging with conda. cairo2 is already available via conda channels. Automate via github actions.

  • Automate the rtd_example_image generation via github workflows (upon each push, but before the RTD workflow starts building the docs)

  • Prepare github templates for PRs, issue reports, etc.

Finally, design choices I’d like to change in the very distant future (read: when hell freezes over):

  • Change ImageBuffer and Painter to follow the RAII idiom.

  • Exceptions are (mostly only) thrown if the user provides invalid inputs. While this is acceptable in Python, I’d like to change that to mostly noexcept calls, which simply log a warning/failure message and ignore invalid inputs.