simulton / QSchematic

A library that allows creating diagrams such as flowcharts or even proper engineering schematics within a Qt application.
https://simulton.com
MIT License
239 stars 62 forks source link

Edge Case in connector_moved and preserveStraightAngles=true resulting in non 90° angle #27

Open ChristianWieden opened 3 years ago

ChristianWieden commented 3 years ago

When moving a connector around it is possible to get a non 90° angle with the setting preserveStraightAngles true.

This happens while moving a node which is attached to a wire/net close to a note which is connected to the same wire. So that they are overlapping or very close.

Image from demo. image

Now then the connector is moved by dx+1 and dy+1 at the same time a non 90° degree angle can be created.

Image from demo image image

I wrote this test case to check and it fails.

FATAL ERROR: REQUIRE( wire->points_count() == 3 ) is NOT correct! values: REQUIRE( 2 == 3 )

TEST_CASE ("connector_moved(): Moving a connector with a wire attached to another connector with distance = 1 by dx=1 and dy=1")
    {
        wire_system::manager manager;

        // Create the first wire
        auto wire = std::make_shared<wire_system::wire>();
        wire->append_point({0, 10});
        wire->append_point({1, 10});
        manager.add_wire(wire);

        // Create and attach the connector
        connector conn;
        conn.pos = QPointF(0, 10);
        manager.attach_wire_to_connector(wire.get(), &conn);
        connector conn2;
        conn2.pos = QPointF(1, 10);
        manager.attach_wire_to_connector(wire.get(), &conn2);

        // Prepare the settings
        Settings settings;
        settings.gridSize = 1;

        SUBCASE("Straight angles are maintained") {
            // Enable the straight angles
            settings.preserveStraightAngles = true;
            manager.set_settings(settings);

            // Move the connector
            conn2.pos = QPointF(2, 11);
            manager.connector_moved(&conn2);

            // Make sure every thing is as expected
            REQUIRE(wire->points_count() == 3);
            REQUIRE(wire->points().at(0).toPointF() == QPointF(0, 10));
            REQUIRE(wire->points().at(1).toPointF() == QPointF(2, 10));
            REQUIRE(wire->points().at(2).toPointF() == QPointF(2, 11));
        }
    }
Tectu commented 3 years ago

Thank you for bringing this to my attention :) Do you feel like working out a patch for this? :p

ChristianWieden commented 3 years ago

Hi,

i can take a look at the wiring code over the weekend. But i promise nothing, did not have the pleasure to work with c++17 code yet :D

ChristianWieden commented 3 years ago

Made a fix for this, every test case was okay, and tried in the Demo and saw a few problems, so at the moment i write more tests before tackling this.

Tectu commented 3 years ago

That sounds great! Looking forward to a pull request :)

Tectu commented 3 years ago

Any news on this? :)

ChristianWieden commented 3 years ago

Hi, sadly not, did not have much time since last time i wrote here.