Unifying code for drawing on a cairo context

Unifying code for drawing on a cairo context

[Pfrommer, Julius]

The X and the PGTK toolkits both support a cairo "glass". Their code for
drawing on a cairo context is virtually identical. Yet the
implementations have started to diverge.

As an example, x_draw_horizontal_wave and pgtk_draw_horizontal_wave are
virtually the same. But the X implementation takes a "GC" graphics
context, whereas the PGTK implementation now takes a "color" integer
argument.

https://git.savannah.gnu.org/cgit/emacs.git/tree/src/xterm.c#n5155
https://git.savannah.gnu.org/cgit/emacs.git/tree/src/pgtkterm.c#n1995

Obviously maintaining duplicate code is an anti-pattern. I'd like to
pull out common cairo functionality bit-by-bit into methods to be used
by both the X and the PGTK toolkit.

With the cairo primitives unified between X and PGTK, adding a cairo
"glass" for more toolkits should also become easier as a future option
(ns, haiku, w32).

But I'm unsure about how this fits with ongoing development and which
conventions to follow.

- Would the unified cairo function be called cairo_draw_horizontal_wave,
establishing a new cairo_ prefix convention?
- Should such functions generally take an immediate cairo_t context as
input or some "larger" struct that includes that cairo_t and can be
extended in the future (e.g. with foreground and background color
information).
- Is it okay to create the files cairoterm.h/.c or can we expect
functionality beyond "term" to be added over time as well?

Regards,
Julius

Re: Unifying code for drawing on a cairo context

[Eli Zaretskii]

> Date: Sat, 23 Apr 2022 11:06:16 +0200
> From: "Pfrommer, Julius" <julius.pfrommer@web.de>
> 
> Obviously maintaining duplicate code is an anti-pattern. I'd like to
> pull out common cairo functionality bit-by-bit into methods to be used
> by both the X and the PGTK toolkit.
> 
> With the cairo primitives unified between X and PGTK, adding a cairo
> "glass" for more toolkits should also become easier as a future option
> (ns, haiku, w32).

In general, I don't think we'd object to having such Cairo-specific
code in one place.  However, having GUI code common to all the GUI
backends in one place is more important, so if this Cairo-specific
module will make that common code less so, it would be a step
backward, IMO.  IOW, as long as the effort to have Cairo-specific code
in one place doesn't "invade" on the common code, it is welcome, I
think.

> - Would the unified cairo function be called cairo_draw_horizontal_wave,
> establishing a new cairo_ prefix convention?

Yes, why not?

> - Should such functions generally take an immediate cairo_t context as
> input or some "larger" struct that includes that cairo_t and can be
> extended in the future (e.g. with foreground and background color
> information).

It could, if that leads to a clean code that is easy to understand and
maintain.  I'm not sure why you are asking this, or what would be the
alternative.

> - Is it okay to create the files cairoterm.h/.c or can we expect
> functionality beyond "term" to be added over time as well?

Yes.  But again, as long as this doesn't make the common GUI code we
already have less common.

Thank you for your interest in Emacs.

P.S. You don't seem to have copyright assignment on file.  As I
presume this work will produce substantial changes in the code, I'd
encourage you to start your legal paperwork rolling at this time, so
that we could accept your contributions without any limitations.  If
you agree, I will send you the form to start the paperwork.

Re: Unifying code for drawing on a cairo context

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> In general, I don't think we'd object to having such Cairo-specific
> code in one place.  However, having GUI code common to all the GUI
> backends in one place is more important, so if this Cairo-specific
> module will make that common code less so, it would be a step
> backward, IMO.  IOW, as long as the effort to have Cairo-specific code
> in one place doesn't "invade" on the common code, it is welcome, I
> think.

I think that we won't be able to get the Cairo code all in one place,
without a large amount of code specific to each window system, which
defeats the point of such refactoring.

