thomasdev wrote:

fixed all spaces and tabs and reduced the patch-size
hope this is better :-D

Attachments

• smallmap_patch2.diff (13.13 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment110

thomasdev wrote:

+added mousewheel-zooming
+added vehilce draw if close enough

Attachments

• smallmap_patch4.diff (16.42 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment111

thomasdev wrote:

+added restricted vehicle draw if zomed out
+added zoom indicator on the lower left
+added industry strings on the map if zoomed in
+added town population if near enough

Attachments

• smallmap_patch5.diff (22.15 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment112

TrueBrain wrote:

I gave it a quick look (sorry, again quick). Some minor things I noticed:

There is a back lack of spaces. Don't do: if (a==b), but if (a == b). See Wiki for Coding-Style.
There is a very inconsitent usage of comment-style. We mostly use /* */.. you mix it :)
You still have adds of empty lines in your patch ;) Check your own patch :)

But something a bit less minor:

We try to keep the amount of doubles as few as possible. Can't you avoid it? And no, don't come with a float :p Can you do it with an integer? First of all, it is much faster, second, it is much faster ;) And mostly it is easy possible to do so.

I will apply it this week or something to my trunk, see what it does :) From the code, it looks very promissing! Keep up the good work!

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment113

thomasdev wrote:

thx for commenting :)
i will fix that code-style soon and noticed the empty lines after adding my comment...
i mix the code-style to fit in the code-style for the other comments in that file-section
new patch is following soon (after my math exam) :-D
just used doubles, no floats, the only (float) was to print a value as i do not know which letter is for doubles...

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment114

thomasdev wrote:

+added smooth smallmap zooming
+added patch-settings in gui configfile and code
+added dualscreen resoultion for my testings
+added colored industry-production-signs
-bug: with repositioning of the zoomed area when window is not at default size ?!

Attachments

• smallmap_patch6.diff (33.94 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment115

peter1138 wrote:

Thomas, as you left IRC so soon, here are my comments:

1. You're still using doubles. Using ints is preferable.
2. Still the spacing on some lines is inconsistent, e.g. "double diff_abs=(diff<0)?diff*-1:diff;" should be "double diff_abs = (diff < 0) ? -diff : diff;" (ignoring the double / int issue)
3. There are at least three separate patches here, a) zooming b) industry information c) vehicle display. Making them discrete incremental patches would be advantageous.
4. Stick to English for variable names (maybe I'm biased here :))
5. (my >> 1) << 4 == my << 3 (though that's not yours)
6. Don't put irrelevant changes in (screen resolution)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment116

thomasdev wrote:

ok, will fix soon

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment117

thomasdev wrote:

+patches seperated, only zoom-patch provided
i tried to convert the zoom-logic from doubles to int, but all my tries failed. hopefully someone can show me how it could be don

Attachments

• zoompatch2.diff (21.03 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment118

lucaspiller wrote:

Nice job reintroducing this patch, hopefully you will have more luck than myself at getting it into the tree.

I haven't tried this yet to see what is different, but I have a couple of suggestions that may help along the way:

1. WE DON'T NEED PATCHES FOR EVERY SINGLE LITTLE (lots of cursing) OPTION - as I said I haven't tried the patch to see what smooth zooming is like, but IMO it seems rather pointless to have it as a patch, just leave it on by default (of course there will be about a hundred people now hunting me down as they disagree). There are already far too many patch options to confuse users to hell, we don't need yet another.

2. Spacing still needs a bit of improvement, when defining variables, i.e. "double smallmap_zoom=WP(w, smallmap_d).zoom_ist;" should be "double smallmap_zoom = WP(w, smallmap_d).zoom_ist;". Spacing helps with readability so do it!

3. Before you post any patch, proof-read it by hand to make sure there are no changes where there shouldn't be, i.e. changing a blank line. Even if there is a space where there should just be a blank line do not change it unless it will end up within a section you have edited (if that makes sense) - even then you should be wary of such changes.

I have only had a brief skim through the patch, but using doubles seems a bit pointless to me. Using an int you can get 256 different levels - surely this is enough even for smooth scrolling?

If you need any help understanding how something works, or have any questions give me a shout (thelucster at gmail dot com). I will try and give you a hand, but I haven't done any OpenTTD stuff since before christmas so I may be a bit rusty, and with college on my plate I haven't got much free time.

Keep up the good work!
- Luca

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment130

Celestar wrote:

I'm going to check that diff out over the weekend. I think it is worth adding for 0.5.0 because the smallmap appears rather useless on large maps in its current incarnation

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment155

thomasdev wrote:

thx for these greate comments :)
some comments from my side:

1. options-checkbox in patch-settings: this was a test, so i could switch "smooth" zooming on and off at runtime, my thoughts: if smooth will proof better, then take this and remove the option ...
2. spacing + blank lines, i know, working on my style :)

using doubles: i was not able to use int instead of doubles, because i couldn get it working to store the zoom-steps as int and calculating with floating-point numbers, i will try it again soon

@victor: thx for your interest, but i do not believe it merges without work anymore, because i wrote the patch against the nightly of 20th feburary
if anything is unclear just mail me thomas @ dontspam @ thomasfischer.biz

@luca: you did the great work with the base of this patch, i only bugfixed and improved it a little bit :-)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment156

