Wish: Diagonal graphics for signals

[makie]

Wish: The possibility to define diagonal graphics for signals.

This look strange:

If it were possible to define custom graphics for diagonals, one could create suitable graphics for this case.

[Isaac Eiland-Hall]

Seems useful for road signs as well

[victor_18993]

Hi Prissi,

I have prepared and visually tested a proof of concept for optional diagonal
graphics on railway signals and static road signs.

I would like to keep the scope of this proposal deliberately small. I am not
proposing an immediate migration of paksets or a replacement of the existing
roadsign format.

Current proof of concept

The current patch adds an optional image block:

    diagonalimage[dir][state]



When present, the signal uses this list on a way for which
weg_t::is_diagonal() is true. Otherwise it continues using its normal
image[dir][state] list.

The runtime implementation has been tested for:

- normal and diagonal track;
- RED and GREEN states;
- per-slot fallback when a diagonal entry is "-";
- automatic image refresh when a later way_builder operation changes an
  existing signal tile from normal to diagonal;
- loading existing roadsign descriptors versions 1–6;
- no savegame, signalling, routing or network-state changes.

Compatibility issue found during review

The proof-of-concept writer currently emits roadsign descriptor version 7 for
every roadsign, including objects that do not define diagonalimage[].

The version-7 layout is:

    [name, copyright, normal images, diagonal images, cursor]

This works with the patched engine, and existing binary paksets continue to load
unchanged. However, recompiling an otherwise unchanged pakset with this makeobj
would unnecessarily convert all its roadsigns to version 7, making them require
the new engine.

I consider that broader than the feature needs, so I do not propose the current
writer behaviour as the final migration strategy.

Proposed intermediate transition

My preferred next step would allow the old and new systems to coexist:

1. Existing binary roadsign versions 1–6 remain fully supported.
2. If a .dat object does not contain usable diagonalimage[] entries, makeobj
  writes the existing version-6 layout exactly as before.
3. Only an object that explicitly opts in to usable diagonal graphics is written
  as version 7.
4. A partial diagonal list remains a fatal makeobj error.
5. An all-"-" diagonal block is treated as no diagonal graphics and remains
  version 6.
6. The new engine reads both version 6 and version 7 objects in the same pakset.
7. Pakset maintainers may therefore introduce diagonal signals gradually,
  object by object, without recompiling every other roadsign into the new
  format.

Under this intermediate model:

- current paksets continue to work unchanged;
- recompiling unchanged pakset sources preserves compatibility with existing
  engines;
- only the individual signals using the new feature require the updated engine;
- experimental diagonal signals can initially be distributed as optional
  addons;
- no pakset-wide migration is required.

This deliberately does not attempt to make a version-7 diagonal object load in
an old engine. The compatibility goal is instead to ensure that objects which do
not use the new feature remain in the historical format.

Possible further compatibility option

If preserving compatibility even for objects carrying diagonal graphics is
important, another possibility might be to keep the existing cursor child
position and append an optional diagonal-image child:

    [name, copyright, normal images, cursor, optional diagonal images]

That would only be viable if older readers safely ignore additional children.
I have not assumed this in the patch because I would prefer your guidance before
testing or relying on that behaviour.

Questions

1. Would conditional version-6/version-7 output be an acceptable intermediate
  migration strategy?
2. Would you prefer an append-only child layout, assuming old readers can safely
  ignore the additional child?
3. Should the first implementation remain an experimental addon/nightly feature
  until pakset maintainers have had time to adopt it?

The attached patch should therefore be considered a tested proof of concept for
the runtime behaviour, not a request to commit the current unconditional
version-7 writer unchanged.

I can revise the writer and regenerate the patch once the preferred compatibility
direction is clear.

Thanks.

[prissi]

First: I would strongly suggest to apply for all signs, not only signals. I think there was even a patch around a long time ago. It had problems with "signals/drive_on_left" settings.

If signs were to included, I think a new imagelist for diagonal graphics works better, as several instances of signs have different number of images. So the offset calculation would be a quite complex. A missign diagonal child imagelist would be very straightforward and could set the flag on laoding time.

