|From:||GNU bug Tracking System|
|Subject:||[debbugs-tracker] bug#35871: closed (27.0.50; [PATCH] Fix SVG transparency with Cairo)|
|Date:||Wed, 29 May 2019 09:08:02 +0000|
Your message dated Wed, 29 May 2019 18:07:04 +0900 with message-id <address@hidden> and subject line Re: bug#35871: 27.0.50; [PATCH] Fix SVG transparency with Cairo has caused the debbugs.gnu.org bug report #35871, regarding 27.0.50; [PATCH] Fix SVG transparency with Cairo to be marked as done. (If you believe you have received this mail in error, please contact address@hidden) -- 35871: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=35871 GNU Bug Tracking System Contact address@hidden with problems
--- Begin Message ---
Subject: 27.0.50; [PATCH] Fix SVG transparency with Cairo Date: Thu, 23 May 2019 22:05:31 +0200Hello, 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.
Description: PNG imageEach 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 == 0) > *dataptr = bgcolor; > else > *dataptr = (iconptr << 16) > | (iconptr << 8) > | iconptr > | (iconptr << 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).
Description: PNG imageNB: the commit named "a7c9365f7956d9d7a089a2f161d2b9d06fc91d58" in the left screenshot is the one which contains my patch, attached here:
Description: Text DataHere is a before-after-patch screenshot with a grey background, which I find makes the problem more noticeable:
Description: PNG imageLooking 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 == 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))
--- End Message ---
--- Begin Message ---
Subject: Re: bug#35871: 27.0.50; [PATCH] Fix SVG transparency with Cairo Date: Wed, 29 May 2019 18:07:04 +0900 User-agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (Gojō) APEL/10.8 EasyPG/1.0.0 Emacs/25.3 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)On Tue, 28 May 2019 19:02:34 +0900, Kévin Le Gouguec wrote: > > YAMAMOTO Mitsuharu <address@hidden> writes: > > > Patch attached. > > > > It makes cairo image support more consistent with the Xlib one. […] > > SVG images should look similar on both builds. > > Just compiled master (c617b4bf1e50bf33f0016bbcd5f502cc88150f26) > --with-cairo and your patch on top; SVG images do seem to look the same > from where I'm standing: Thanks for testing. Pushed to master as c89900ebd79. Closing. YAMAMOTO Mitsuharu address@hidden
--- End Message ---
|[Prev in Thread]||Current Thread||[Next in Thread]|