peter1138 wrote:

This version of the patch replaces the floating point operations with integers and removes the patch options

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment193

thomasdev wrote:

patch ported to the latest svn, zooming & focus does still not work properly, hope luca will fix this

Attachments

• int_zoom_latest.diff (17.28 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment197

peter1138 wrote:

This version of the patch has the zoom value the correct way around, i.e. 200 = 200%. Zooming appears to work correctly (although it's possible to stop at a arbitary value (is that good or bad?)) but recentreing on zoom is a little odd.

Attachments

• int_zoom2.diff (15.80 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment198

thomasdev wrote:

updated patch to svn revision 5694 (latest) (based on peter's patch int_zoom2) by the hope, that it will be integrated some day ;-)

added function for ScrollMainWindowTo so send a RECENTER event to the smallmap-window in order to recenter to the current viewport. ==> when you click twice on a newspaper-heading or something like this, the smallmap scrolls also to this position

Attachments

• int_zoom3.diff (16.11 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment366

thomasdev wrote:

fixed a bug that caused a segfault, when smallmap is not opened

Attachments

• int_zoom3_1.diff (16.16 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment367

TrueBrain wrote:

I applied one small piece of your patch. The patch looks very nice to have, so I promise you I will read through it soon and comment on it.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment434

TrueBrain wrote:

Okay, this patch should really be applied ASAP :) It only has some minor problems and defects, and some things that can be done more pretty. Also, this patch in fact are several patches. So I will take this patch and apply it piece by piece over the next few days.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment444

precision wrote:

TrueLight, I hope you didn't forget this patch :(.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment519

Rubidium wrote:

Updated the patch so it compile with revision 7236. One issue remains and that is the fact that when you're zooming it doesn't stay focussed at the point that was in the center of the map. I've looked at this problem for quite a while and was not able to solve this problem.

Attachments

• smallmap_zoom.diff (10.57 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment648

lucaspiller wrote:

I had problems with the centering when zooming myself, I think I got it to work in one of my patches on SF, so have a looky.

http://sourceforge.net/tracker/?func=detail&aid=1206659&group_id=103924&atid=636367

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment649

lucaspiller wrote:

I had problems with the centering when zooming myself, I think I got it to work in one of my patches on SF, so have a looky.

http://sourceforge.net/tracker/?func=detail&aid=1206659&group_id=103924&atid=636367

Also did you forget TrueLight? :P

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment650

TrueBrain wrote:

Such a nice patch, and totally forgotten... if someone can update it, we can reconsider adding it to trunk, if not, this task will be closed.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1439

NukeBuster wrote:

I happened to cross this task and thought it would be nice to bring this patch up to it's current revision (r10450).

Hopefully it's trunk worthy.

Please let me know.

Attachments

• smallmap_zoom_10450_Nukeb.patch (10.83 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1532

NukeBuster wrote:

This is the latest rev patch I have at this moment. The last problem is, when you scroll at a certain zoomlevel, and then zoom in. It doesn't stay focused on the place on the map where you have scrolled to.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1671

NukeBuster wrote:

forgot to add attachment.

Attachments

• smallmap_zoom_10643_Nukeb.patch (11.41 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1672

thomasdev wrote:

that problem existed also in the base versions i based my patch on :-
i hope someone with the knowledge of the map positioning stuff can fix this, i tried but failed :(
and thanks for updating the patch!!

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1674

NukeBuster wrote:

I have already had some tries, but the coordinate system in the smallmap still eludes me a bit.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1675

thomasdev wrote:

i will just have a look at Luca's old code, maybe i will find something.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1677

bilbo wrote:

I think options should be with other options to be consistent ... it's not good to have some extra options at many individual widgets ... and it will IMHO add quite unnecessary button to the smallmap gui ... most people will set up once what they want and then touch the setting no more.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1706

NukeBuster wrote:

I will think about the possibilities. I'm about to go on vacation for a week or 2 so the next update may take a while.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment1708

Phazorx wrote:

adapted to recent trunk (buttons were moved)
It should be noted somewhere that this does NOT work in paused mode

Attachments

• int_zoom5.3-r11344.diff (12.90 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment2524

Romanski wrote:

Task 1222 (http://bugs.openttd.org/task/1222) implements zoomable smallmap in a different way:

Better:
+ 4x higher resolution when zoomed out (the task 54 patch uses the original smallmap algorithm, which is restricted to drawing the map out of 4x1 pixel blocks)
+ draw towns and industries as dots when zoomed out, to avoid them disappearing altogether
+ pretty dithering
+ works when paused

Worse:
- no smooth zooming
- can't zoom to arbitrary zoom levels
- can't zoom in more than the original, un-zoomable map.

Please consider both fairly! :) (disclaimer: obviously I'm the author of 1222; I'm trying to be as objective as I can here; if there is an advantage of task 54 patch which I didn't mention it was not on purpose!)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment2549

Romanski wrote:

Actually I forgot a couple of other advantages of 1222:

+ zoom in/out at cursor, just like the main map
+ scrolling the wheel quickly will zoom several levels in/out quickly

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment2550

fonsinchen wrote:

I have adapted the patch to current trunk, rewriting most of it. I think it is a little cleaner like this. The large size of the diff is mostly due to DrawSmallMapStuff being moved into the SmallMapWindow class (where it belongs). The function itself has hardly changed. I didn't try to understand or clarify the arcane magic involved in drawing the smallmap, so the code is still not nice to read, but at least it didn't get worse. I also removed the smooth zooming, the zoom level indicator and the dependence on ticks to zoom. And I'm using the regular ZoomLevel enum, but I had to create a few more zoom levels in order to be able to scroll in. I need the scroll in functionality for cargodist. However, as the rest of the code doesn't play well with that, I didn't change the BEGIN, END and COUNT values of the enum, which is sort of a hack. I'll happily change those things if any specific ideas show up or if anyone can give any relevant details on the previous functioning of the smallmap.

Attachments

• smallmap_zoom_r16552.diff (16.33 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6194

SmatZ wrote:

The minimap gets incredibly glitchy (it's "shaking") when you are moving the minimap (right-click in the minimap window).
What's the point of zoom-in in the minimap? I would remove it, really. It's buggy anyway (at least wrt. vehicles) and the changes done in zoom_type.hpp are not nice. I think you would like to remove it when you will be fixing that "shaking" problem. (buggy, hacky and useless anyway)
I would expect the mouse wheel to work for zooming.
Added buttons are not disabled when further zooming isn't possible.

#1222 has better behaviour in this sense. But has different issues, town names are disappearing, and diff is harded to read.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6227

Romanski wrote:

I could change the town names behaviour in #1222. Would you prefer if they disappeared later on or not at all?

Also, someone suggested that if I reformatted the diff manually it would be easier to read - i just relied on svn diff there... I'll give it a go, see if it helps.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6228

SmatZ wrote:

I think it's fine the names are getting smaller, but I would prefer not to remove them completely. Also, please stick with official coding style (curly brackets, comments) - http://wiki.openttd.org/Coding_style

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6229

fonsinchen wrote:

The point of zooming in is that I need it for cargodist. The problem there is that you cannot read the link statistics of short links. I can split up the patch to form one version with zooming out and one with additional zooming in. The other issues will be fixed in future versions. My post shows the bare minimum needed to allow zooming. I chose this patch especially because it can zoom in and because it changes much less code than #1222.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6230

SmatZ wrote:

Yes, functionality of #1222 and patch size/code changes from #54 would be the best.
Cargodist isn't in trunk...

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6231

Romanski wrote:

Heh, it would, wouldn't it :)

The size of the change cannot be reduced because the patch rewrites the core algorithm involved in drawing the minimap. The upside is that it can draw single pixels per tile when zoomed out, whereas # 54 draws 4x1 pixel "tiles" even when zoomed out. The downside is the patch size.

Zooming in can be added... and would increase the patch even further :)

Not sure whether it's worth spending the time prettifying the diff - if it's too large to be looked at anyway...

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6232

fonsinchen wrote:

I could revert the changes in zoom_type.hpp if I added a flag in smallmap that denotes if the zoom level is to be interpreted as zooming in or zooming out. Would that be more acceptable than the additional zoom levels? I mean, cargodist is really my only motivation for picking up this patch and zooming in is the only function I need there.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6233

bilbo wrote:

With 2048x2048 map and usual screen sizes, the entire minimap does not fit on screen. Even worse if you have small display (some smallish notebooks/netbooks, etc ...)

Therefore I think the feature is good even without any patches - though for some patches, like cargodest or my large map size patch, having zoomable minimap would help much more.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6234

fonsinchen wrote:

This version tries to fix the "shaking" problem. In the lowest zoom level you still can't scroll pixel by pixel, but the situation is much better than in the previous iteration. For my taste it's good like this. Please tell me what you think about it.

Attachments

• smallmap-zoom_r16780.diff (19.54 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6306

fonsinchen wrote:

This patch fixes the remaining issues. Changes:
* disable the "+" and "-" buttons when reaching max/min zoom levels.
* enable mouse wheel zooming
* make zooming a little more smooth
* document some things
* remove useless code
* draw vehicles correctly on zoom in

By now I have understood most of the obscurity in smallmap and either documented or removed it. The only thing left are some magic offsets in DrawSmallmap which I can't make any sense of. I have, however, marked those places accordingly.

Attachments

• smallmap_zoom_r16799.diff (23.33 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6317

fonsinchen wrote:

new version. changes:
* split up in zoom-in and zoom-out as requested on IRC. zoom-out can be applied to plain trunk. zoom-in is to be applied on top of both zoom-out and the patch from #3094
* corrected a small glitch in DrawIndustries

Attachments

• smallmap-zoom-out_r17064.diff (29.21 KiB)
• smallmap-zoom-in_r17064.diff (4.93 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6450

fonsinchen wrote:

New version:
* +, - and center buttons are visibly depressed now when clicked on
* some constants enumified and consistently named

Attachments

• smallmap-zoom-in_r17163.diff (4.98 KiB)
• smallmap-zoom-out_r17163.diff (30.98 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6479

fonsinchen wrote:

New version:

\* fixed centermap button getting pressed on implicit centering
\* fixed "+" button being enabled on initialization
\* fixed hiding/showing enable/disable industries buttons in OnPaint (it's doing that in constructor and OnClick instead now)

Attachments

• smallmap-zoom-out_r17169.diff (32.39 KiB)
• smallmap-zoom-in_r17169.diff (5.29 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment6482

Alberth wrote:

Zoom-out is implemented in trunk in r19039-r19042 using a somewhat different approach.
Keeping the issue open for the zoom-in part.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment7538

fonsinchen wrote:

The repository has moved to git://github.com/fonsinchen/openttd-cargodist.git
An up-to-date patch can now be found at http://github.com/fonsinchen/openttd-cargodist/raw/patches/current/smallmap-zoom-in.diff
The trunk version it applies to can be found at http://github.com/fonsinchen/openttd-cargodist/raw/patches/current/TRUNK_VERSION.txt

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment8134

fonsinchen wrote:

I have cleaned up the patch and removed the lowest zoom in level as that was as detailed as the main map. I think it is ready for trunk now.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment8819

fonsinchen closed the ticket.

Reason for closing: Out of date

Those patches definitely won't apply anymore today. If we want smallmap-zoom-in we have to start from scratch

This comment was imported from FlySpray: https://bugs.openttd.org/task/54

fonsinchen wrote:

ulfhermann/openttd@ed06fdb is pretty broken regarding the positioning of town names and the current viewport indicators. However, it's a fairly clean patch on top of the last supported version (r23343) - just in case anyone wants to pick it up again.

This comment was imported from FlySpray: https://bugs.openttd.org/task/54#comment13124