One thing that stopped diagonals was the drive-on-left feature. With this, the signals should be still end up in a reasonable position. Well, and someone has to draw new signs, signals, of course.

[makie]

First: I would strongly suggest to apply for all signs, not only signals. I think there was even a patch around a long time ago. It had problems with "signals/drive_on_left" settings.

New graphics won't change anything regarding the problem with "signals/drive_on_left". The problem already exists, and new graphics won't alter that.
Since they are then independent graphics, they can be positioned better than is currently the case as today the graphics are primarily positioned and designed for straight ways.

For road traffic signs, there is a problem with horizontal diagonal signs. Correctly aligned traffic signs are not really recognizable for the player. I mean the content of the traffic sign. Viewed from the side, a metal sign is simply a line. Probably need some tricks there. Railway signals are simpler here because they are always three-dimensional and therefore better recognizable.

If signs were to included, I think a new imagelist for diagonal graphics works better, as several instances of signs have different number of images. So the offset calculation would be a quite complex. A missign diagonal child imagelist would be very straightforward and could set the flag on laoding time.
 

I don't really care how it's implemented program technically.

One thing that stopped diagonals was the drive-on-left feature. With this, the signals should be still end up in a reasonable position.

This is a problem independent of the graphics. Think, an individual offset parameter for this in the .dat is the simplest solution to this problem. As "drive_left_offset" maybe for x and for y.

Well, and someone has to draw new signs, signals, of course.

That's the goal. Although pixel art isn't really my strong suit, so I'm very slow at it and for me it is hard work.

[victor_18993]

I reviewed the current roadsign image path and the earlier proof of concept before going any further with this request.

The most compatible approach seems to be adding a second optional image list for diagonal graphics, parallel to the existing image list, rather than extending the current list.

Conceptually:

Code Select Expand

image[direction][state]
diagonalimage[direction][state]

Both lists would use the same direction slots, states and index calculation. When the way is displayed as diagonal and a diagonal image exists, the object would use it. If that entry is missing, it would fall back to the existing normal image.

This appears to have several advantages:

- it can apply to all roadsigns, not only railway signals;
- existing paksets would keep their current behaviour;
- signal states, bidirectional signs, electrified variants and rotation could keep the current indexing;
- no signalling, reservation or routing logic would need to change;
- the existing way diagonal state could be reused instead of introducing a second definition of "diagonal".

There are still some design decisions that should be agreed before implementing it:

1. Whether the additional list should remain compatible with roadsign descriptor version 6, using an unused flag to indicate its presence, or whether the descriptor should be increased to version 7.
2. Whether traffic lights should be included in the first implementation or handled separately, since they currently use eight direction slots.
3. Whether the first version may intentionally leave closed diagonals outside its scope, because the current is_diagonal() flag does not cover that case.
4. Whether drive_on_left positioning should remain a separate issue, as Makie suggested.

There is also one required implementation detail: when construction changes a tile from a normal alignment to a diagonal one, the roadsign image must be recalculated as well as the way image, otherwise the old graphic can remain visible.

Before preparing another patch, does this optional parallel image-list design match what you had in mind?

Makie, if so, would you be able to provide a small test object or the diagonal graphics you are already preparing? Without real images we can test backward compatibility and image selection, but not the final positioning and visual result.

[makie]

Makie, if so, would you be able to provide a small test object or the diagonal graphics you are already preparing? Without real images we can test backward compatibility and image selection, but not the final positioning and visual result.

I need 1–2 days for that.

but not the final positioning

Don't worry about the final positioning.
That depends on the graphic and can easily be adjusted using the parameters in the .dat file as well as within the graphic itself.

[victor_18993]

Thanks. Before you prepare any test graphics, it may be better to wait until Prissi has confirmed the final scope.
I would not want you to spend time on assets that might need to be changed or redone afterwards. 

Once the remaining decisions are settled, we can work on the implementation and the graphics in sync, using the same agreed scope.

[makie]

