Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change: Use xcf2png instead of GIMP #26

Closed
wants to merge 1 commit into from
Closed

Conversation

Milek7
Copy link

@Milek7 Milek7 commented Feb 23, 2020

Contrary to previous gimpscript which preserved layer order from .xcf file, xcf2png composes layers in specified order. Only situation when this yields different output is in waterfeatures.xcf2png, so this is fixed.
With Xcftools 1.0.7 it gives pixel-identical results as .png commited in repository.
Curiously GIMP 2.10 gives different output because of some blending changes, but I guess it won't matter now.

@TrueBrain
Copy link
Member

Confirmed that the md5sum and sha256sum of the produced GRFs are identical with and without this patch (given NEWGRF_VERSION is set to the same value). LGTM.

@@ -122,9 +122,6 @@ Your system should already have most tools, you'll probably only need NML and gi
Install the developers tools and get NML from the source mentioned above. git is easiest installed via MacPorts:
`sudo port install git`

On OSX GIMP is not found in the path, if you installed the app package as supplied from the GIMP's project page. You can add that to your search path if you link the binary which requires the X-environment to be running:
`sudo ln /Applications/Gimp.app/Contents/Resources/bin/gimp /usr/local/bin/gimp`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure OSX people want their hand hold; ask an OSX user where xcftools can be found? (brew? port?)

@glx22
Copy link
Contributor

glx22 commented Feb 23, 2020

It's a good idea, but the main issue is xcftools are not really available outside linux it seems

@kaechele
Copy link

I'm somewhat opposed to this change as xcftools is unmaintained by the original maintainer and has known bugs.
There is a fork of xcftools available at https://github.com/j-jorge/xcftools where you can also check the issue tracker for the known bugs (including some arbitrary code execution exploits).
Furthermore xcftools was removed as a package from Fedora about a year ago because it is unmaintained and has unfixed security vulnerabilities.
As the Fedora maintainer of both openttd and openttd-opengfx I would at least have to drop opengfx from the repos or fix-up xcftools to be acceptable for re-introduction into Fedora repos (as you probably figured that's not going to happen).
So my ask would be to make xcftools optional for those who want to use it and still have gimp support for those who have to use it.

@TrueBrain
Copy link
Member

TrueBrain commented Feb 26, 2020

Not disagreeing with what you wrote, just an observation:

For the record, gimp (and xcftools with this PR) are always optional. I don't see any real need why you, as package maintainer, would want either :) The PNGs it produces are stored in the repository, and should, in my opinion, be used by package maintainers. As that means everyone gets the same package. Otherwise it could mean that, for example because of different GIMP versions, Fedora has another OpenGFX than, say, Debian.

Of course, for -dev package I can understand the need. But for OpenGFX I doubt anyone would use it :)

And of course, I am open to be convinced of other opinions :D

@planetmaker
Copy link
Contributor

planetmaker commented Feb 26, 2020

as sprite generation from xcf to png is optional and a maintainer thing, keeping gimp seems more reasonable: a maintainer for this graphics set will be in need of gimp. Using gimp as cmd-line tool works on a CI/CD just fine, too, if you use it as additional integration check. If anyone wants to use xcf2png, one can do so by setting "GIMP=xcf2png" and "GIMP_FLAGS=" - and it could be even done on a CI/CD.

@LordAro
Copy link
Member

LordAro commented Mar 29, 2020

Given the difficulty in obtaining xcf2png on some systems, and it's possible unmaintained status, I'm going to close this for now.

@LordAro LordAro closed this Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants