esmini / esmini

a basic OpenSCENARIO player
Mozilla Public License 2.0
756 stars 216 forks source link

Signals are at a wrong position (t-value) when lane offset is present #625

Open johschmitz opened 3 days ago

johschmitz commented 3 days ago

Consider this example:

export.zip

image

It seems that the signals are in the wrong position (wrong t-value).

As far as I understand (I hope I am not wrong, I will also ask some other people) the signal and object positioning has to ignore the lane offset.

I also wonder why the visualization of the reference line (or is it the center lane) includes the lane offset, I think that could be improved by having a separate visualization for the reference line and the center lane.

See https://publications.pages.asam.net/standards/ASAM_OpenDRIVE/ASAM_OpenDRIVE_Specification/latest/specification/11_lanes/11_04_lane_offset.html

image

eknabevcc commented 1 day ago

I think you are right. What happens is that esmini shift also the reference line. This is a known deviation in esmini. See here: https://esmini.github.io/#_reference_line_and_center_lane_while_using_laneoffset

It's in the long backlog, but will probably not be fixed in near time.

Anyway, thanks for reporting this case, it's a good and clear example.

johschmitz commented 1 day ago

Would it be at least possible to fix it for the signal/object position? For the road/lanes I think it is less critical but for the signal position it makes tooling quite incompatible.

johschmitz commented 1 day ago

I want to add to my question, in case there's no capacity to implement this on your side, would you at least accept a PR for this? Or are there some downwards compatibility issues with that?

eknabevcc commented 1 day ago

I did think briefly about your question regarding special case for signs. The feasibility could be investigated. However, I have a vague feeling that it might clutter the code. Or at least make thing inconsistent. But I might be wrong (I better add :))

Preferred approach would be to implement the correct lane offset handling. I'm not, as of now, totally sure what that means. We haven't really investigated what needs to be done in esmini. Hopefully it's not very tricky. But I wouldn't count on it.

Regarding compatibility, I hope and don't think it should become an issue. But I don't know for sure.

We have so much in the pipe that we are working on, why we need to prioritize hard.

Sorry for dizzy elaboration. I guess the answer is: Feel free to investigate feasibility of 1. special case and/or 2. full lane offset fix. But don't count on PR acceptance.

Regardless, the reported issue itself affects the prioritization of lane offset fix. Tool harmonization we strive for.

johschmitz commented 1 day ago

Okay that sounds at least as if there are no gigantic blockers relying on this "wrong" behavior hindering the introduction of a "fix".

From my understanding the high level approach is rather simple and goes like this:

That is why it is also in the XML inside of the <lanes> element and not the <geometry> element AFAIK.

I don't know what it means for the esmini implementation but I guess I will have a rough look when I find time and report my findings in another comment.

eknabevcc commented 13 hours ago

It makes sense. At least at first glance. I hope it's a simple change that we can test for unexpected side effects.

johschmitz commented 3 hours ago

I hacked together a quick first draft PR to try to fix this #626

image

Would be nice if it could be reviewed. If possible please also suggest how to rename the variables in RoadManager.cpp Line 8359 in XYZ2TrackPos() it is somewhat confusing with all the different things called lane offset.