Refactor Backend Indirection?

[elcritch]

Looking through the backend implementation it seems like using case statements would make adding more backends difficult. It also seems like it'd be harder to conditionally compile only certain backends.

Would you be open to a refactored backend using OO methods:

type
  GingerBackend = ref object of RootObj
  EmptyBackend = ref object of GingerBackend

  CairoBackend = ref object of GingerBackend
      cCanvas*: PSurface
      ctx*: PContext
      created*: bool # if surface was created

  BImage* = object
    fname*: string
    width*: int
    height*: int
    ftype*: FileTypeKind
    backend: GingerBackend


method drawLineImpl*(engine: GingerBackend, 
                 img: var BImage;
                 start, stop: Point,
                 style: Style,
                 rotateAngle: Option[(float, Point)] = none[(float, Point)]()
) {.base.} =
  debugecho "WARNING: `drawLine` of dummy backend is being called!"

...

proc drawLineImpl*(engine: GingerBackend, 
                 img: var BImage;
                 start, stop: Point,
                 style: Style,
                 rotateAngle: Option[(float, Point)] = none[(float, Point)]()
  drawLineImpl(img.backend, img, start, stop, style, rotatAngle)


proc initBImage*(filename: string,
                 width, height: int,
                 backend: BackendKind,
                 fType: FiletypeKind,
                 texOptions = TeXOptions()): BImage =
    case backend
    of bkCairo:
      when not defined(noCairo):
        result = newCairoBackend(filename, width, height, fType)
      else:
        discard
    ...

[Vindaar]

it seems like using case statements would make adding more backends difficult

Well, I see your point, but in practice given the very few number of procedures a backend provides, it's really not a lot of work to write those O(5) lines. At the same time I'm really not a fan of OO stuff.

That doesn't mean I'm not open to refactoring it, especially to optionally enable / disable a backend that won't be used. But I'd probably simply make BImage a generic. That way we just modify the draw procedure (in ginger.nim) to:

  proc draw*(view: Viewport, filename: string, texOptions: TeXOptions = TeXOptions()) =
    ## draws the given viewport and all its children and stores it in the
    ## file `filename`
    let fType = parseFilename(filename)
    let backend = fType.toBackend(texOptions)
    case backend
    of bkDummy: ...
    of bkCairo: 
      var img = CairoBackend.initBImage(
        filename,
        width = view.wImg.val.round.int, height = view.hImg.val.round.int,
        ftype = fType
      )
      img.draw(view)
      img.destroy()
    of bkTikZ:
      var img = TikZBackend.initBImage( ... )
      # draw & cleanup
    of bkPixie:
      ...
    ...

Here CairoBackend etc. are just objects that contain the fields currently living in the variant branches. The signatures of the backend specific draw* procedures just gets an additional typedesc[Cairo/TikZ/...Backend] argument as a means to resolve to the correct procedure.

That way we:

• don't have to deal with methods etc.
• only have a single case in the draw procedure
• get rid of the wrapper draw procedures in main backends.nim
• achieve conditional compilation by adding a single when defined or similar line in the draw case branch for the disabled / enabled backends. The only thing I need to think about is how we might achieve erroring out at CT if a non-compiled branch were used (solutions to this depend a bit on how in the future different backends for the same use case will be handled in ggplotnim / by users of ginger, e.g. pixie and cairo both giving you the ability to generate PNG plots. How will that choice be handled? By deciding that we can then better think about CT safety)

[elcritch]

Here CairoBackend etc. are just objects that contain the fields currently living in the variant branches. The signatures of the backend specific draw* procedures just gets an additional typedesc[Cairo/TikZ/...Backend] argument as a means to resolve to the correct procedure.

That'd work well. I was a bit torn between generics or the OO approach, but wasn't sure if adding generics to the draw* methods would break the API.

Reading ginger.nim I see that the draw functions aren't even exposed. Refactoring would simplify a good portion of the codebase IMHO.

Though generics could make some of the code larger if procs like drawPoint had to be duplicated for each backend type and you wanted to keep them all. That's where the OO/methods would be nice, especially for having runtime indirection. Though OO isn't my favorite either, at lease Nim's are lightweight.

• get rid of the wrapper draw procedures in main backends.nim

That'd be handy. Also, it could enable extended API's for some backends. There's often the case of "but this one backend needs a special proc" that OO APIs prohibit adding.

• achieve conditional compilation by adding a single when defined or similar line in the draw case branch for the disabled / enabled backends.

That wouldn't be too tricky.

The only thing I need to think about is how we might achieve erroring out at CT if a non-compiled branch were used

For direct usage of Ginger, maybe something like:

proc drawRectangle*[B: TikZBackend](img: var BImage[B], left, ...) =
  when not compileTikZBackend:
    error("TikZ not enabled")
  ...

(solutions to this depend a bit on how in the future different backends for the same use case will be handled in ggplotnim / by users of ginger, e.g. pixie and cairo both giving you the ability to generate PNG plots. How will that choice be handled? By deciding that we can then better think about CT safety)

I'm not sure I completely follow. Though it sort of makes sense.

It looks like there might be room to rework the backends a bit so it'd be more of a choice based on how an img object was initialized. The generics would help in that case, maybe the toBackend proc also be generic, then it could be setup to choose... Maybe.

I'll read through the code a bit more to see what might make more sense. Especially wanting to switch at runtime between different backends seems a bit trickier, but compile time would be configuration would need some sort of sanity checking too.

[curioussavage]

I would really appreciate any change that makes it easier to compile without unwanted backends. Right now I'm just commenting out all the Cairo stuff in my fork.

On a related note before I switched to ggplotnim I was using the plotters rust lib and It was really nice that backends are implemented in their own modules. I realize that would be a bigger initial change but it could ultimately lighten the maintenance load in the end as well as totally sidestep the issue of compiling out backends. Also makes redundant backends a non issue.

On the topic of simplifying, I also realized that I may want the GraphObjects passed to the backend draw procs so I can do stuff like batching backend draw calls.

[Vindaar]

Oh, interesting. That sounds like you use ggplotnim quite a bit then? Which backend are you mainly using though?

On the topic of simplifying, I also realized that I may want the GraphObjects passed to the backend draw procs so I can do stuff like batching backend draw calls.

While that could be useful, it would be a rather big change, as it would turn the backends from being ultra primitive to containing a lot of duplicate code. E.g. how points / markers are currently first turned into primitive objects depending on the MarkerKind would need to be handled in the backends (and thus duplicated) then.

If you have any good ideas, I'm open to hear them though!

[Vindaar]

#42 should address this mostly. There's still a RT ⇔ CT mapping going on in ginger now to make the transition in ggplotnim easier. Enabling / disabling backends can now be done via the -d:use<Backend>=true|false command line options (which are by default added in a project nim.cfg).