The platforms where Cairo drawing is currently used already have subtle
differences in their Cairo-related code to take care of
platform-specific quirks regarding fringes, image transforms, scaling,
etc.  There will probably be more of these quirks in the future, as we
support more and more window systems and display features (such as
device scaling) that have to behave differently depending on the window
system being used.  Moving all the Cairo code in one place would impose
a severe limitation on the ability to change the Cairo code for a single
window system, at least without the preprocessor definition mess that
this refactoring will hopefully avoid.

Further, the amount of drawing code is quite minimal, and the currently
existing duplication does not impede maintenance at all, so this is most
likely a case of premature optimization.

Re: Unifying code for drawing on a cairo context

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: "Pfrommer, Julius" <julius.pfrommer@web.de>,  emacs-devel@gnu.org
> Date: Sat, 23 Apr 2022 19:58:04 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > In general, I don't think we'd object to having such Cairo-specific
> > code in one place.  However, having GUI code common to all the GUI
> > backends in one place is more important, so if this Cairo-specific
> > module will make that common code less so, it would be a step
> > backward, IMO.  IOW, as long as the effort to have Cairo-specific code
> > in one place doesn't "invade" on the common code, it is welcome, I
> > think.
> 
> I think that we won't be able to get the Cairo code all in one place,
> without a large amount of code specific to each window system, which
> defeats the point of such refactoring.

Clearly, I was writing on the assumption that this would be possible,
without independently assessing whether in fact it is possible.

Re: Unifying code for drawing on a cairo context

[Pfrommer, Julius]

> I think that we won't be able to get the Cairo code all in one place,
> without a large amount of code specific to each window system, which
> defeats the point of such refactoring.

Not all Cairo drawing, but those parts where it makes sense.
But having some level of code-sharing will decrease the "entropy"
pulling the toolkits and platforms apart over time.

> The platforms where Cairo drawing is currently used already have subtle
> differences in their Cairo-related code to take care of
> platform-specific quirks regarding fringes, image transforms, scaling,
> etc.

Agreed, there are differences. But every Cairo operation eventually
translates to an API call on a cairo_t "glass". Some platforms already
have a "scaling-factor" as part of their Cairo drawing. Most of these
platform-dependent differences could be handled by a slim wrapper on top
of a common drawing implementation.

> Moving all the Cairo code in one place would impose
> a severe limitation on the ability to change the Cairo code for a single
> window system, at least without the preprocessor definition mess that
> this refactoring will hopefully avoid.

I would argue to the contrary. Developers typically only have access to
a subset of the platforms. Developers will improve and bugfix only those
platforms where they can test their changes. Having a common core for
Cairo-drawing (with no #ifdef mess and assuming that Cairo behaves
similarly across platforms) would reduce the duplication of the effort
to fix bugs.

Anyhow, instead of hypothesizing, I will try out a couple of approaches
to see how/whether the overall complexity is reduced by pulling out
common drawing operations on top of the cairo_t "glass".

Regards, Julius

Re: Unifying code for drawing on a cairo context

[Po Lu]

"Pfrommer, Julius" <julius.pfrommer@web.de> writes:

> Not all Cairo drawing, but those parts where it makes sense.
> But having some level of code-sharing will decrease the "entropy"
> pulling the toolkits and platforms apart over time.

There will be no benefit to this at all.  We only recently introduced a
second platform where Cairo drawing is used, and aside from GTK (and X
Windows, where using Cairo for stuff other than fonts was apparently
implemented so that we could use it for printing in the future, but
nobody has shown any interest in doing that work), there are no other
platfoms where Cairo drawing is required or would be beneficial.

So IMNSHO this refactoring is premature, and will also likely cause
problems down the road when we want to change the behavior of the Cairo
code on a platform-specific basis, such as when the device scaling
feature is implemented on X.

