search

AppImage / AppImageSpec

This repository holds the specification for the AppImage format.
http://appimage.org/
MIT License
71 stars 22 forks source link

Add rule that .DirIcon should be png #27

Closed probonopd closed 5 years ago

probonopd commented 5 years ago

Add check that .DirIcon should be png

Reference: AppImage/libappimage#104 (comment)

probonopd commented 5 years ago

Also, placeholders like Thumb::URI=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(...) (to be defined).

TheAssassin commented 5 years ago

Why does it have to be PNG? Is that like, most compatible? What's the use case?

TheAssassin commented 5 years ago

Actually, we can just mention that the old ROX filer spec also requires PNG there. We can add some explanatory stuff (like the answers to the questions I wrote in my last comment).

probonopd commented 5 years ago

Why does it have to be PNG? Is that like, most compatible? What's the use case?

@azubieta says all thumbnails need to be PNGs, and we want to have something in the AppImage that can be used as a thumbnail without performance-costly conversions/calculations at integration time

azubieta commented 5 years ago

Why does it have to be PNG?

See https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html#CREATION

TheAssassin commented 5 years ago

@azubieta mind to send a PR?

azubieta commented 5 years ago

Also, placeholders like Thumb::URI=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(...) (to be defined).

This can only be done in libappimage as it's a reference to where the AppImage file is located.

TheAssassin commented 5 years ago

@azubieta well you can write it into the spec nevertheless.

azubieta commented 5 years ago

@azubieta well you can write it into the spec nevertheless.

The .DirIcon must follow the spec but adding such placeholders will be pointles as the files will have to be edited anyway. I consider that we should not annoy people with it.

TheAssassin commented 5 years ago

Makes sense. Didn't get fully clear from the previous comments.

probonopd commented 5 years ago

The .DirIcon must follow the spec but adding such placeholders will be pointles as the files will have to be edited anyway. I consider that we should not annoy people with it.

I don't get it. The point of the placeholder is that one can do a search and replace of strings without the need for an image library (e.g., using C in runtime.c).

TheAssassin commented 5 years ago

We have to extract the icon anyway, so we can just read it into memory normally and edit it on demand. No need to add such a rule to the specification, which would also break the backwards compatibility. We have too do it anyway for all old AppImages, and this "optimization" just creates many issues code wise. The gain is very little, actually, you could claim that this would add bloat to the PNG file.

probonopd commented 5 years ago

So, how are the Thumb::URI= and other tags going to be inserted? Can it be done in pure C?

TheAssassin commented 5 years ago

Anything can be done. But, no need for it, is there? We just do it with libappimage when registering AppImages.

probonopd commented 5 years ago

We just do it with libappimage when registering AppImages.

Isn't that what we said is a) requiring integration-time dependencies that we want to get rid of, and b) time-consuming? (some measurements would be good)

TheAssassin commented 5 years ago

Any updates on this? Waiting for this to be implemented to adjust checks in appimagelint.

probonopd commented 5 years ago

Which part of this ticket do you mean with "this"?

TheAssassin commented 5 years ago

Is there an official rule in the spec that says "must be PNG, anything else is invalid"?

probonopd commented 5 years ago

So the logic is that in order to be able to use .DirIcon as a thumbnail icon with minimal overhead for the thumnailer, it should be a 256x256 png. Correct?

TheAssassin commented 5 years ago

@azubieta your call.

azubieta commented 5 years ago

So the logic is that in order to be able to use .DirIcon as a thumbnail icon with minimal overhead for the thumnailer, it should be a 256x256 png. Correct?

Yes, using the maximum icon size is the preferred way. Reducing the size of an image doesn't produce ugly results and it's something that desktop environments can easily do. Additionally we can save any image manipulation on libappiamge. The image metadata should (still have to look for a header only lib to do that) be edited with a minimum effort.

TheAssassin commented 5 years ago

@probonopd please update the spec if you reference this issue in other issues. This is not even a rule yet, so it shouldn't be carried out as such yet.

probonopd commented 5 years ago

Closed in https://github.com/AppImage/AppImageSpec/commit/fc746b5acc34709e09f5e7c7ac632ba507b1c1e3

TheAssassin commented 5 years ago

Now, "should" it be a PNG, or "must" it be a PNG? That's a big difference for appimagelint.

probonopd commented 5 years ago

For now it should, otherwise you'd get a lot of fails on existing AppImages.

TheAssassin commented 5 years ago

Well it's a spec draft, it's not like we'd make releases. For the next type we have to professionalize this a bit, I'd like to have versions myself, really useful for governance. The next runtime should also be released under the terms of the zlib license, current license (MIT) has a few drawbacks.

probonopd commented 5 years ago

Drawbacks: namely?