OpenOrienteering / mapper

OpenOrienteering Mapper is a software for creating maps for the orienteering sport.
https://www.openorienteering.org/apps/mapper/
GNU General Public License v3.0
404 stars 108 forks source link

Spurious text in title #2206

Closed hfm-zz closed 5 months ago

hfm-zz commented 8 months ago

Steps to reproduce

  1. I entered the title of a map in OOM using the Write text tool
  2. I saved the map and viewed it in Condes, which I am using for course planning on OOM: image

Actual behaviour

The title of the map was displayed but with other text (present elsewhere on the map) added at the end.
I deleted the title, created a new version and got different spurious text On Condes: image

Expected behaviour

Just the title text I had entered to display

Configuration

Mapper Version: 0.9.5 Operating System: W 10 Condes 10.5.7

hfm-zz commented 8 months ago

Note, I have only had this problem with one Text tool, i.e. teh largest text used on the map, not smaller text with teh same font, or (e.g.) the small text for legend

hfm-zz commented 8 months ago

When I opened my map (saved in OCAD v11 format) in OOM today, the title had gained some spurious non-alphabetic characters: image which is what then appeared in Condes on saving in OOM I have deleted the Title on the map and re-entered it, only to get different spurious text. It doesn't seem to matter which version of OCAD I save as, v10, v11 or v12

dg0yt commented 8 months ago

With regard to the text object of interest:

Do I understand correctly that the text object from files saved as OCD in Mapper is not displayed "as saved" when viewing in Condes? Is it displayed "as saved" when viewing in OCAD (Viewer)? Do I understand correctly that the text object from files saved as OCD as Condes is displayed "as saved" when viewing in Mapper?

You can try to provide a minimal test file.

hfm-zz commented 8 months ago

Hello Kai,

I have attached an OOM map file saved as OCAD v10 and a test event created using that as the map file in Condes in my email. Attach to the forum as a .zip file at the end

I am also presenting thumbnail screen shots in case it doesn’t reproduce for you.

OOM image

OCAD viewer image

Condes, using raw OOM map image

Condes after saving in OCAD viewer image

I highlight 3 differences

  1. The Sandall Beat and Armthorpe Park title gains extra spurious characters if the map file is used raw from OOM. If the OOM map is edited and saved, if Condes is running it will update the map but add the spurious characters. A work-round is to save the map, open it in OCAD viewer where it is converted, save it and then Condes automatically updates correctly. (This works but is an annoying extra step - if you are updating the map as you check control sites, spot a difference, make a change but have to do an extra step)

  2. A blue line has appeared coinciding with the bottom of the map frame, and the frame has extended. This didn’t appear in the OCAD viewer, but occurs with both versions of the file in Condes

  3. A long term problem – I originally created the file to check in the appearance of short paths. In OOM, short paths, indistinct paths and rides are presented as solid lines, whereas OCAD allows the mapper to ‘bend’ the rules and show breaks in the line. Condes allows the mapper to present the paths as desired, with both map versions.

In direct answer to your questions:

The text object from files saved as OCD in Mapper is not displayed "as saved" when viewing in Condes

The text object is displayed "as saved" when viewing in OCAD (Viewer)

I don’t quite understand the last question, but The text object from files converted and saved using the OCAD viewer is displayed "as saved" in Mapper when viewing in Condes

Answers would be different for short paths and the blue line

For some reason, the ‘Sandall Beat and Pit Top Park’ title does not misbehave. I have tried deleting and re-creating the Sandall Beat and Armthorpe Park title with no success (but different spurious characters)

All the best,

Henry

Sandall 2024.zip

lpechacek commented 8 months ago

When I opened my map (saved in OCAD v11 format) in OOM today, the title had gained some spurious non-alphabetic characters:

FWIW, I observed the same effect on one of my maps. I didn't bother debugging the issue though as I had other entertainment at hand.

hfm-zz commented 8 months ago

Hello Ipechacek In confirmation of your observation, the effect was not so easy to document and report. Could be overcome by deleting those characters. However, I have just closed my test map in OOM and reopened it with no further action and got this, using OOM to edit and save files in .ocd format: image

For some reason, only one of the 2 titles using the same symbol icon are affexted, and none of the other text boxes. Opening the condes file again, I got: image

dl3sdo commented 8 months ago

I am working on a fix. The error is caused by an invalid text object without a terminating null character. You will notice that the error is not present for shorter or longer text objects but the current text has a length of 32 characters (including \n\r). BTW: If I create a new text object in OCAD10 with 32 characters the object is ok, so it's unclear what causes this invalid text object.

lpechacek commented 7 months ago

If you don't mind, I'll record my side remarks on this issue here. The text adds context for the issue report and the fixes.

It looks like that this issue report is actually about two issues. The observations are: 1) Export 32-character long text object into OCD format, open it in Condes, and there is some trailing garbage appended. 2) Open the same OCD file in Mapper and there is garbage appended. 3) Open the OCD file in OCAD Viewer and all is fine. 4) Save the file from OCAD Viewer and both Mapper and Condes display the text object correctly

Mapper had a bug in the OCD import routine because it relied on the fact that there will be zeros at the end of the text (point 2 above). Condes shows the same behavior, so I guess that it has the same bug (point 1). OCAD Viewer is immune to the problem (point 3, Pascal does not rely on terminating zeros). OCAD can save the text object in such a form so that both Condes and Mapper bugs are not triggered.

As a result, we've got two fixes in Mapper for the two issues: 1) Make Mapper respect the reported string length in OCD import (PR#2207) 2) Ensure that there is a zero at the end of the string in the exported OCD file (PR#2208)

The combination seems to resolve this issue in full, and I agree with both the fixes. The only thing that could be improved is to mention that we are adding the trailing zero due to compatibility with Condes and older Mapper releases. I believe that future code explorers will be happy for that commit message remark.


Illustration of the missing terminating zero in Kaitai Struct IDE (without the fix from PR#2208): Screenshot from 2024-04-04 12-01-24 The characters following the blue area are added to the text in import.


Illustration of the additional zeros in Kaitai Struct IDE (with the fix from PR#2208): Screenshot from 2024-04-04 12-06-28 There is a "protection wall" of zeros in the blue area that ensures that the text will be correctly imported by Condes.

dl3sdo commented 7 months ago

Libor, thank you for summary. I only disagree with one statement: "The only thing that could be improved is to mention that we are adding the trailing zero due to compatibility with Condes and older Mapper releases" We are adding the trailing zero because of the official .ocd format that recommends a trailing zero although OCAD is tolerant with respect to a missing terminating zero. If you enter 32 characters in OCAD, OCAD will attach a 64 block of zeros, so OCAD seems to always enforce a terminating zero.

lpechacek commented 7 months ago

We are adding the trailing zero because of the official .ocd format that recommends a trailing zero

Thanks for the remark, Matthias. Now I can see it in the docs too. The description of TOcadObject.Poly reads: "... coordinates of the object followed by a zero-terminated string if nText > 0." So yes, I'd say that the null-termination is even mandatory.

Still, I recommend amending the commit message with the reason why was the trailing zero missing. As you can see, even a guy who saw the OCD format docs about 20 years ago, may have missed that detail.

hfm-zz commented 7 months ago

Thank you!

All the best,

Henry Marston

From: Libor Pecháček @.*** Sent: 04 April 2024 22:47 To: OpenOrienteering/mapper Cc: hfm-zz; Author Subject: Re: [OpenOrienteering/mapper] Spurious text in title (Issue #2206)

We are adding the trailing zero because of the official .ocd format that recommends a trailing zero

Thanks for the remark, Matthias. Now I can see it in the docs too. The description of TOcadObject.Poly reads: "... coordinates of the object followed by a zero-terminated string if nText > 0." So yes, I'd say that the null-termination is even mandatory.

Still, I recommend amending the commit message with the reason why was the trailing zero missing. As you can see, even a guy who saw the OCD format docs about 20 years ago, may have missed that detail.

— Reply to this email directly, view it on GitHub https://github.com/OpenOrienteering/mapper/issues/2206#issuecomment-2038289161 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6USTWX7D5L36RXWVRJT3LY3XC4HAVCNFSM6AAAAABELPJ2J2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZYGI4DSMJWGE . You are receiving this because you authored the thread. https://github.com/notifications/beacon/AA6USTQS4XDE7NYD5SOVKSTY3XC4HA5CNFSM6AAAAABELPJ2J2WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTZPXJQS.gif Message ID: @.***>

dl3sdo commented 7 months ago

Libor, I extended the commit message. Is it ok for you?

lpechacek commented 7 months ago

Libor, I extended the commit message. Is it ok for you?

Yeah, that one is nice. Thanks!