search

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

OcdFileImport: Revise path coord processing #2224

Open dg0yt opened 7 months ago

dg0yt commented 7 months ago

Resolves #2144: Importing accepting pairs of hole points, but editor crashing when selecting the export.

This PR moves some single-use helper functions back into the processing loops. This allows optimizing the conditions and adding the desired fix. (The original functions might have been more easy to analyze separately, but passing and modifying pos in the call stack added complexity.)

The PR is broken into a series of commits which allow to track the source code transformation. There is no specific unit test to cover this change.

dl3sdo commented 3 months ago

I'm currently writing an unit test.

dl3sdo commented 3 months ago

Kai, I had to rebase your PR to base it on the latest version of file_format_t. I was not sure whether I should force-push to your branch, so I pushed to https://github.com/dl3sdo/mapper/tree/ocd-hole-points

Please check whether the unit test goes into the right direction.

I'm currently only testing area objects and I don't test all the flags. Although I crafted the .ocd coords to be real coordinates I intentionally don't check the x- and y-values of the coordinates after conversion.

dg0yt commented 3 months ago

https://github.com/dl3sdo/mapper/tree/ocd-hole-points

I will pick the commits which add the actual test and work on it separately first.

dg0yt commented 3 months ago

Moving to this test data pattern:

    QTest::addColumn<OcdPointsView>("data");
    QTest::addColumn<FlagsView>("expected");

    {
        static Ocd::OcdPoint32 ocd_points[] = {
            // +0,  bezier
            { C(-1109), C(212) },
            { C(-1035) | Ocd::OcdPoint32::FlagCtl1, C(302) },
            { C(-1008) | Ocd::OcdPoint32::FlagCtl2, C(519) },
            { C(-926), C(437) },
        };
        static int expected_flags[] = {
            MapCoord::CurveStart, 0, 0, 0, MapCoord::ClosePoint,
        };
        QTest::newRow("bezier") << OcdPointsView(ocd_points) << FlagsView(expected_flags);
    }

Those view classes are litte more than [ size, pointer ].

dg0yt commented 3 months ago

Plan: