From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: GDI+ take 3 Date: Tue, 21 Apr 2020 17:13:25 +0300 Message-ID: <83lfmpexkq.fsf@gnu.org> References: <83d088fwgt.fsf@gnu.org> <835ze0fqk2.fsf@gnu.org> <83sgh3eogs.fsf@gnu.org> <838sitazal.fsf@gnu.org> <86imhxufx9.fsf@csic.es> <83y2qsap7r.fsf@gnu.org> <20200418201943.GA57763@breton.holly.idiocy.org> <83eesjibo6.fsf@gnu.org> <86368zxlsn.fsf@csic.es> <834ktei7pj.fsf@gnu.org> <86h7xd72yq.fsf@csic.es> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="94604"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Juan =?utf-8?Q?Jos=C3=A9_Garc=C3=ADa-Ripoll?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Apr 21 16:14:31 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jQtfT-000OWg-K3 for ged-emacs-devel@m.gmane-mx.org; Tue, 21 Apr 2020 16:14:31 +0200 Original-Received: from localhost ([::1]:58986 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jQtfR-0004cZ-Ld for ged-emacs-devel@m.gmane-mx.org; Tue, 21 Apr 2020 10:14:29 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:58384) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jQtes-00043y-NT for emacs-devel@gnu.org; Tue, 21 Apr 2020 10:13:55 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:42098) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jQtes-0002gP-DP; Tue, 21 Apr 2020 10:13:54 -0400 Original-Received: from [176.228.60.248] (port=1637 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1jQtek-0004FI-38; Tue, 21 Apr 2020 10:13:53 -0400 In-Reply-To: <86h7xd72yq.fsf@csic.es> (juanjose.garciaripoll@gmail.com) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:247467 Archived-At: > From: Juan José García-Ripoll > > Date: Tue, 21 Apr 2020 08:44:13 +0200 > > > That makes some assumptions I felt uneasy about. First, it assumes > > that the native_image entry will never have an init method; this could > > become wrong at some future point, at which time someone will have to > > remember to adapt initialize_image_type to handle that (since > > native_image doesn't really have a library to load). > > I do not think this is right. The current implementation is > > - Something wants to load an image and invokes lookup_image_type() > - lookup_image_type() calls image_can_use_native_api > + within image_can_use_native_api, the type of image is verified > - if it is right, it _initializes_ gdi+ (= init method) > - otherwise returns false > + when image_can_use_native_api returns false, lookup_image_type > loops over image type > - for each image type it calls initialize_image_type() > * initialize_image_type aclls image_can_use_native_api again! > * it then invokes the initialization method > > So, in the current architecture, image_can_use_native_api() is called > redundantly. Yes, but it's very fast. And also, is the above flow the only one? Can we ever call initialize_image_type directly? Doesn't that happen when we first time call image-type-available-p, for example? Such a call can be issued by Lisp which tries to see if a certain type of images can be displayed, and if not, use some fallback. > > Second, what happens if the TYPE argument specifies an image type the > > native_image cannot handle (e.g., SVG), and the corresponding optional > > image library is not linked in or is unavailable at run time? With > > your patch, we would return true, right? > > No. It would call the initialization method. I think we may be miscommunicating. If Emacs is built without librsvg, the HAVE_RSVG macro is not defined, and the SVG part of the image_types[] array and init_svg_functions are not compiled into Emacs. So there's no initialization method to call for SVG images, and initialize_image_type should return false. > > You are right. However, I believe I already fixed that, albeit a bit > > differently, as part of commit 423089d (after bumping into the same > > problem during testing my recent changes). We can use your change > > instead, if you think it is better for some reason. > > I am not sure. I pulled latest emacs and w32_frame_delay() currently has > this > > delay = decode_delay (propertyItem, frame); > if (delay <= 0) > { > /* In GIF files, unfortunately, delay is only specified > for the first frame. */ > delay = decode_delay (propertyItem, 0); > } > > This code is wrong. It originates in my wrong decoding from > PropertyItem. I was testing the GDI+ library with various GIF files and > they returned 0 for the delay at later frames, but it was because I > misunderstood how Property Item works. It should read > > delay = decode_delay (propertyItem, frame); OK, I'll remove that part. > Now, in both cases (before and with this change) the output of this > function is used here (this is the code in master). > > if (delay >= 0) > metadata = Fcons (Qdelay, Fcons (make_float (delay), metadata)); > > If delay == 0, it should not be returned. But I suppose it is just > nitpicking. I thought a delay of zero was a valid value. But if it isn't, you are, of course, right, and we should change the inequality to use >.