bug#35871: 27.0.50; [PATCH] Fix SVG transparency with Cairo

[Kévin Le Gouguec]

Hello,

While trying out the Cairo build, I noticed that some SVG images did
not look as smooth as in the "regular" build.  More specifically,
semi-transparent parts of SVG images (when the alpha value is neither
0% nor 100%) did not seem to be displayed properly.

This is especially noticeable on the splash screen (see attached
screenshot) and when starting Gnus.

cairo-svg.png
Description: PNG image

Each window in the screenshot was obtained with:

- ./src/emacs -Q -rv,
- C-h C-a (for some reason the splash screen wasn't displayed at
  startup)
- C-x 2, moving to the *scratch* buffer, and displaying
  - system-configuration-options,
  - system-configuration-features,
  - emacs-repository-version.

I could reproduce this on OpenSUSE Tumbleweed, Debian stretch and
Xubuntu 16.04.

I looked at the SVG section in src/image.c, in particular at any
USE_CAIRO section; I stumbled on this bit:

> if (iconptr[3] == 0)
>   *dataptr = bgcolor;
> else
>   *dataptr = (iconptr[0] << 16)
>     | (iconptr[1] << 8)
>     | iconptr[2]
>     | (iconptr[3] << 24);

Looking at create_cairo_image_surface, I saw that we use
CAIRO_FORMAT_ARGB32, which according to the documentation works with
"premultiplied alpha" values.

Quoting 
<https://www.cairographics.org/manual/cairo-Image-Surfaces.html#cairo-format-t>:

> CAIRO_FORMAT_ARGB32      each pixel is a 32-bit quantity, with alpha
>                          in the upper 8 bits, then red, then green,
>                          then blue. The 32-bit quantities are stored
>                          native-endian. Pre-multiplied alpha is
>                          used. (That is, 50% transparent red is
>                          0x80800000, not 0x80ff0000.) (Since 1.0)

AFAICU, this means that Cairo expects the RGB values to be scaled down
according to the alpha value.  Having no idea whether this was the issue
(I had no idea whether gdk-pixbuf gave us premultiplied alpha or not), I
added that multiplication and AFAICT, this does solve the issue (see
other screenshot).

cairo-svg-fixed.png
Description: PNG image

NB: the commit named "a7c9365f7956d9d7a089a2f161d2b9d06fc91d58" in the
left screenshot is the one which contains my patch, attached here:

0001-Fix-transparency-handling-in-SVG-images-with-Cairo.patch
Description: Text Data

Here is a before-after-patch screenshot with a grey background, which I
find makes the problem more noticeable:

cairo-svg-fixed-personal-config.png
Description: PNG image

Looking for references afterward, I found this bit about Cairo
integration in GTK+ in the er… GNOME Wiki Attic?

https://wiki.gnome.org/Attic/GtkCairoIntegration

> gdk-pixbuf only handles the pixel formats packed RGB and RGBA. Both of
> these formats are inefficient memory and performance wise.
>
> […]
>
> 2. Most Xservers uses the color format ARGB so to blit a GdkPixbuf you
>    need to convert each pixel from RGBA to ARGB which means that you are
>    taking an unnecessary performance hit. It is even worse if you are
>    blitting pixbufs using Cairo because then you have to convert from
>    premultiplied to non-premultiplied alpha.

See also the note on "Pixel formats" from a GNOME developer:

https://people.gnome.org/~federico/blog/my-gdk-pixbuf-braindump.html

> Every time we paint a GdkPixbuf to a cairo_t, there is a conversion
> from unpremultiplied RGBA to premultiplied, platform-endian ARGB.

(I find the GNOME Wiki Attic page a bit perplexing; they talk about
converting *from* premultiplied *to* non-premultiplied when working with
Cairo, which contradicts the Cairo manual, the GNOME developer's blog,
and my observations.)


Some questions I have about my patch:

- Is it OK to have array sizes in argument lists?  I know they are
  mostly decorative (e.g. sizeof still returns the size of a pointer),
  but I find the practice useful as a way to document intent (no idea
  if static analyzers are smart enough to use them nowadays).
  
  I can't say off the top of my head whether they come from C99 or
  later, but I found a few other functions using them
  (prepare_standard_handles in src/w32.h,
  get_lface_attributes_no_remap in src/xfaces.c), so I figured we
  already support the feature in practice.

