mapper icon indicating copy to clipboard operation
mapper copied to clipboard

Undefined rendering behavior with multiple mid symbols in dash break

Open dg0yt opened this issue 10 months ago • 6 comments

Crashing in debug builds:

Use ISOM 2017-2 symbol 517 Ruined fence,
change Mid symbols settings to Center of gaps, 3 mid symbols per spot and increase mid symbol distance.
The symbol editor will crash Mapper while increasing to 0.xx mm.
ASSERT: "p >= 0.0f" in file path_coord.cpp, line 471

Originally posted by @dl3sdo in https://github.com/OpenOrienteering/mapper/issues/2099#issuecomment-2611068947

dg0yt avatar Jan 24 '25 08:01 dg0yt

  • assert is inactive in release builds.
  • SplitPathCoord::at(length_type len, SplitPathCoord first) is defined to search from first. It is not defined what should happen when len is negative, before first, or after the end.

dg0yt avatar Jan 24 '25 08:01 dg0yt

Summary:

  • Everything is fine when all n symbols fit into the gap, i.e. (n-1) x symbol_distance <= gap_length. 🎉
    • The row of symbols is centered around the middle of the gap.
  • Unlike dash length, gap length doesn't vary. 🎉
  • Unlike dash length, distance of symbols doesn't vary. 🎉
  • Rendering may be unexpected when the length needed to place the symbols exceeds the length of the gap.
    • Placing of symbols depends on user-defined points.
    • Placing of symbols depends on internal representations of curved paths.
    • Placing of symbols may vary for straight vs. curved paths.
    • The (visible) number of symbols may be less than the defined number of symbols.
    • Some symbols may appear in unexpected locations (still on the path, but breaking the equal spacing).
    • Debug builds may hit an assert.
      Which is a good thing. This assert is meant to point out that implemented behavior is undefined and that input to that function must be improved.

Suggestion:

  • Place symbols with fixed distance.
  • Always center around middle of gap.
  • Only drop symbols which are outside of the total path.

dg0yt avatar Jan 31 '25 08:01 dg0yt

I thought about whether it makes sense to either inhibit or indicate in the line symbol editor a combination of number of mid symbols and their distance which exceeds the gap, however:

  • most users just use provided symbols sets
  • users editing a symbol set should know what they do and they will immediately see the result
  • Mapper will not crash

Therefore I'm fine with your proposal, Kai.

dl3sdo avatar Jan 31 '25 09:01 dl3sdo

Test file: 2324.zip

dg0yt avatar Jan 31 '25 14:01 dg0yt

I've hit this assertion also in a dev build of master 69da0d02. It occurs when I scale the following isolated railway line down to about 25% of original size. The file originates from an OCAD file, but has been stripped of other junk in process of isolating the cause of the assertion failure. File is attached.

railway-symbol-scale-assertion.zip

elektronisk avatar Apr 09 '25 19:04 elektronisk

#2325 (merged) is one change for this issue. https://github.com/OpenOrienteering/mapper/pull/2331 is another relevant PR but I don't want to merge without more testing. (It refers to #2325 but I think it was meant to refer to #2324.) The assertion is only enabled in debug builds.

dg0yt avatar Apr 09 '25 21:04 dg0yt