mkeeter / antimony

CAD from a parallel universe
2.09k stars 159 forks source link

Fix use of deprecated APIs in Qt 5.15 #235

Closed rnhmjoj closed 1 year ago

rnhmjoj commented 1 year ago

This is necessary to build Antimony with Qt 5.15, older versions of Qt are being deprecated and removed by distributions.

mkeeter commented 1 year ago

This looks reasonable, except the usage of QMultiMap instead of QMap - can you explain what that's about / what errors you were seeing before those changes? It changes the semantics of the operations, so it's not a drop-in replacement.

rnhmjoj commented 1 year ago

It changes the semantics of the operations, so it's not a drop-in replacement.

I tried following the Qt guide regarding the deprecation and migrating the code from QMap to QMultiMap assuming multiple values per-key weren't allowed before. If this is not true, I don't how to port the code, because I'm not sure about what map[key] would do before, with multiple values.

mkeeter commented 1 year ago

I don't believe I'm using multiple values per key here. The list of obsolete members doesn't include operator[].

rnhmjoj commented 1 year ago

I don't believe I'm using multiple values per key here.

Ok, then it should be fine. By the way, I build Antimony with Qt 5.15 and tested it by opening some .sb files and playing around a bit: everything seems fine.

The list of obsolete members doesn't include operator[].

As I said in the comment above, I had to change QMap to QMultiMap because unite() has been moved into QMultiMap and deprecated in QMap.

mkeeter commented 1 year ago

Aha, I missed your comment about unite.

I believe that it's only used to join two maps – and not for the multi-value behavior – so could be replaced here with

void CanvasInfo::unite(const CanvasInfo& other)
{
    for (const auto i: other.inspector) {
        inspector.insert(i);
    }
    for (const auto s: other.subdatum) {
        subdatum.insert(s);
    }
}

(untested)

It looks like operator[] is still public for QMap, so if you modify the CanvasInfo::unite implementation (and switch back to QMap), you could go back to using it.

rnhmjoj commented 1 year ago

I've found a simpler solution: I missed that 5.15 overloaded the insert() method to take another QMap: assuming that no duplicate keys exists it works exactly like unite().

mkeeter commented 1 year ago

Thanks!

rnhmjoj commented 1 year ago

Thank you. I really appreciate the quick response even if the project is in low mantainenance mode.