odb: fix wire decoding when the target shape is connected to a colinear junction

[AcKoucher]

Related to #3969.

Context

While using the new segment regression from #3989 to generate new set_layer_rc values for asap7, I stumbled into a behavior in which, only on M2, the regression coefficient of determination was very poor for resistance.

This didn't seem to make any sense, because, based on the fact that the RCX extraction model computes resistance as

R = sheet_resistance * length / width

And for minimum-width segments width is constant and we'd have:

R = (sheet_resistance / width) * length = reg_coef * length

The result is literally a linear relation, so we should have R² == 1 - which we do for all the other layers.

After some debugging I realized that in the generated RC .csv there was a couple of very large segments (literally the size of the entire core width) of a certain net with a much lower per-length resistance than the rest of the segments. After using the GUI to take a look at the net it became clear that the segments' shapes were being wrongly computed as it was a net with relatively short segments.

Example of a net (signal) with segments being wrongly computed by odb::Wire::getShape on asap7/ibex, A and B are the problematic segments:

Segment	Shape (wrong coord. in bold)
	(58896, 63324) (118242, 63342)
	(-792, 65718) (61506, 65736)

As we can see the point is wrongly computed for the colinear junctions of these segments (right junction in the first one, left junction on the second one).

Problem

1. Colinear junctions are not correctly handled by the decoder state machine in dbWire::getSegment. The decoder currently treats the junction as if it were non colinear and retrieves the extension value from the wrong data_ index. The extension value for a kColinear junction is stored in its own index (data_[idx]):

Entry	Opcode	Data
0	kJunction	jct_id
1	kColinear | WOP_EXTENSION	extension ← read from data_[idx]
2	kX	x_coord

Currently, the decoder is reading the extension value from data_[idx+1] and giving the wrong extension value to the function that actually computes the geometry, resulting in wrong final geometry.

Changes

When decoding and stumbling into a kColinear junction, retrieve the extension value correctly.

[gemini-code-assist]

Code Review

This pull request correctly fixes a bug in the wire decoding logic for kColinear junctions with extensions. The change ensures the extension value is read from the correct index. A new test case has been added to cover this specific scenario, which is great. My review focuses on a maintainability improvement: the fix introduces duplicated code in two overloads of the getSegment function. I've suggested refactoring this duplicated logic to improve code quality, aligning with the principle of encapsulating logic in helper functions to avoid duplication.

[gemini-code-assist]

This logic is duplicated in the getSegment overload at lines 790-791. To improve maintainability and avoid code duplication, consider extracting this logic into a static helper function. This aligns with the principle of encapsulating logic in helper functions to avoid duplication.

For example:

static int getCurExt(const _dbWire* wire, unsigned char opcode, int idx)
{
  const WireOp wire_op = static_cast<WireOp>(opcode & WOP_OPCODE_MASK);
  return wire_op == kColinear ? wire->data_[idx] : wire->data_[idx + 1];
}

This would allow you to call cur_ext = getCurExt(wire, opcode, idx); in both locations.

As a smaller change, this logic can be condensed into a single line.

      cur_ext = (static_cast<WireOp>(opcode & WOP_OPCODE_MASK) == kColinear) ? wire->data_[idx] : wire->data_[idx + 1];

References

1. Encapsulate the logic for calculating master symmetry in a helper function to avoid code duplication.

[AcKoucher]

I think that conceptually it would be better to refactor the entire state machine as there are two large blocks of code for it. That said, it would make the review rather painful if included in this PR.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[maliberty]

Please share an example net that shows the issue. Its is very messy, old code but also quite important.

[AcKoucher]

Please share an example net that shows the issue. Its is very messy, old code but also quite important.

I added images to the description.

[AcKoucher]

@maliberty I rebased to improve the commit message and fix the bazel.lock check failure that was happening.

This should be good to go.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[maliberty]

To really review this I would have to first fully grok the wire encoder and the representation it builds. I'm not prepared to spend the time on that now so I'll go with the empirical result.