[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: GDI+ take 3
From: |
Juan José García-Ripoll |
Subject: |
Re: GDI+ take 3 |
Date: |
Tue, 21 Apr 2020 08:44:13 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (windows-nt) |
Eli Zaretskii <address@hidden> writes:
>> From: Juan José García-Ripoll
>> <address@hidden>
>> Date: Sun, 19 Apr 2020 22:28:24 +0200
>>
>> Two minor changes attached. First, the introduction of the Qnative_image
>> type makes one call to image_can_use_native_api irrelevant.
>
> 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. The second call can be eliminated because it will always
return false. Moreover, image_can_use_native_api() always must implement
an init method, because failed initialization is a cause for not
supporting the API.
> 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.
>> Regarding this, I think it is a bad idea to introduce Qnative_image,
>> because that is not an image format and users cannot recognize the
>> actual image type from the image descriptor.
>
> I'm not sure I understand the issue. When you say "users", what do
> you mean in this context? If you mean users like me and you, then how
> and where would we see Qnative_image?
By "users" I mean anyone using the image library, from image.el,
subcompoments or third-party libraries that use that. I assumed that the
internal type in image.c was also the type returned by functions such as
IMAGE-TYPE and the like. I am not sure any more.
> What would you propose to do instead?
To separate the notion of image type from the notion of image
handler. The former is a unique identifier for the type of image or
format, the latter is which library handles it. But, as I said above, if
img->type.type is not exposed, it does not matter.
>> The second fix makes w32image.c behave like nsimage.c, in that a delay=0
>> is regarded as non-existant, thus returning nil. This makes animations
>> default to the minimum animation delay, which currently is 0.01, and
>> more useful than a delay of 0.
>
> 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);
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.
--
Juan José García Ripoll
http://juanjose.garciaripoll.com
http://quinfog.hbar.es
- Re: GDI+ take 3, (continued)
- Re: GDI+ take 3, Alan Third, 2020/04/20
- Re: GDI+ take 3, Juan José García-Ripoll, 2020/04/21
- Re: GDI+ take 3, Alan Third, 2020/04/25
- Re: GDI+ take 3, Eli Zaretskii, 2020/04/25
- Re: GDI+ take 3, Juan José García-Ripoll, 2020/04/26
- Re: GDI+ take 3, Eli Zaretskii, 2020/04/19
- Re: GDI+ take 3, Juan José García-Ripoll, 2020/04/19
- Re: GDI+ take 3, Eli Zaretskii, 2020/04/20
- Re: GDI+ take 3,
Juan José García-Ripoll <=
- Re: GDI+ take 3, Eli Zaretskii, 2020/04/21
- Re: GDI+ take 3, Juan José García-Ripoll, 2020/04/21
- Re: GDI+ take 3, Eli Zaretskii, 2020/04/15
- Re: GDI+ take 3, Eli Zaretskii, 2020/04/15
re: GDI+ take 3, Angelo Graziosi, 2020/04/15