Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#877 closed enhancement/feature request (fixed)

Replace xpms with svgs #2

Reported by: themroc.myopenid.com Owned by: woglinde
Priority: minor Milestone: version 0.5.0
Component: core Version: git master
Severity: Keywords: xpm png svg icons
Cc: sebastian.leske@…

Description

Some more svgs, this time all from sjjb (one modified by me). The license (PD) is included. The colors were modified a bit, so that the groups are now distinguishable. Also, this fixes some older xpm->svg transitions, that somehow were forgotten in navit_shipped.xml.

Cheers!

Should i better mail such patches somewhere or do you prefer it when i continue to (ab)use this bug-tracker?

Attachments (5)

navit-r4517-xpm2svg-no2b.patch (1 bytes) - added by themroc.myopenid.com 7 years ago.
Delete me!
navit-r4517-xpm2svg-no2.patch.gz (52.3 KB) - added by themroc.myopenid.com 7 years ago.
navit-r5003-autogen.patch (425 bytes) - added by themroc.myopenid.com 7 years ago.
navit-r5005-xpm2svg-no2.patch.gz (80.4 KB) - added by themroc.myopenid.com 7 years ago.
navit-r5005-cmake-fix.patch (1.1 KB) - added by themroc.myopenid.com 7 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 Changed 7 years ago by themroc.myopenid.com

D'oh! Yet another error... forget the 2b-patch, i will replace the 1st one.

comment:2 Changed 7 years ago by themroc.myopenid.com

Finally! A patch that actually works (i hope...), even with make dist and make install. Because i wanted to keep the sjjb directory structure but still have all generated icons in one place, i had to hack navit_svg2png.

Last edited 7 years ago by themroc.myopenid.com (previous) (diff)

Changed 7 years ago by themroc.myopenid.com

Delete me!

Changed 7 years ago by themroc.myopenid.com

comment:3 Changed 7 years ago by sleske

  • Cc sebastian.leske@… added

comment:4 follow-up: Changed 7 years ago by sleske

Hi, thanks for your patch, and sorry it was not reviewed earlier. Seems to somehow have escaped everyone's attention.

Unfortunately, the patch no longer applies cleanly (navit/xpm/Makefile.am was modified in the meantime). Also, I'm still kind of torn on the complex build process it introduces (running various shell scripts on the SVGs). I understand you would like to put the unchanged SVGs into the repo, but at the same time I think it's important to keep the build process as simple as possible.

I'll have to think about that...

comment:5 in reply to: ↑ 4 Changed 7 years ago by themroc.myopenid.com

Replying to http://sleske.myopenid.com/:

Hi, thanks for your patch, and sorry it was not reviewed earlier. Seems to somehow have escaped everyone's attention.

Yeah, that seems to happen sometimes... #862 and #882 have to endure the same fate (hint, hint!).

Unfortunately, the patch no longer applies cleanly (navit/xpm/Makefile.am was modified in the meantime).

That should be easy to fix.

Also, I'm still kind of torn on the complex build process it introduces (running various shell scripts on the SVGs). I understand you would like to put the unchanged SVGs into the repo, but at the same time I think it's important to keep the build process as simple as possible.

Those shell-scripts serve as a kind of config-files, so users can easily tune the color scheme. Also, they could select variants of icons, like shelter2.svg vs. shelter.svg. Or rather that was the idea, but since shelter.svg isn't included, thats basically useless.

Maybe the best would be to put those rename/recolor-scripts in /contrib, let some developer run 'em and put only the converted svgs in the cvs? That way the rather simple navit_svg2png can stay as it is and users who really want can still tune colors. This patch needs work anyway, so i'd be happy both ways...

comment:6 follow-up: Changed 7 years ago by sleske

Yes, maybe putting the scripts into some "contrib" subdir is best. If the SVGs are updated, the scripts may need to be adapted anyways. As to the Makefile.am: I'll try to adapt the patch.

I'll also do a last check of the images, maybe then I can commit this patch.

comment:7 in reply to: ↑ 6 Changed 7 years ago by themroc.myopenid.com

Replying to http://sleske.myopenid.com/:

