Add weak constraints to make rects closer to each other in size

[kdheepak]

This PR introduces a SegmentSize enum that will allow adding the following constraints optionally:

        for i in 0..elements.len() {
            for j in (i + 1)..elements.len() {
                solver.add_constraint(elements[j].size() | EQ(WEAK) | (elements[i].size()))?;
            }
        }

This makes chunks similar in size to each other if other constraints are satisfied.

This PR also changes the implementation of Max and Min by adding a MEDIUM equality constraint instead of a WEAK equality constraint.

     Constraint::Max(m) => {
         solver.add_constraints(&[                           
             element.size() | LE(STRONG) | f64::from(m),     
-            element.size() | EQ(WEAK) | f64::from(m),
+            element.size() | EQ(MEDIUM) | f64::from(m),
         ])?;                                                
     }                                                       
     Constraint::Min(m) => {                                 
         solver.add_constraints(&[                           
             element.size() | GE(STRONG) | f64::from(m),     
-            element.size() | EQ(WEAK) | f64::from(m),
+            element.size() | EQ(MEDIUM) | f64::from(m),
         ])?;                                                
     }                                                       

It should result in the same layouts for us though, because there were no other MEDIUM constraints used.

[hasezoey]

i know this a bit unrelated, but this change causes some side-effects, please test against #404 with this change applied, see #370 (comment) for more context

[joshka]

i know this a bit unrelated, but this change causes some side-effects, please test against #404 with this change applied, see #370 (comment) for more context

The main blocker for this PR right now is that it results in non-deterministic splits more often, which is makes it difficult to reason about how space will be allocated for users.

[kdheepak]

I did some testing with this and wrote a integer optimization program (in Julia) to simulate "ideal" behavior to compare. I think we are never going to be able to guarantee that there won't be gaps with cassowary.

For example, take a constraints Percent(33), Percent(33), Percent(33) in a Horizontal layout of area whose width is 100. The "ideal" result is going to be 33.3333, 33.3333 and 33.3333 for each Rect, and rounded you are going to get 33, 33, 33 which will leave a gap of 1.

I implemented this using integer variables in Julia and I get a solution that we would expect, i.e. 33, 34 and 33.

    area = Rect(0, 0, 100, 1)
    constraints = [Percent(33), Percent(33), Percent(33)]
    r1, r2, r3 = split(Horizontal(constraints), area; kwargs...)
    @test r1.width == 33
    @test r2.width == 34
    @test r3.width == 33

Unfortunately, this behavior is not guaranteed with cassowary because cassowary uses floating point values.

[kdheepak]

Adding the weak constraints to make the widths (or heights) similar did have the intended effect (imo). Take this test for example:

    area = Rect(0, 0, 100, 1)
    constraints = [Percent(50), HardMin(10), Percent(50)]
    r1, r2, r3 = split(Horizontal(constraints), area; kwargs...)
    @test r1.width == 45
    @test r2.width == 10
    @test r3.width == 45

Without the weak constraints, I was getting results like this:

    area = Rect(0, 0, 100, 1)
    constraints = [Percent(50), HardMin(10), Percent(50)]
    r1, r2, r3 = split(Horizontal(constraints), area; kwargs...)
    @test r1.width == 50
    @test r2.width == 10
    @test r3.width == 40

[joshka]

I think we are never going to be able to guarantee that there won't be gaps with cassowary.

The approach that makes this round without gaps is to use the left/right position rather than the widths to work out how to round.
Thus instead of thinking of [33.33, 33.33, 33.33] => [33, 33, 33], think instead of [0.00, 33.33, 66.66, 100.00] => [0, 33, 67, 100]

This works pretty nicely as far as I can tell and is my intended change as soon as the refactor in #405 lands. See #408 for the change.

[kdheepak]

Interesting idea! I think there are some corner cases there as well (I'll have to test to come up with examples), but maybe fewer issues than what we have at the moment, so it should be good!

[joshka]

Interesting idea! I think there are some corner cases there as well (I'll have to test to come up with examples), but maybe fewer issues than what we have at the moment, so it should be good!

Perhaps something that evaluates to (positions): [0.0, 0.51, 1.49, 2] => [0, 1, 1, 2]. The widths would be [0.51, 0.98, 0.51], but become [1, 0, 1] after rounding.

[codecov]

Codecov Report

Merging #395 (23af050) into main (dc55211) will increase coverage by 0.12%.
The diff coverage is 100.00%.

❗ Current head 23af050 differs from pull request most recent head 29bd834. Consider uploading reports for the commit 29bd834 to get more accurate results

@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
+ Coverage   86.28%   86.41%   +0.12%     
==========================================
  Files          40       40              
  Lines        9343     9432      +89     
==========================================
+ Hits         8062     8151      +89     
  Misses       1281     1281              

Files Changed	Coverage Δ
src/layout.rs	96.72% <100.00%> (+0.39%)	⬆️
src/widgets/table.rs	94.51% <100.00%> (ø)

[hasezoey]

code looks good to me, though it would probably be great to also have some tests for Max and Length to ensure they are not somehow heigher / lower than what they should be

[kdheepak]

i know this a bit unrelated, but this change causes some side-effects, please test against #404 with this change applied, see #370 (comment) for more context

@hasezoey I had to fix some tests after #404 was merged and after I rebased on top of it.

[kdheepak]

I renamed it to SegmentSize and made it pub(crate) so that we can finalize the name later with more feedback.

[joshka]

I pushed a rebase on the fix in #419 - happy for this to be approved and merged now @kdheepak. We can work out the public name for the API at some point later. I lean towards using Allocation as the name and communicating that this is a preference not a requirement in the enum variant itself, and also using an imperative tone that says what the allocation will do (e.g. PreferEvenWidths, FillLastArea, etc.)

[kdheepak]

I did the rebase locally as well but you are faster than me :) I'll discard my changes.

Yes, let's merge and figure out the right names for it later. I'm not tied to any names here.