- I originally divided by 255, since I figured that would be the
  maximal value for any given component and we want x*a / max to yield
  x when a is 255; however I later noticed that emacs_color_to_argb32
  divides by 256, so I went for that instead.  I am still not sure
  whether it's correct, or whether it matters much.

- I added my static function to the "Image type independent image
  structures" section so that it ends up close to emacs_color_to_argb32,
  but maybe I should move it down to the "SVG" section?  In which case I
  will probably need to add a new #ifdef USE_CAIRO block.

  (Alternately, should I just inline the function?)

- I haven't thought really hard about that "if (iconptr[3] == 0)"
  branch.  Is it still relevant?


Thank you for your time.



In GNU Emacs 27.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.8, cairo 
version 1.16.0)
 of 2019-05-15 built on my-little-tumbleweed
Repository revision: 4fa6029f7ee30aa3316d6d73db61462730042908
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-xwidgets --with-cairo'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS XWIDGETS JSON PDUMPER
LCMS2 GMP

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix

Major mode: Magit Process

Minor modes in effect:
  global-magit-file-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  shell-dirtrack-mode: t
  show-paren-mode: t
  minibuffer-depth-indicate-mode: t
  icomplete-mode: t
  global-page-break-lines-mode: t
  page-break-lines-mode: t
  electric-pair-mode: t
  diff-hl-flydiff-mode: t
  global-diff-hl-mode: t
  delete-selection-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow emacsbug rect help-fns radix-tree ffap log-view misearch
multi-isearch browse-url cus-edit whitespace magit-patch cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs diff-hl-dired notifications dbus xml org-indent org-rmail
org-mhe org-irc org-info org-gnus org-docview doc-view image-mode
org-bibtex bibtex org-bbdb org-w3m org-element avl-tree generator org
org-macro org-footnote org-pcomplete org-list org-faces org-entities
org-version ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table
ob-keys ob-exp ob-comint ob-core ob-eval org-compat org-macs
org-loaddefs find-func cal-menu calendar cal-loaddefs find-dired xref
vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs project magit-ediff
ediff-merg ediff-wind ediff-diff ediff-mult ediff-help ediff-init
ediff-util ediff bug-reference flyspell ispell view markdown-mode rx
color thingatpt noutline outline magit-extras sh-script smie
magit-submodule magit-obsolete magit-popup magit-blame magit-stash
magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone
magit-remote magit-commit magit-sequence magit-notes magit-worktree
magit-tag magit-merge magit-branch magit-reset magit-files magit-refs
magit-status magit magit-repos magit-apply magit-wip magit-log
which-func imenu magit-diff magit-core magit-autorevert autorevert
filenotify magit-margin magit-transient magit-process magit-mode
transient git-commit magit-git magit-section magit-utils crm log-edit
pcvs-util add-log with-editor async-bytecomp async shell pcomplete
server dash smerge-mode jka-compr mm-archive executable vc-git mailalias
smtpmail sendmail iso-transl nnir sort gnus-cite mail-extr gnus-async
gnus-bcklg qp gnus-ml nndraft nnmh nnfolder utf-7 epa-file gnutls
network-stream nsm gnus-agent gnus-srvr gnus-score score-mode nnvirtual
gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig mailcap nntp
gnus-cache gnus-sum gnus-group gnus-undo gnus-start gnus-cloud nnimap
nnmail mail-source utf7 netrc nnoo parse-time gnus-spec gnus-int
gnus-range message rmc puny dired dired-loaddefs format-spec rfc822 mml
mml-sec epa derived epg mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader gnus-win gnus nnheader gnus-util rmail
rmail-loaddefs rfc2047 rfc2045 ietf-drums text-property-search time-date
mail-utils mm-util mail-prsvr wid-edit delight advice eighters-theme
quail cl-extra help-mode rg rg-ibuffer rg-result wgrep-rg wgrep s
rg-history rg-header rg-compat ibuf-ext ibuffer ibuffer-loaddefs grep
compile comint ansi-color ring edmacro kmacro disp-table paren mb-depth
icomplete page-break-lines elec-pair diff-hl-flydiff diff diff-hl vc-dir
ewoc vc vc-dispatcher diff-mode easy-mmode delsel cus-start cus-load
mule-util info package easymenu epg-config url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting xwidget-internal cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 444631 125454)
 (symbols 48 47237 83)
 (strings 32 176476 9183)
 (string-bytes 1 5274843)
 (vectors 16 64009)
 (vector-slots 8 2012096 107118)
 (floats 8 522 596)
 (intervals 56 17166 2584)
 (buffers 992 85))