Yes, maybe putting the scripts into some "contrib" subdir is best.

I'd recommend the existing "/contrib/", the one that holds my little android_build.sh. But please, don't bother. Do this:

As to the Makefile.am: I'll try to adapt the patch.

I'll also do a last check of the images, maybe then I can commit this patch.

... and i will add some recolor-script later. And please don't waste too much time on this, i'd be more than happy to make a new, clean patch for r4977.

comment:8 Changed 7 years ago by sleske

Ok, I've had a look at things. I see three problems:

  • I think the images should be "positives" (i.e. transparent background). The images with the coloured background take a lot of room in the map and look visually intrusive. Note how e.g. Mapnik (www.openstreemap.org) also uses icons with transparent background.
  • The memorial icon is unrecognizable at small scales; we need a better solution for this one.

If you could change the SVGs to positives and submit a new patch, I'll apply it. As discussed above I'd just commit the finished SVGs; the scripts to recolor etc. can go into some contrib subdir, along with a README explaining where they come from etc.

In the meantime I'll extract the other changes from the old patch (the forgotten xpm->svg transitions) and apply them.

Last edited 7 years ago by sleske (previous) (diff)

comment:9 Changed 7 years ago by sleske

The first part of the patch has been applied: menu.svg, toggle_fullscreen.svn, and the fixes for older xpm->svg transitions (rev 5003).