> I would argue to the contrary. Developers typically only have access to
> a subset of the platforms. Developers will improve and bugfix only those
> platforms where they can test their changes. Having a common core for
> Cairo-drawing (with no #ifdef mess and assuming that Cairo behaves
> similarly across platforms) would reduce the duplication of the effort
> to fix bugs.

I work on both platforms where Cairo drawing is used for something other
than fonts and see no "ifdef mess" you allude to in the Cairo code, and
so far there hasn't been a single instance of the two instances of
subtly different Cairo code causing fixing a bug to require a
duplication of effort.  Most of the preprocessor conditionals in the X
Windows drawing code stem from the presence of both a Cairo and
non-Cairo configuration, neither of which are going away.

Cairo is not used at all on NS and MS Windows, so bug fixes will have to
be made separately there in any case.  The Haiku port will also never
use Cairo for drawing anything other than text with complex shaping
requirements.

On the other hand, why not dedicate your time to some work on the PGTK
port that will lead to an immediate improvement, such as refactoring the
selection code in `pgtkselect.c' to use the low-level GDK selection API,
so that drag-and-drop will work, and most of the Lisp-level selection
code can be shared with X?

Thanks in advance.

Re: Unifying code for drawing on a cairo context

[Po Lu]

Po Lu <luangruo@yahoo.com> writes:

> There will be no benefit to this at all.  We only recently introduced
> a second platform where Cairo drawing is used, and aside from GTK (and
> X Windows, where using Cairo for stuff other than fonts was apparently
> implemented so that we could use it for printing in the future, but
> nobody has shown any interest in doing that work), there are no other
> platfoms where Cairo drawing is required or would be beneficial.

Not to mention that Cairo is in effect unmaintained, and using it is
discouraged in new software, so I wouldn't count on it being present
forever, much less design code around the assumption that we will always
keep using Cairo.

Re: Unifying code for drawing on a cairo context

[Pfrommer, Julius]

> Not to mention that Cairo is in effect unmaintained, and using it is
> discouraged in new software, so I wouldn't count on it being present
> forever, much less design code around the assumption that we will always
> keep using Cairo.

Cairo is not unmaintained. There were a couple hundred commits from
various developers since the beginning of this year.

https://github.com/freedesktop/cairo/graphs/contributors?from=2021-04-02&to=2022-04-29&type=c

> There will be no benefit to this at all.  We only recently introduced a
> second platform where Cairo drawing is used [...]

Case in point. For GTK and PGTK, developers need different setups (using
X vs. using Wayland) and the similar code-paths for drawing on cairo_t
started to diverge and continues to diverge.

Quoting Eli:
> In general, I don't think we'd object to having such Cairo-specific
> code in one place.  However, having GUI code common to all the GUI
> backends in one place is more important,

It appears that increasing the amount of code-sharing between platforms
is a goal for Emacs overall.

I understand that "developer freedom" to work on platforms independently
is important to some. Hopefully that freedom can be retained even when
the level of code-sharing across some platforms is increased.

Let's not jump to premature conclusions but see how the first
experiments for Cairo pan out.

Re: Unifying code for drawing on a cairo context

[Po Lu]

"Pfrommer, Julius" <julius.pfrommer@web.de> writes:

> Cairo is not unmaintained. There were a couple hundred commits from
> various developers since the beginning of this year.
>
> https://github.com/freedesktop/cairo/graphs/contributors?from=2021-04-02&to=2022-04-29&type=c

The Cairo developers haven't been able to put out a stable release with
fixes to major bugs in over 3 years, which is interfering with the
development of many large programs and toolkits.  Most of those commits
you allude to are either deleting unbuildable code, or fixing the tests,
and are useless because so far the maintainer(s) capable of making a
release are all missing, so the code doesn't get to anyone.

> Case in point. For GTK and PGTK, developers need different setups (using
> X vs. using Wayland) and the similar code-paths for drawing on cairo_t
> started to diverge and continues to diverge.

And that divergence has caused no problems whatsoever.

> It appears that increasing the amount of code-sharing between platforms
> is a goal for Emacs overall.

That makes sense in the general case, but as the platform-specific
maintainer for both platforms, I disagree this is a direction we want to
work towards in the Cairo code.  In contrast, using the GDK selections
API is a similar refactoring task that would be much more worth spending
your time on, and one that I hope someone will complete soon.

Re: Unifying code for drawing on a cairo context

[Eli Zaretskii]

> Date: Fri, 29 Apr 2022 10:01:01 +0200
> From: "Pfrommer, Julius" <julius.pfrommer@web.de>
> Cc: emacs-devel@gnu.org
> 
> Anyhow, instead of hypothesizing, I will try out a couple of approaches
> to see how/whether the overall complexity is reduced by pulling out
> common drawing operations on top of the cairo_t "glass".

Thanks in advance.

Re: Unifying code for drawing on a cairo context

[Po Lu]

"Pfrommer, Julius" <julius.pfrommer@web.de> writes:

> The X and the PGTK toolkits both support a cairo "glass". Their code for
> drawing on a cairo context is virtually identical. Yet the
> implementations have started to diverge.
>
> As an example, x_draw_horizontal_wave and pgtk_draw_horizontal_wave are
> virtually the same. But the X implementation takes a "GC" graphics
> context, whereas the PGTK implementation now takes a "color" integer
> argument.
>
> https://git.savannah.gnu.org/cgit/emacs.git/tree/src/xterm.c#n5155
> https://git.savannah.gnu.org/cgit/emacs.git/tree/src/pgtkterm.c#n1995
>
> Obviously maintaining duplicate code is an anti-pattern. I'd like to
> pull out common cairo functionality bit-by-bit into methods to be used
> by both the X and the PGTK toolkit.
>
> With the cairo primitives unified between X and PGTK, adding a cairo
> "glass" for more toolkits should also become easier as a future option
> (ns, haiku, w32).

They take different arguments because they operate on top of different
graphics APIs.  Graphics contexts are an important resource in the X
Windows graphics API, while GDK got rid of them in favor of specifying
colors directly.

On Haiku, Cairo is only used for fonts that have complex shaping
requirements, and it will not be used for anything else.  I don't see
what benefits Cairo drawing would bring on NS or W32 either, except for
apparently more flicker, which is so bad on Haiku that double buffering
cannot be turned off when Cairo drawing is enabled.

But at present, there is no benefit to those changes.  I'm also not
convinced we will be able to have "one-size-fits-all" Cairo code for the
different window systems that are out there: X, PGTK and Haiku already
have different methods of handling scaling, fringe bitmaps, and even
images and image transforms, and the existing Cairo code in both
terminals (and fringe.c, and some other place that I can't find right
now) is certainly not general enough to apply to every system out there.

There is nothing troublesome about maintaining the Cairo code as it is.

I would rather wait for that to become an actual problem (by my
judgement, or the judgement some other person who is actually working on
the relevant code) before trying to put all the code together.

Thanks.

To answer your other questions:

> But I'm unsure about how this fits with ongoing development and which
> conventions to follow.
>
> - Would the unified cairo function be called cairo_draw_horizontal_wave,
> establishing a new cairo_ prefix convention?

No, it should be shorter, something like `cr_draw_horizontal_wave'.

> - Should such functions generally take an immediate cairo_t context as
> input or some "larger" struct that includes that cairo_t and can be
> extended in the future (e.g. with foreground and background color
> information).

It would ideally take an Emacs_GC and a cairo context, and use the
foreground color of that GC, except on the window systems where there is
no concept of a GC (namely Haiku), in which case it should take the
system native color type (unsigned long on Haiku.)

> - Is it okay to create the files cairoterm.h/.c or can we expect
> functionality beyond "term" to be added over time as well?

Just call it 'crfns.c', which describes its functionality much better,
since it doesn't read input or write to a terminal.