Actually override FFI methods in renderer system?

[aartaka]

Our FFI design relies on renderer class inheritance for method specialization. So renderer-buffer is a superclass of buffer, allowing one to specialize methods on renderer-buffer to add some renderer-specific logic. The problem is: regular classes, being subclasses of renderer ones, are more specific and thus their methods override the renderer-specific methods. Even though it should've happened the other way around.

Here's one example:

• Invoking zooming commands modifies buffer zoom settings.
• This setting modification triggers (setf ffi-buffer-zoom-level) method.
• This method tries to dispatch over the current buffer.
  • If it's a regular buffer, use fallback JavaScript-based zooming.
  • If it's a gtk-buffer, use WebKit-provided scrolling.
• Here's when it breaks: buffer is inherited from gtk-buffer and thus is more specific.
• So JavaScript-invoking method is picked instead of FFI one.
• And thus the zooming is slightly broken, because JS-based zooming is somewhat broken by design.

One option for fixing it is making our code a bit dirtier with actual renderer-specific overrides. Even though we make our FFI architecture harder to grasp, it ensures that we actually override the necessary methods. In case of example above, we can do:

(defmethod (setf current-zoom-ratio) (value (buffer document-buffer))
  (when (plusp value)
    (setf (webkit:webkit-web-view-zoom-level (gtk-object buffer)) value)))

in renderer/gtk.lisp. This will override the method from buffer.lisp, which is bad. But it'll also ensure that only the renderer-specific methods are called. And it's also quite Lispy—monkey-patching the imperfect behavior to make it better 😛

Originally posted by @aartaka in #3250 (comment)

[aadcg]

Your observation is correct: if a method exists in both foreign-interface and one of the renderers, then the most specific one (the one from foreign-interface) prevails.

This follows from the fact that buffer inherits from renderer-specific-buffer, as you noted. We can ask: "could it have been the other way around?". Maybe, but we have invested in the current design and it has been serving us well.

Therefore, the conclusion to draw seems to be: foreign-interface must only define methods in case the implementation serves all renderers. In other words, we can either (1) delete the method in gtk.lisp and accept that ffi-buffer-zoom-level is implemented via JS and that's how it will work for all renderers, or (2) delete that method in foreign-interface.

Option 1 should guarantee an uniform cross-renderer behavior.

[aartaka]

This follows from the fact that buffer inherits from renderer-specific-buffer, as you noted. We can ask: "could it have been the other way around?". Maybe, but we have invested in the current design and it has been serving us well.

@jmercouris expressed that foreign interface design is imperfect. So maybe we can make it better.

Therefore, the conclusion to draw seems to be: foreign-interface must only define methods in case the implementation serves all renderers.

Not necessarily. See below.

In other words, we can either (1) delete the method in gtk.lisp and accept that ffi-buffer-zoom-level is implemented via JS and that's how it will work for all renderers, or (2) delete that method in foreign-interface.

Or (3, suggested in the top post) override the commands/interfaces in renderer/gtk. This way, we have

• clean FFI architecture,
• sane (JS) fallbacks,
• and meningful renderer-specific behavior.

[aadcg]

So maybe we can make it better.

We can, and I have been working in it within the context of #2989.

---

The solution you proposed has the shortcomings that you have outlined in the top post. Our current goal is to have a simple foreign interface without tricks or dark corners.

[aartaka]

I mean, either option has shortcomings. It's about picking the one that's fulfilling the requirements. And the initial suggestion does that, also reducing the line count and making foreign interface less renderer-specific. My only concern about it was it's aesthetic value, and that I'm slowly accepting its minor ugliness for its effectiveness.

[aadcg]

I think that the best way to address this is to tweak :method-combination.

[aartaka]

That sounds wrong, because we're going against CLOS. Still, it's an elegant and simple solution.

[aadcg]

There is some degree of confusion in this thread and the report below attempts to clarify it.

---

The factors that determine whether a renderer-specific method or fallback one (general, for all renderers) is triggered are:

1. primary methods in CLOS run in most-specific-first order by default;
2. the inheritance relationship between renderer and non-renderer classes (i.e. either buffer inherits from <renderer>-buffer or vice-versa);
3. the method's specialized-lambda-list (i.e. ((buffer t)) and ((buffer buffer)) are radically different).

With that in mind, the two strategies below are worth of attention.

(define-ffi-generic ffi-foo (buffer)
  "..."
  ;; This strategy would fail if <renderer>-buffer would inherit from buffer.
  (:method ((buffer buffer))
    (or (call-next-method) (fallback-logic))))

(define-ffi-generic ffi-foo (buffer)
  "..."
  ;; This strategy is more lenient wrt typing.  If there is a renderer
  ;; specialization, it will be invoked instead of the fallback logic below.
  (:method ((buffer t))
    (fallback-logic)))

As of today, we follow the second option and it seems reasonable.

---

It should now be clear that, when running Nyxt with WebKitGTK, the specialized GTK method of ffi-buffer-zoom-level is being honored. Note that the corresponding method defined in foreign-interface (relying on JS) takes (buffer) as argument (i.e. ((buffer t))), meaning that the GTK method is more specific and the conclusion follows.

Nonetheless, it did trick me for a while as well.

I'm keeping the issue open since I'm making a very deep review of foreign-interface.