For the rest of the patch: The build system changes will probably not go in in the present form (it's ab it complicated, and the autotools build will soon go away anyway). The SVGs from SJJB can go in once they are fixed as described in my comment above.

comment:10 follow-ups: Changed 7 years ago by korrosa

Looks like this commit broke Android nightly builds?

Build log: http://download.navit-project.org/logs/navit/android_armv5te/svn/navit-svn-5003.failed

Changed 7 years ago by themroc.myopenid.com

comment:11 in reply to: ↑ 10 Changed 7 years ago by themroc.myopenid.com

Replying to sleske:

The first part of the patch has been applied

Gr8! Just one tiny prob: autogen.sh is not working anymore. fix: navit-r5003-autogen.patch.

The images will follow asap. Just as you wrote: the mapnik version looks much better. When i made the original patch i had potlach open, which uses inverted icons.

comment:12 in reply to: ↑ 10 Changed 7 years ago by themroc.myopenid.com

comment:13 Changed 7 years ago by tryagain

I was able to configure successfully my android build after I had removed menu.xpm and toggle_fullscreen.xpm, for which .svg analogs have been added with r5003.

I use CMake build system.

Last edited 7 years ago by tryagain (previous) (diff)

comment:14 Changed 7 years ago by sleske

Sorry about the typo in Makefile.am. I applied the patch by themroc (rev. 5005). That fixes autogen.sh.

As to the problem with the CMake build reported by the Android nightly builds: I cannot reproduce this. On my Linux system, the CMake build works. Unfortunately, I don't have the Android tools installed to test the Android build, so I'm afraid someone else will have to fix this.

Changed 7 years ago by themroc.myopenid.com

comment:15 follow-up: Changed 7 years ago by tryagain

We have to decide what to do with Android builds broken while solving this ticket.

Trivial solution is to remove two .xpms for which corresponding svgs are added. Drawback is that users having their own navit.xml changes may be forced to edit them.

Less trivial would be fix CMake files to skip xpm to png conversion when corresponding svg exist.

The problem most probably can be reproduced by definnig XPMTOPNG flag for any build.

comment:16 Changed 7 years ago by themroc.myopenid.com

Ok, so here are the images as discussed, plus a fix for #882, plus some fixes for wrong images extensions and a little inconsistency in navit_shipped.xml, and finally a little addon for #863.

comment:17 in reply to: ↑ 15 Changed 7 years ago by themroc.myopenid.com

Replying to http://wiki.navit-project.org/index.php/user:tryagain:

Less trivial would be fix CMake files to skip xpm to png conversion when corresponding svg exist.

I'll do that, gotta learn cmake anyway...

comment:18 follow-up: Changed 7 years ago by sleske

OK, I can reproduce the problem now. The variable to set is "XPM2PNG" (not XPMTOPNG), as in:

cmake -D XPM2PNG=y ../navit/

Maybe I'll have a look...

Changed 7 years ago by themroc.myopenid.com

comment:19 in reply to: ↑ 18 ; follow-up: Changed 7 years ago by themroc.myopenid.com

Fixed. At least now cmake doesn't stop with error-msg and make generates lots of png's...

Thanks to http://sleske.myopenid.com/:

OK, I can reproduce the problem now. The variable to set is "XPM2PNG" (not XPMTOPNG), as in:

cmake -D XPM2PNG=y ../navit/

for pointing this out.

Maybe I'll have a look...

Please look at #862 instead.

Btw. cmake is uuuugly! Reminds me of prehistoric stuff like tcl. I'd rather keep the auto-tools. But i guess the decision to switch has been made, so i'll go along with it...

Cheers!

comment:20 in reply to: ↑ 19 Changed 7 years ago by tryagain

Replying to http://themroc.myopenid.com/:

Fixed. At least now cmake doesn't stop with error-msg and make generates lots of png's...

Worked for me fine. Applied as is in r5007

Thank you!

comment:21 Changed 7 years ago by themroc.myopenid.com

Now that the nightly builds are fixed, how about closing this ticket after this is applied? It contains the images as discussed, plus a fix for #882, plus some fixes for wrong image extensions and a little inconsistency in navit_shipped.xml and finally a little addon for #863.

comment:22 Changed 7 years ago by sleske

Now that the nightly builds are fixed, how about closing this ticket after this is applied?

I'm working on it. I hope to apply it over the holidays... after I have found all the eggs :-).

Last edited 7 years ago by sleske (previous) (diff)

comment:23 follow-up: Changed 7 years ago by tryagain

comment:24 in reply to: ↑ 23 Changed 7 years ago by themroc.myopenid.com

Replying to http://wiki.navit-project.org/index.php/user:tryagain:

Do you have any idea if this failure is caused by your changes?

http://download.navit-project.org/logs/navit/wince/svn/navit-svn-4977.failed

I'd say impossible, because the first patch for this was committed 2012-03-30 (see http://trac.navit-project.org/ticket/877#comment:9). The culprit must be a commit done before 20-Mar-2012 01:35 (and after 14-Mar-2012 01:36). This one http://sourceforge.net/mailarchive/message.php?msg_id=28982294 perhaps?

comment:25 Changed 7 years ago by sleske

Do you have any idea if this failure is caused by your changes? http://download.navit-project.org/logs/navit/wince/svn/navit-svn-4977.failed

Yes, probably related. If you use custom PNG sizes with --enable-svg2png-scaling-*, the sizes must be separated with spaces: --enable-svg2png-scaling-nav="16 32 48 64" , not --enable-svg2png-scaling-nav=16,32,48,64 . Then it'll work.

It may have worked with comma before, but that was not clearly documented. Anyway, autotools build is deprecated, better to switch to cmake. So I'm not gonna work on this much more.

comment:26 follow-up: Changed 7 years ago by sleske

  • Resolution set to fixed
  • Status changed from new to closed

Ok, finally did it. Changes were committed in rev.5023 & 5024.

One last-minute change: I had to resize the SVGs, because the build process relies on them having a size of 22x22, but they were 580x580 :-(.

Tested with both autotools and CMake build, should work. There I'm closing this ticket. Thanks for your attention! ;-)

comment:27 in reply to: ↑ 26 Changed 7 years ago by themroc.myopenid.com

Replying to http://sleske.myopenid.com/:

Ok, finally did it. Changes were committed in rev.5023 & 5024.

Juhuuuu! Wun-der-bar! Und fast wär ich schon vom Glauben abgefallen... Err... i mean: Great! Thanx a million!

One last-minute change: I had to resize the SVGs, because the build process relies on them having a size of 22x22, but they were 580x580 :-(.

Ooops... ok, i'll fix the import-script eventually...

Tested with both autotools and CMake build, should work. There I'm closing this ticket. Thanks for your attention! ;-)

Yepp, i can confirm the auto-stuff works with r5025.

thx & cu

Note: See TracTickets for help on using tickets.