2. Whether traffic lights should be included in the first implementation or handled separately, since they currently use eight direction slots.

Traffic lights can not build at diagonals, there are no diagonal crossings.

[prissi]

My late comment:
I would call the diagonal image definition just diagonal[.... as the string matchin could catch the image[ part of diagonalimage[ Also, it is less to type and similar to ways and wayobjs.

[makie]

Makie, if so, would you be able to provide a small test object or the diagonal graphics you are already preparing?

Here the normal rail signal: Standardsignal
https://makie.de/diagonal-signal.zip

[victor_18993]

We are already working on this patch and will provide a diff together with the corresponding tests once it is ready.

The implementation is not entirely straightforward, as it affects the descriptor format, image selection, compatibility with existing objects and the behaviour of roadsigns when the way geometry changes. At the moment, it looks possible to have a testable version during this week, but I would prefer to leave the timing open.

As you know, once the implementation is underway, additional variables or compatibility issues may appear and require more work than initially expected.

Thank you for providing the test object. It will be useful for validating the final behaviour.

[victor_18993]

Patch attached, against r12130. It follows what you asked for: it works for all signs, not only
signals, and it adds a new image list instead of extending the existing one.

How it is defined

Code Select Expand

Image[0]=Lichtsignale_hv.0.0,50,0
Diagonal[0]=Lichtsignale_hv.0.10,50,0

Both syntaxes are accepted, diagonal[n] and diagonal[dir][state], the same two the
normal images already use. makie's test object builds unchanged.

How the flag is set

The diagonal list is written as the last child node, so the reader knows it is there because the
node has five children instead of four, and sets the flag while loading. This is the same thing
building_reader.cc already does for the optional cursor node.

That means the descriptor stays at version 6 and the pak format code does not change, so:

• An older Simutrans loads a new pakset. It reads the extra child, ignores it and draws the
  straight images. Tested with r12109: no error. As a control, the same build does abort on a
  deliberately truncated pak, so it really is reading the file.
  
• makeobj produces byte-identical output for any sign without diagonals.
  
• The checksum does not change, not even for a sign that has diagonals, because images and
  the child count do not enter it. A player who updates their pakset can still join a server running
  the old one. I compared every field that feeds calc_checksum and they are identical.
  

I checked all 171 road signs of pak64, pak128, pak128.german and pak192.comic: every one of them has
exactly four children, so the "five children means diagonals" rule cannot be confused with anything
that already exists.

If the diagonal list is shorter than the normal one, makeobj refuses to build the pak. If an entry
is missing at run time the straight image is used, so a sign built with some other tool stays
visible instead of disappearing. That also keeps the front/back order and the offsets of a sign
exactly as they are today, which I verified: xoff/yoff are identical with and without diagonals.

Limitations, all of them inherited

• Nothing is drawn diagonally unless the way itself has diagonal images, since
  check_diagonal() is only called inside that condition.
  
• On a close diagonal the straight image is used: check_diagonal() sets close_diagonal_state and

returns without setting diagonal_flag.

• Traffic lights are not covered, as makie pointed out there are no diagonal crossings.
• The drive-on-left offset is not adapted to a bend, it is the same vector as on a straight
  way. Whether that looks reasonable is for the pakset side to judge; this is where makie's offset
  proposal would fit.
  

Testing

Full suite 237/237, three runs on each of the graphical and headless builds, no errors. The
selection itself was measured with an instrumented build: the same signal uses different images on a
bend and on a straight tile, in both the normal and the drive-on-left branch.

Something else I ran into

While testing I found that a way loses its diagonal state when a neighbouring tile becomes a
junction, because the recursion that notifies the neighbours only lives in the flat-way branch of
weg_t::calc_image(). check_diagonal() is then never called again for that tile, so the way
itself keeps the wrong image, not just any sign on it. It is unrelated to this request and older
than it, so I will open a separate thread rather than mix it in here.

[makie]

I really like the patch; it is clear and simple.

Unfortunately, matching the graphics to the diagonals isn't that straightforward. There are two diagonal segments. It works in one direction, but the other doesn't fit. See the image; the mapping was set up this way previously, but since the signals were positioned at an angle anyway, it was up to now a reasonably good fit.


The blue number is the correct one.

[prissi]

Good to see the quick prograss. Apart from this issue, I think the diagonal positions may work also with is_bend() as way ribi.

On the other issue ... maybe this was what had stopped it before.

[makie]

Possible idea:

There might be issues with positioning the graphics. As shown in the image, there are eight possible ways to place signals on diagonals. Each graphic is used in twice, which can lead to positioning problems because the identical graphic needs to be placed at two different locations within the tile.

A simple solution: Provide graphics for all eight possibilities—i.e., diagonal 0 to 7 . This allows package manager to shift each one individually to the correct position.

[victor_18993]

prissi, I looked into is_bend() as way ribi and it works. It is exactly the piece of information
that is missing today, and it also explains makie's picture from the code alone. Patch attached,
against r12130, and it replaces the one I posted yesterday.

Why the four image version cannot place them

On a bend two images are drawn, and which two depends on the bend:

Code Select Expand

N|E -> images 0 and 3        N|W -> images 0 and 2
E|S -> images 3 and 1        S|W -> images 2 and 1

Every image is used by exactly two bends, which is makie's "each graphic is used twice". I measured
this with an instrumented build on pak128.german: the four bends together use only four images, and
each of them twice. And in the right hand branch no per direction offset is applied at all, so the
position comes from the image itself. One image therefore cannot sit correctly in both of its bends,
whatever the engine does.

What the way ribi adds

It says which of the two bends we are in, so the pair (bend, direction) has eight members. With the
same index arithmetic the ways already use, ribi/3-1 in way_desc_t::get_diagonal_image_id:

Code Select Expand

base = 2 * (ribi/3 - 1)
low  = lowest set bit of ribi
slot = base + (direction == low ? 0 : 1)


Code Select Expand

N|E (3):  north=0  east=1
E|S (6):  east=2    south=3
N|W (9):  north=4  west=5
S|W (12): south=6  west=7

Eight slots, no gaps and no overlaps, which is exactly makie's diagonal[0..7], per state. Measured
with the same instrumented build: the four bends now use eight different images, each exactly once.
Same result for signals and for ordinary road signs, tested separately.

One detail worth recording: the bend has to be read from the way ribi, not from the sign direction.
For a one way signal the mask removes one direction, so the sign direction alone is no longer a bend
- measured, ribi 3 becomes 2 and ribi 6 becomes 4.

Please do not take the four image version

It is not committed yet, and this patch changes what the list means: eight entries per state instead
of four. If the older one goes in first, a pakset could ship a list whose meaning changes afterwards,
which is the one thing this design was meant to avoid.

Everything else survives unchanged: detection by the number of children, no descriptor version
change, no checksum change, fallback to the straight image when an entry is missing, and the
diagonal[ key. What changes is the index arithmetic and the length check in makeobj.

I also checked what this engine does with makie's existing object, which is built to the older four
image layout: it loads, it draws, and nothing breaks. The green state asks for slots 8..15, which are
past the end of an eight entry list, and those fall back to the straight image - image_list.h bounds
checks and returns NULL, and the fallback takes it from there.

makie: a worked example rather than a description

Attached is Signal_diag8, a test object with the sixteen diagonal entries filled in and working, and
two screenshots of it next to your object on the same map. It is not artwork - the model comes from
a 3D kit and it looks nothing like pak128.german - it is there so the slot order and the offsets are
something you can read off a working file instead of deducing them.

What it shows is that the placement is entirely yours to control: each entry carries its own offset
in the usual ,x,y form, and the post lands where that offset puts it. In the screenshots the two
posts of every tile sit beside the rail because the offsets say so.

Your current file is exactly what was asked for when you made it - the layout only changed with this
patch - so what it needs now is sixteen entries in the order above rather than eight. If your own
numbering runs differently, say so and I will use yours: the engine does not care, but if we disagree
the graphics land in the wrong place and no automated test can see it. That is the only part of the
whole change that has to be checked by eye.

Scope, so it does not widen by itself

• Traffic lights are out. There are no diagonal crossings, as makie said, and the engine agrees:

roadsign.cc:526 removes a traffic light whose road is not at least straight.

• Private way signs draw a single image, so they have nothing to disambiguate.
• Drive on left is out of this patch. Those offsets are computed in code from vectors meant
  for straight ways and added after the image is chosen, so eight images do not change them. It needs
  its own change, and makie's offset proposal looks like the right place for it.
  

Testing

Full suite 237/237 on the graphical and the headless build. The selection itself was measured with an
instrumented build on pak128.german, for all four bends, for signals and for road signs separately,
and for a one way signal. makeobj refuses a diagonal list of the wrong length and produces
byte-identical output for any sign without diagonals.

Two things I could not settle from here. The front/back order is decided by the code and not by the
pakset - on a N|E bend the east image is in front and the north one behind, on E|S the east one is in
front - and whether that is right on a diagonal needs real graphics to judge. And rotating the map:
neither roadsign_t::rotate90() nor grund_t::rotate90() nor karte_t::rotate90() calls calc_image(),
they only set_dirty(), so whether sign images refresh after a rotation is a question that predates
this patch and I have not answered it.

[makie]

The diagonal sections are working well. I like them. I haven't moved them into their final positions yet—that's still to come.

But something has broken. The trains keep running through red signals on the standard straight sections. The signals aren't switching to green anymore.

[victor_18993]

Thank you for testing it and for reporting this.

It is good to know that the diagonal sections are now working correctly, but the behaviour you describe on normal straight signals is clearly a serious regression. The patch should not affect signal logic, reservations or train movement, so I first need to determine whether the signal is only displaying the wrong aspect or whether trains are actually ignoring the signal state.

I will reproduce the case with your pak files and review the state-to-image mapping introduced by the new diagonal graphics. The patch should be considered not ready for integration until this is understood and fixed.

Please do not spend time adjusting the final graphic positions yet, as the descriptor or image indexing may still need changes.
Thanks again for testing the real use case — this is exactly the kind of issue that the current regression tests did not cover.

[makie]

But something has broken. The trains keep running through red signals on the standard straight sections. The signals aren't switching to green anymore.

I have to retract that.

The signals worked after loading an old map.
A newly built signal didn't work at first, but it did start working after a short while.

It seems signals don't show green when the train is getting out of a station.

On a newly created map, I can no longer reproduce the effect I described in my post from 13:01.

Sorry I no longer think that this is due to our changes. That must be historical glitches.

[makie]

I fell victim to a quirk of the program.

If a train start out of a station in direction north oder west (up the screen)
the reservation is so short that it got no green signal.

until a curve or something else then it works normal

It is just funny.

[prissi]

A train will usually reserve four tiles ahead or until the next signal. Reserving through four signals like below looks wrong.

On the other hand, signal spacing of zero means max speed of 50km/h (1=100, 2=200, 3>=unlimited)

[makie]

Reserving through four signals like below looks wrong.

No i am fine with that.
Curious is that this depend and varies of the driving direction and where it comes from.

The handling in the program is not equal.

On the other hand, signal spacing of zero means max speed of 50km/h (1=100, 2=200, 3>=unlimited)

Reserving through four signals allow fast then that. I think that's good.

[victor_18993]

Thanks, Makie and Prissi.

Since the signal behaviour that was mentioned appears to be unrelated to this patch and may be an existing reservation-system behaviour, I consider it outside the scope of this work.

The diagonal image selection is now working with eight positions per state, and the submitted diff contains everything requested within the agreed scope.

Unless the review reveals a regression or an additional adjustment is required, this should be the final diff for review and integration.

[makie]

Unless something else comes up, I'd say that looks good and is finished.


It's going to take a while for me to work my way through all the signals and traffic signs. After all, there are only three of them.

Edit:
Oh i forgot: Drive to the left is open. I don't need it, but maybe some other.
Quick check: nothing change when activated. The position by diagonals is as bevor.